Changes after review

3 files changed, 54 insertions, 49 deletions

diff --git a/src/main/java/org/traccar/MainModule.java b/src/main/java/org/traccar/MainModule.java
index 3acd19b6a..24448ef51 100644
--- a/src/main/java/org/traccar/MainModule.java
+++ b/src/main/java/org/traccar/MainModule.java
@@ -73,6 +73,7 @@ import org.traccar.reports.model.TripsConfig;
 
 import javax.annotation.Nullable;
 import javax.ws.rs.client.Client;
+import io.netty.util.Timer;
 
 public class MainModule extends AbstractModule {
 
@@ -375,6 +376,12 @@ public class MainModule extends AbstractModule {
         return new DriverEventHandler(identityManager);
     }
 
+    @Singleton
+    @Provides
+    public static Timer provideGlobalTimer() {
+        return GlobalTimer.getTimer();
+    }
+
     @Override
     protected void configure() {
         binder().requireExplicitBindings();
diff --git a/src/main/java/org/traccar/WebDataHandler.java b/src/main/java/org/traccar/WebDataHandler.java
index e2f7c1e8b..58b9ca73b 100644
--- a/src/main/java/org/traccar/WebDataHandler.java
+++ b/src/main/java/org/traccar/WebDataHandler.java
@@ -18,6 +18,7 @@ package org.traccar;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import io.netty.channel.ChannelHandler;
+import io.netty.util.Timer;
 import io.netty.util.Timeout;
 import io.netty.util.TimerTask;
 import org.slf4j.Logger;
@@ -66,11 +67,11 @@ public class WebDataHandler extends BaseDataHandler {
     private final boolean json;
 
     private final boolean retryEnabled;
-    private final int retryDelayMin;
-    private final int retryDelayMax;
-    private final int deliveryPendingLimit;
+    private final int retryDelay;
+    private final int retryCount;
+    private final int retryLimit;
 
-    private AtomicInteger deliveryPendingCurrent;
+    private AtomicInteger deliveryPending;
 
     @Inject
     public WebDataHandler(
@@ -84,11 +85,11 @@ public class WebDataHandler extends BaseDataHandler {
         this.json = config.getBoolean(Keys.FORWARD_JSON);
 
         this.retryEnabled = config.getBoolean(Keys.FORWARD_RETRY_ENABLE);
-        this.retryDelayMin = config.getInteger(Keys.FORWARD_RETRY_DELAY_MIN, 1);
-        this.retryDelayMax = config.getInteger(Keys.FORWARD_RETRY_DELAY_MAX, 10);
-        this.deliveryPendingLimit = config.getInteger(Keys.FORWARD_RETRY_PENDING_LIMIT, 100);
+        this.retryDelay = config.getInteger(Keys.FORWARD_RETRY_DELAY, 100);
+        this.retryCount = config.getInteger(Keys.FORWARD_RETRY_COUNT, 10);
+        this.retryLimit = config.getInteger(Keys.FORWARD_RETRY_LIMIT, 100);
 
-        this.deliveryPendingCurrent = new AtomicInteger(0);
+        this.deliveryPending = new AtomicInteger(0);
     }
 
     private static String formatSentence(Position position) {
@@ -183,7 +184,7 @@ public class WebDataHandler extends BaseDataHandler {
 
         class AsyncRequestAndCallback implements InvocationCallback<Response> {
 
-            private int delay = retryDelayMin;
+            private int retries = 0;
             private Map<String, Object> payload;
             private Invocation.Builder requestBuilder;
 
@@ -208,7 +209,7 @@ public class WebDataHandler extends BaseDataHandler {
                     payload = prepareJsonPayload(position);
                 }
 
-                deliveryPendingCurrent.incrementAndGet();
+                deliveryPending.incrementAndGet();
 
                 send();
             }
@@ -222,40 +223,37 @@ public class WebDataHandler extends BaseDataHandler {
             }
 
             private void retry() {
+                boolean ok = false;
                 try {
-                    String message = "Position forwarding failed.";
-                    if (!retryEnabled) {
-                        LOGGER.warn(message);
-                    } else {
-                        int pending = deliveryPendingCurrent.get();
-                        if (pending <= deliveryPendingLimit) {
-                            LOGGER.warn(message + " Pending: " + pending
-                                + ". Retrying in " + delay + " seconds.");
-                            GlobalTimer.getTimer().newTimeout(new TimerTask() {
-                                @Override
-                                public void run(Timeout timeout) {
+                    if (retryEnabled && deliveryPending.get() <= retryLimit && retries < retryCount) {
+                        Main.getInjector().getInstance(Timer.class).newTimeout(new TimerTask() {
+                            @Override
+                            public void run(Timeout timeout) {
+                                boolean ok = false;
+                                try {
                                     if (!timeout.isCancelled()) {
-                                        if (delay < retryDelayMax) {
-                                            delay++;
-                                        }
                                         send();
+                                        ok = true;
+                                    }
+                                } finally {
+                                    if (!ok) {
+                                        deliveryPending.decrementAndGet();
                                     }
                                 }
-                            }, delay, TimeUnit.SECONDS);
-                            return;
-                        }
-                        LOGGER.warn(message + " Pending: " + pending
-                            + ". Delivery will not be retried.");
+                            }
+                        }, retryDelay * (int) Math.pow(2, retries++), TimeUnit.MILLISECONDS);
+                        ok = true;
                     }
-                } catch (Exception e) {
+                } finally {
+                    int pending = ok ? deliveryPending.get() : deliveryPending.decrementAndGet();
+                    LOGGER.warn("Position forwarding failed: " + pending + " pending");
                 }
-                deliveryPendingCurrent.decrementAndGet();
             }
 
             @Override
             public void completed(Response response) {
-                if (response.getStatus() == 200) {
-                    deliveryPendingCurrent.decrementAndGet();
+                if (response.getStatusInfo().getFamily() == Response.Status.Family.SUCCESSFUL) {
+                    deliveryPending.decrementAndGet();
                 } else {
                     retry();
                 }
diff --git a/src/main/java/org/traccar/config/Keys.java b/src/main/java/org/traccar/config/Keys.java
index 999a1d6df..d88b36d28 100644
--- a/src/main/java/org/traccar/config/Keys.java
+++ b/src/main/java/org/traccar/config/Keys.java
@@ -101,35 +101,35 @@ public final class Keys {
             "forward.json", Boolean.class);
 
     /**
-     * Position forwarding retrying enable. When enabled, additional attempts are made to deliver positions.
-     * If initial delivery fails, because of an unreachable server or an HTTP response different from '200 OK',
-     * the software waits for 'forward.retry.delay.min' seconds to retry delivery. On subsequent failures, this
-     * delay is incremented by 1 second up to 'forward.retry.delay.max'. Positions pending to be delivered
-     * are limited to 'forward.retry.pending.limit'. If this limit is reached, positions get discarded.
+     * Position forwarding retrying enable. When enabled, additional attempts are made to deliver positions. If initial
+     * delivery fails, because of an unreachable server or an HTTP response different from '2xx', the software waits
+     * for 'forward.retry.delay' milliseconds to retry delivery. On subsequent failures, this delay is duplicated.
+     * If forwarding is retried for 'forward.retry.count', retrying is canceled and the position is dropped. Positions
+     * pending to be delivered are limited to 'forward.retry.limit'. If this limit is reached, positions get discarded.
      */
     public static final ConfigKey FORWARD_RETRY_ENABLE = new ConfigKey(
             "forward.retry.enable", Boolean.class);
 
     /**
-     * Position forwarding retry minimum delay in seconds.
-     * Can be set to anything greater than 0. Defaults to 1 second.
+     * Position forwarding retry first delay in milliseconds.
+     * Can be set to anything greater than 0. Defaults to 100 milliseconds.
      */
-    public static final ConfigKey FORWARD_RETRY_DELAY_MIN = new ConfigKey(
-            "forward.retry.delay.min", Integer.class);
+    public static final ConfigKey FORWARD_RETRY_DELAY = new ConfigKey(
+            "forward.retry.delay", Integer.class);
 
     /**
-     * Position forwarding retry maximum delay in seconds.
-     * Can be set to anything greater than 0. Defaults to 10 seconds.
+     * Position forwarding retry maximum retries.
+     * Can be set to anything greater than 0. Defaults to 10 retries.
      */
-    public static final ConfigKey FORWARD_RETRY_DELAY_MAX = new ConfigKey(
-            "forward.retry.delay.max", Integer.class);
+    public static final ConfigKey FORWARD_RETRY_COUNT = new ConfigKey(
+            "forward.retry.count", Integer.class);
 
     /**
-     * Position forwarding retry pending limit.
+     * Position forwarding retry pending positions limit.
      * Can be set to anything greater than 0. Defaults to 100 positions.
      */
-    public static final ConfigKey FORWARD_RETRY_PENDING_LIMIT = new ConfigKey(
-            "forward.retry.pending.limit", Integer.class);
+    public static final ConfigKey FORWARD_RETRY_LIMIT = new ConfigKey(
+            "forward.retry.limit", Integer.class);
 
     /**
      * Boolean flag to enable or disable position filtering.