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 b96a2242..1dfa2043 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 @@ -88,10 +88,7 @@ synchronized long recordFailure(boolean unexpected, long retryAfterMillis) { } long waitMillis; if (retryAfterMillis >= 0) { - // Every instance hears the same value at the same moment, so it is jittered to keep them from - // posting together when it runs out. - long directed = Math.min(retryAfterMillis, MAX_SERVER_DIRECTED_WAIT_MILLIS); - waitMillis = directed - (long) (random.nextDouble() * (directed / 2.0)); + waitMillis = directedWaitMillis(retryAfterMillis); } else { // Not jittered: each instance's failures follow its own history, which already spreads them. int doublings = Math.min(attempts - 1, 40); @@ -122,14 +119,28 @@ synchronized void recordFailureDuringWait(boolean unexpected, long retryAfterMil if (retryAfterMillis < 0) { return; } - long directed = Math.min(retryAfterMillis, MAX_SERVER_DIRECTED_WAIT_MILLIS); - long waitMillis = directed - (long) (random.nextDouble() * (directed / 2.0)); - long asked = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(waitMillis); + long asked = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(directedWaitMillis(retryAfterMillis)); if (asked - nextAttemptNanos > 0) { nextAttemptNanos = asked; } } + /** + * The wait a {@code Retry-After} sets. Every instance hears the same value at the same moment, so it is + * jittered to keep them from posting together when it runs out: up, by as much as half, so it is never + * shorter than the service asked, but no further than the hour a server-directed wait is capped at. A + * wait asked for of an hour or more is jittered down from the hour instead, as nothing above it is left + * to spread over. + */ + private long directedWaitMillis(long retryAfterMillis) { + long cap = MAX_SERVER_DIRECTED_WAIT_MILLIS; + if (retryAfterMillis >= cap) { + return cap - (long) (random.nextDouble() * (cap / 2.0)); + } + long spread = Math.min(retryAfterMillis + retryAfterMillis / 2, cap) - retryAfterMillis; + return retryAfterMillis + (long) (random.nextDouble() * spread); + } + /** How long until a post is allowed, in nanoseconds; zero once it is. */ synchronized long remainingWaitNanos() { if (!waiting) { 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 6cf57611..966d8cf2 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 @@ -74,18 +74,32 @@ public void anAcceptedDeliveryPutsEverythingBack() { } @Test - public void aRetryAfterSetsTheWaitJitteredToNoLessThanHalfOfIt() { + public void aRetryAfterSetsTheWaitJitteredUpByNoMoreThanHalfOfIt() { for (int i = 0; i < 100; i++) { long wait = state.recordFailure(false, 10 * SECOND); - assertTrue(wait + "ms", wait >= 5 * SECOND && wait <= 10 * SECOND); + assertTrue(wait + "ms", wait >= 10 * SECOND && wait <= 15 * SECOND); } } @Test - public void aRetryAfterIsCappedAtAnHour() { - 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); + public void aRetryAfterIsJitteredUpToTheHourAtMost() { + long fortyFiveMinutes = TimeUnit.MINUTES.toMillis(45); + assertEquals(fortyFiveMinutes, withRandom(0).recordFailure(false, fortyFiveMinutes)); + assertEquals(TimeUnit.SECONDS.toMillis(3_150), withRandom(0.5).recordFailure(false, fortyFiveMinutes)); + assertTrue(withRandom(0.999_999).recordFailure(false, fortyFiveMinutes) <= TimeUnit.HOURS.toMillis(1)); + } + + @Test + public void aRetryAfterOfAnHourOrMoreIsJitteredDownFromTheHour() { + for (long retryAfter : new long[] { TimeUnit.HOURS.toMillis(1), TimeUnit.HOURS.toMillis(5) }) { + assertEquals(TimeUnit.HOURS.toMillis(1), withRandom(0).recordFailure(false, retryAfter)); + assertEquals(TimeUnit.MINUTES.toMillis(45), withRandom(0.5).recordFailure(false, retryAfter)); + } + } + + private static DeliveryRetryState withRandom(double value) { + return new DeliveryRetryState(SECOND, FLUSH_INTERVAL, 512 * SECOND, TimeUnit.HOURS.toMillis(1), + new FixedRandom(value)); } @Test 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 2978646e..5f21ef2b 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 @@ -3553,16 +3553,17 @@ public void aRefusedBatchIsKeptAndPostedAgainOnceTheLongerWaitIsOver() throws Ex @Test public void aRetryAfterSetsTheWaitBeforeTheNextPost() throws Exception { - // Four seconds, jittered down to no less than two, which is past the second a flush waits out. + // Three seconds, jittered up to no more than four and a half, which is past the second a flush + // waits out. try (HttpServer server = HttpServer.start(Handlers.sequential( - Handlers.all(Handlers.header("Retry-After", "4"), Handlers.status(503)), + Handlers.all(Handlers.header("Retry-After", "3"), Handlers.status(503)), Handlers.status(202)))) { EventProcessor eventProcessor = makeEventProcessor(server, DEFAULT_CAPACITY); try { eventProcessor.recordCustomEvent(CONTEXT, "an-event", LDValue.ofNull(), null); assertFalse(awaitFlush(eventProcessor, 10, TimeUnit.SECONDS)); server.getRecorder().requireRequest(1, TimeUnit.SECONDS); - server.getRecorder().requireNoRequests(1_800, TimeUnit.MILLISECONDS); + server.getRecorder().requireNoRequests(2_500, TimeUnit.MILLISECONDS); assertTrue(server.getRecorder().requireRequest(5, TimeUnit.SECONDS).getBody() .contains("\"key\":\"an-event\"")); } finally { diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventLogFormatTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventLogFormatTest.java index fa0444cd..b8ef5d87 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventLogFormatTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventLogFormatTest.java @@ -21,6 +21,36 @@ * The frame checksum: a frame the storage changed after it was written ends the walk, as a torn tail does. */ public class EventLogFormatTest { + /** The events of the page-cache persistence spec's worked example (Appendix A). */ + static final List WORKED_EXAMPLE_EVENTS = Arrays.asList( + "{\"kind\":\"custom\",\"key\":\"checkout-opened\"}".getBytes(StandardCharsets.UTF_8), + "{\"kind\":\"identify\",\"context\":{\"key\":\"u\"}}".getBytes(StandardCharsets.UTF_8)); + + /** + * The worked example's log, byte for byte: the file header, then a frame per event. The iOS SDK's + * {@code EventStoreSpec} holds the same bytes, so a drift on either side fails one suite or the other. + */ + static final byte[] WORKED_EXAMPLE = bytes( + "4C444556 0001" + + " 0001 00000029" + + " 7B226B696E64223A22637573746F6D222C226B6579223A22636865636B6F75742D6F70656E6564227D" + + " 194AD1F0" + + " 0001 00000029" + + " 7B226B696E64223A226964656E74696679222C22636F6E74657874223A7B226B6579223A2275227D7D" + + " C549D122"); + + static final byte[] WORKED_EXAMPLE_BODY = ("[{\"kind\":\"custom\",\"key\":\"checkout-opened\"}," + + "{\"kind\":\"identify\",\"context\":{\"key\":\"u\"}}]").getBytes(StandardCharsets.UTF_8); + + private static byte[] bytes(String hex) { + String digits = hex.replace(" ", ""); + byte[] bytes = new byte[digits.length() / 2]; + for (int i = 0; i < bytes.length; i++) { + bytes[i] = (byte) Integer.parseInt(digits.substring(2 * i, 2 * i + 2), 16); + } + return bytes; + } + private static byte[] event(String key) { return ("{\"kind\":\"custom\",\"key\":\"" + key + "\"}").getBytes(StandardCharsets.UTF_8); } @@ -66,6 +96,24 @@ public void writesTheFrameTheIosSdkWrites() { assertArrayEquals(expected.toByteArray(), frame.toByteArray()); } + @Test + public void writesTheWorkedExamplesLogByteForByte() { + assertArrayEquals(Arrays.copyOf(WORKED_EXAMPLE, EventLogFormat.FILE_HEADER_SIZE), + EventLogFormat.fileHeader()); + ByteArrayOutputStream frames = new ByteArrayOutputStream(); + for (byte[] event : WORKED_EXAMPLE_EVENTS) { + EventLogFormat.writeFrame(frames, event); + } + assertArrayEquals(WORKED_EXAMPLE, EventLogFormat.withFileHeader(frames.toByteArray())); + } + + @Test + public void readsTheWorkedExamplesLog() { + assertEquals(2, EventLogFormat.eventCount(WORKED_EXAMPLE)); + assertFalse(EventLogFormat.isDamaged(WORKED_EXAMPLE)); + assertArrayEquals(WORKED_EXAMPLE_BODY, EventLogFormat.assembleBody(WORKED_EXAMPLE)); + } + @Test public void readsBackEveryFrameItWrote() { byte[] log = log("event-0", "event-1", "event-2"); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventStoreTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventStoreTest.java index d9f5a235..06dc20c8 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventStoreTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/EventStoreTest.java @@ -221,6 +221,33 @@ public void everyEventBeforeAFrameTheWriterDidNotFinishIsRecovered() throws IOEx assertEquals(Arrays.asList("event-0", "event-1"), keysIn(nextRun.body(recovered.get(0)))); } + @Test + public void writesTheWorkedExamplesLogByteForByte() throws IOException { + EventStore store = store(); + for (byte[] event : EventLogFormatTest.WORKED_EXAMPLE_EVENTS) { + store.stage(event); + } + store.commit(); + store.close(); + + assertArrayEquals(EventLogFormatTest.WORKED_EXAMPLE, + Files.readAllBytes(onlyFileNamed(EventDirectory.LOG_PREFIX).toPath())); + } + + @Test + public void deliversTheWorkedExamplesLogAsTheIosSdkWritesIt() throws IOException { + assertTrue(directory().mkdirs() || directory().isDirectory()); + File log = new File(directory(), EventDirectory.LOG_PREFIX + UUID.randomUUID()); + Files.write(log.toPath(), EventLogFormatTest.WORKED_EXAMPLE); + + EventStore nextRun = store(); + assertEquals(0, nextRun.preparePayloadsFromInterruptedLogs()); + List recovered = nextRun.pendingBatches(); + assertEquals(1, recovered.size()); + assertEquals(2, recovered.get(0).eventCount); + assertArrayEquals(EventLogFormatTest.WORKED_EXAMPLE_BODY, nextRun.body(recovered.get(0))); + } + @Test public void aLogFromAPreviousRunWhoseFirstFrameIsDamagedIsDeletedAndCountedLost() throws IOException { EventStore store = store();