diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java index 82b8de8d..89a14144 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java @@ -108,12 +108,15 @@ static final class EventProcessorBuilderImpl extends EventProcessorBuilder // normal ceiling means the specified delays. private long retryInitialDelayMillis; private long retryNormalCeilingMillis = -1; + private long retryExtendedInitialDelayMillis; private long retryExtendedCeilingMillis; EventProcessorBuilderImpl retryDelaysForTesting(long initialDelayMillis, long normalCeilingMillis, + long extendedInitialDelayMillis, long extendedCeilingMillis) { retryInitialDelayMillis = initialDelayMillis; retryNormalCeilingMillis = normalCeilingMillis; + retryExtendedInitialDelayMillis = extendedInitialDelayMillis; retryExtendedCeilingMillis = extendedCeilingMillis; return this; } @@ -131,7 +134,7 @@ private DeliveryRetryState makeRetryState() { return new DeliveryRetryState(flushIntervalMillis); } return new DeliveryRetryState(retryInitialDelayMillis, retryNormalCeilingMillis, - retryExtendedCeilingMillis, new Random()); + retryExtendedInitialDelayMillis, retryExtendedCeilingMillis, new Random()); } @Override diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DeliveryRetryState.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DeliveryRetryState.java index fdb31706..b96a2242 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DeliveryRetryState.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DeliveryRetryState.java @@ -24,20 +24,18 @@ final class DeliveryRetryState { /** The ceiling once a refusal has been seen, until a delivery is accepted. */ static final long EXTENDED_CEILING_MILLIS = TimeUnit.HOURS.toMillis(1); - /** - * Where the attempt count goes at least to on a refusal, so its first wait is 2^9 initial delays, about - * eight and a half minutes, rather than starting from a second. - */ - static final int ATTEMPTS_AFTER_UNEXPECTED = 10; + /** The first wait after a refusal: 2^9 initial delays, about eight and a half minutes. */ + static final long EXTENDED_INITIAL_DELAY_MILLIS = TimeUnit.SECONDS.toMillis(512); private final long initialDelayMillis; private final long normalCeilingMillis; + private final long extendedInitialDelayMillis; private final long extendedCeilingMillis; private final Random random; // Guarded by this. private int attempts; - private long maxDelayMillis; + private boolean inExtendedRegime; private boolean waiting; private long nextAttemptNanos; @@ -47,16 +45,18 @@ final class DeliveryRetryState { */ DeliveryRetryState(long flushIntervalMillis) { this(DirectEventProcessor.RETRY_DELAY_MILLIS, Math.max(flushIntervalMillis, - DirectEventProcessor.RETRY_DELAY_MILLIS), EXTENDED_CEILING_MILLIS, new Random()); + DirectEventProcessor.RETRY_DELAY_MILLIS), EXTENDED_INITIAL_DELAY_MILLIS, + EXTENDED_CEILING_MILLIS, new Random()); } - DeliveryRetryState(long initialDelayMillis, long normalCeilingMillis, long extendedCeilingMillis, - Random random) { + /** The extended delays are never below the initial delay (RETRY 1.5.4.1). */ + DeliveryRetryState(long initialDelayMillis, long normalCeilingMillis, long extendedInitialDelayMillis, + long extendedCeilingMillis, Random random) { this.initialDelayMillis = initialDelayMillis; this.normalCeilingMillis = normalCeilingMillis; - this.extendedCeilingMillis = extendedCeilingMillis; + this.extendedInitialDelayMillis = Math.max(extendedInitialDelayMillis, initialDelayMillis); + this.extendedCeilingMillis = Math.max(extendedCeilingMillis, initialDelayMillis); this.random = random; - this.maxDelayMillis = normalCeilingMillis; } /** The wait after a first failure, which a flush waiting on the delivery that failed sits out. */ @@ -67,7 +67,7 @@ long initialDelayMillis() { /** An accepted delivery, which is all it takes for the next failure to start from a second again. */ synchronized void recordSuccess() { attempts = 0; - maxDelayMillis = normalCeilingMillis; + inExtendedRegime = false; waiting = false; } @@ -79,9 +79,10 @@ synchronized void recordSuccess() { * @return how long nothing may be posted */ synchronized long recordFailure(boolean unexpected, long retryAfterMillis) { - if (unexpected) { - attempts = Math.max(attempts + 1, ATTEMPTS_AFTER_UNEXPECTED); - maxDelayMillis = extendedCeilingMillis; + if (unexpected && !inExtendedRegime) { + // The first wait of the extended regime is its initial delay, however many failures came before. + inExtendedRegime = true; + attempts = 1; } else { attempts++; } @@ -94,7 +95,9 @@ synchronized long recordFailure(boolean unexpected, long retryAfterMillis) { } else { // Not jittered: each instance's failures follow its own history, which already spreads them. int doublings = Math.min(attempts - 1, 40); - waitMillis = Math.min(maxDelayMillis, initialDelayMillis << doublings); + long baseMillis = inExtendedRegime ? extendedInitialDelayMillis : initialDelayMillis; + long ceilingMillis = inExtendedRegime ? extendedCeilingMillis : normalCeilingMillis; + waitMillis = Math.min(ceilingMillis, baseMillis << doublings); } waiting = true; nextAttemptNanos = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(waitMillis); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DeliveryRetryStateTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DeliveryRetryStateTest.java index 33a838f5..6cf57611 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DeliveryRetryStateTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DeliveryRetryStateTest.java @@ -42,6 +42,22 @@ public void aRefusalWaitsAboutEightAndAHalfMinutesAndDoublesToAnHour() { assertEquals(TimeUnit.HOURS.toMillis(1), state.recordFailure(true, -1)); } + @Test + public void aRefusalAfterOrdinaryFailuresStartsFromTheExtendedInitialDelay() { + for (int i = 0; i < 12; i++) { + state.recordFailure(false, -1); + } + assertEquals(512 * SECOND, state.recordFailure(true, -1)); + } + + @Test + public void theExtendedDelaysAreNeverBelowTheInitialDelay() { + DeliveryRetryState inverted = new DeliveryRetryState(2 * SECOND, FLUSH_INTERVAL, SECOND, SECOND, + new Random()); + assertEquals(2 * SECOND, inverted.recordFailure(true, -1)); + assertEquals(2 * SECOND, inverted.recordFailure(true, -1)); + } + @Test public void anOrdinaryFailureAfterARefusalKeepsTheLongerCeiling() { state.recordFailure(true, -1); @@ -67,8 +83,8 @@ public void aRetryAfterSetsTheWaitJitteredToNoLessThanHalfOfIt() { @Test public void aRetryAfterIsCappedAtAnHour() { - long wait = new DeliveryRetryState(SECOND, FLUSH_INTERVAL, TimeUnit.HOURS.toMillis(1), new FixedRandom(0)) - .recordFailure(false, TimeUnit.HOURS.toMillis(5)); + long wait = new DeliveryRetryState(SECOND, FLUSH_INTERVAL, 512 * SECOND, TimeUnit.HOURS.toMillis(1), + new FixedRandom(0)).recordFailure(false, TimeUnit.HOURS.toMillis(5)); assertEquals(TimeUnit.HOURS.toMillis(1), wait); } @@ -94,8 +110,8 @@ public void aFailureDuringTheWaitLeavesTheBackoffWhereItWas() { @Test public void aRetryAfterDuringTheWaitCanOnlyLengthenIt() { - DeliveryRetryState fixed = new DeliveryRetryState(SECOND, FLUSH_INTERVAL, TimeUnit.HOURS.toMillis(1), - new FixedRandom(0)); + DeliveryRetryState fixed = new DeliveryRetryState(SECOND, FLUSH_INTERVAL, 512 * SECOND, + TimeUnit.HOURS.toMillis(1), new FixedRandom(0)); fixed.recordFailure(true, -1); long waiting = fixed.remainingWaitNanos(); @@ -108,7 +124,7 @@ public void aRetryAfterDuringTheWaitCanOnlyLengthenIt() { @Test public void aFailureDuringAWaitThatHasRunOutIsAnOrdinaryOne() throws InterruptedException { - DeliveryRetryState quick = new DeliveryRetryState(20, 1_000, 1_000, new Random()); + DeliveryRetryState quick = new DeliveryRetryState(20, 1_000, 1_000, 1_000, new Random()); quick.recordFailure(false, -1); Thread.sleep(40); @@ -119,7 +135,7 @@ public void aFailureDuringAWaitThatHasRunOutIsAnOrdinaryOne() throws Interrupted @Test public void theWaitRunsOut() throws InterruptedException { - DeliveryRetryState quick = new DeliveryRetryState(20, 20, 20, new Random()); + DeliveryRetryState quick = new DeliveryRetryState(20, 20, 20, 20, new Random()); quick.recordFailure(false, -1); assertTrue(quick.remainingWaitNanos() > 0); Thread.sleep(40); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DirectEventProcessorTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DirectEventProcessorTest.java index 0489e132..2978646e 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DirectEventProcessorTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DirectEventProcessorTest.java @@ -3520,7 +3520,8 @@ public void aRefusedBatchIsKeptAndPostedAgainOnceTheLongerWaitIsOver() throws Ex try (HttpServer server = HttpServer.start(service)) { EventProcessor eventProcessor = makeEventProcessor(server, ((ComponentsImpl.EventProcessorBuilderImpl) eventsBuilder(DEFAULT_CAPACITY)) - .retryDelaysForTesting(second, second, refusalWaitMillis), true); + .retryDelaysForTesting(second, second, refusalWaitMillis, + refusalWaitMillis), true); try { eventProcessor.recordCustomEvent(CONTEXT, "refused", LDValue.ofNull(), null); assertFalse(awaitFlush(eventProcessor, 10, TimeUnit.SECONDS)); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventProcessorTestBase.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventProcessorTestBase.java index cc74775c..16d4e9f8 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventProcessorTestBase.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventProcessorTestBase.java @@ -92,7 +92,7 @@ protected EventProcessorBuilder eventsBuilder(int capacity) { */ protected EventProcessorBuilder eventsBuilderWithoutBackoff(int capacity) { return ((ComponentsImpl.EventProcessorBuilderImpl) eventsBuilder(capacity)) - .retryDelaysForTesting(0, 0, 0); + .retryDelaysForTesting(0, 0, 0, 0); } /** diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventProcessorWithoutPersistenceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventProcessorWithoutPersistenceTest.java index a9ceabc3..5a20fd22 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventProcessorWithoutPersistenceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventProcessorWithoutPersistenceTest.java @@ -140,7 +140,7 @@ public File getNoBackupFilesDir() { try (HttpServer server = HttpServer.start(service)) { EventProcessor eventProcessor = buildOfflineEventProcessor(server, ((ComponentsImpl.EventProcessorBuilderImpl) eventsBuilder(100)) - .retryDelaysForTesting(8, 8, TimeUnit.HOURS.toMillis(1)), + .retryDelaysForTesting(8, 8, 4_096, TimeUnit.HOURS.toMillis(1)), true, platformState); eventProcessor.setOffline(false); try {