Merge pull request #1253 from microsoft/trask/fix-backoff-retries

Fix backoff retries

Author: trask

core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ExponentialBackOffTimesPolicy.java

@@ -51,8 +51,7 @@ final class ExponentialBackOffTimesPolicy implements BackOffTimesPolicy {
             FIVE_SECONDS_IN_MILLIS,
             FOUR_MINUTES_IN_MILLIS,
             FIVE_SECONDS_IN_MILLIS,
-            SIX_MINUTES_IN_MILLIS,
-            FIVE_SECONDS_IN_MILLIS
+            SIX_MINUTES_IN_MILLIS
     };
 
     @Override

core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/SenderThreadLocalBackOffData.java

@@ -21,7 +21,6 @@
 
 package com.microsoft.applicationinsights.internal.channel.common;
 
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Condition;
 import java.util.concurrent.locks.ReentrantLock;
 
@@ -78,50 +77,6 @@ public void onDoneSending() {
         currentBackOffIndex = -1;
     }
 
-    /**
-     * The calling thread will be suspended for an amount of time that is
-     * set in the 'backOffTimeoutsInSeconds' array by using its index 'currentBackOffIndex'.
-     *
-     * @return True if the thread completed the suspension time as expected, in which case
-     *         the caller should re-try to send the Transmission.
-     *         False, in which case the caller should 'abandon' this Transmission and move to the next one,
-     *         if:
-     *         1. The instance was marked as non-active before the thread started to wait.
-     *         2. The thread was signaled to stop while was waiting.
-     *         3. The thread was interrupted while was waiting.
-     *         4. The thread has exhausted all the back-off timeouts
-     */
-    public boolean backOff() {
-        try {
-            lock.lock();
-            ++currentBackOffIndex;
-            if (currentBackOffIndex == backOffTimeoutsInMillis.length) {
-                currentBackOffIndex = -1;
-
-                // Exhausted the back-offs
-                return false;
-            }
-
-            if (!instanceIsActive) {
-               return false;
-           }
-
-            try {
-                long millisecondsToWait = backOffTimeoutsInMillis[currentBackOffIndex];
-                if (millisecondsToWait > BackOffTimesPolicy.MIN_TIME_TO_BACK_OFF_IN_MILLS) {
-                    millisecondsToWait += addMilliseconds;
-                }
-                backOffCondition.await(millisecondsToWait, TimeUnit.MILLISECONDS);
-                return instanceIsActive;
-           } catch (InterruptedException e) {
-                Thread.currentThread().interrupt();
-                return false;
-           }
-       } finally {
-           lock.unlock();
-       }
-   }
-
     /**
      * Increment the current back off amount or resets the counter if needed.
      * <p>
@@ -132,13 +87,8 @@ public boolean backOff() {
     public long backOffTimerValue() {
         try {
             lock.lock();
-            ++currentBackOffIndex;
-            if (currentBackOffIndex == backOffTimeoutsInMillis.length) {
-                currentBackOffIndex = -1;
-
-                // Exhausted the back-offs
-                return -1;
-            }
+            // when the last backoff index is hit, stay there until backoff is reset
+            currentBackOffIndex = Math.min(currentBackOffIndex + 1, backOffTimeoutsInMillis.length - 1);
 
             if (!instanceIsActive) {
                return 0;

core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/SenderThreadsBackOffManager.java

@@ -69,11 +69,6 @@ public long backOffCurrentSenderThreadValue() {
         return currentThreadData.backOffTimerValue();
     }
 
-    public boolean backOffCurrentSenderThread() {
-        SenderThreadLocalBackOffData currentThreadData = this.get();
-        return currentThreadData.backOff();
-    }
-
     public synchronized void stopAllSendersBackOffActivities() {
         for (SenderThreadLocalBackOffData sender : allSendersData) {
             sender.stop();

core/src/test/java/com/microsoft/applicationinsights/internal/channel/common/ExponentialBackOffTimesPolicyTest.java

@@ -35,7 +35,7 @@ public final class ExponentialBackOffTimesPolicyTest {
     public void testBackOffs() {
         long[] backOffs = new ExponentialBackOffTimesPolicy().getBackOffTimeoutsInMillis();
         assertNotNull(backOffs);
-        assertTrue(backOffs.length % 2 == 1);
+        assertTrue(backOffs.length % 2 == 0);
         int couples = backOffs.length / 2;
         long lastEventValue = BackOffTimesPolicy.MIN_TIME_TO_BACK_OFF_IN_MILLS;
         for (int i = 0; i < couples; ++i) {

core/src/test/java/com/microsoft/applicationinsights/internal/channel/common/SenderThreadLocalDataTest.java

@@ -48,58 +48,17 @@ public void testStateAfterCtor() {
         assertFalse(sender.isTryingToSend());
     }
 
-    @Test
-    public void testMultipleOnFailedSending() {
-        final SenderThreadLocalBackOffData sender = createSenderThreadLocalData(new long[] {1000,2000,1000});
-        verifyBackOff(sender, 3, 4);
-    }
-
     @Test
     public void testOnDoneSending() {
         final SenderThreadLocalBackOffData sender = createSenderThreadLocalData(new long[] {1000});
         verifyOnDoneSending(sender);
     }
 
-    @Test
-    public void testDoneSendingAfterFailedSending() {
-        final SenderThreadLocalBackOffData sender = createSenderThreadLocalData(new long[]{1000});
-        verifyBackOff(sender, 1, 1);
-        verifyOnDoneSending(sender);
-    }
-
-    @Test
-    public void testStopWhileWaiting() throws InterruptedException {
-        final SenderThreadLocalBackOffData sender = createSenderThreadLocalData(new long[]{10000});
-        Thread thread = new Thread(new Runnable() {
-            @Override
-            public void run() {
-                sender.stop();
-            }
-        });
-        thread.setDaemon(true);
-        thread.start();
-        sender.backOff();
-        verifyOnDoneSending(sender);
-        thread.join();
-    }
-
     private SenderThreadLocalBackOffData createSenderThreadLocalData(long[] backOffs) {
         SenderThreadLocalBackOffData sender = new SenderThreadLocalBackOffData(backOffs, 0);
         return sender;
     }
 
-    private static void verifyBackOff(SenderThreadLocalBackOffData sender, int backOffTimes, int expectedSeconds) {
-        long started = System.nanoTime();
-        for (int i = 0; i < backOffTimes; ++i) {
-            sender.backOff();
-        }
-
-        int elapsed = (int)((double)(System.nanoTime() - started) / 1000000000.0);
-        assertTrue(String.format("BackOff lasted %d which is less than expected %d", elapsed, expectedSeconds), elapsed >= expectedSeconds);
-        assertTrue(String.format("BackOff lasted %d which is more than expected %d", elapsed, expectedSeconds), elapsed <= expectedSeconds + 2);
-        assertTrue(sender.isTryingToSend());
-    }
-
     private static void verifyOnDoneSending(SenderThreadLocalBackOffData sender) {
         sender.onDoneSending();