Repository navigation
feat(events): persist events to disk so they survive the process (tier 3) - #408
abelonogov-ld wants to merge 279 commits into
Conversation
…-durability-tier2-bounded-flush
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # example/src/main/java/com/launchdarkly/example/MainActivity.java
Coming online now delivers, and the fixture brings the processor online as it builds it, so the queued delivery could drain the store before the test read it. The commit these cover runs offline just the same. Co-authored-by: Cursor <cursoragent@cursor.com>
java-sdk-internal's DefaultEventProcessor, which this path replaced, skipped the summary counter for an event marked excludeFromSummaries. OutboundEventBuffer.summarize counted unconditionally, so that flag was the one thing the replacement did not carry over; addFullEvent already honoured the sampling ratio, which is the sibling flag on the same object. Nothing observable changes today. DirectEventProcessor builds its feature events through the constructor overload that fixes both samplingRatio at 1 and excludeFromSummaries at false, so neither guard can fire through the SDK's own recording path. The point is that the two are defaulted by the same overload and would go live together: anything that moves to the fourteen-argument constructor, which adopting sampling requires, arms both at once. The failure mode is silent and arrives as wrong analytics rather than as an error, since an event carries its sampling ratio on the wire for the receiver to scale by. The guard goes in the buffer rather than in the processor because the buffer is where an event arrives from outside. A check in the processor would sit three lines below the constructor call that hardcodes the value, could never fire, and would read as dead. In the buffer it is reachable, and OutboundEventBufferFlagsTest now covers both flags -- including sampling, which had no coverage at all. Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush
…y/event-durability-tier3-persistence
…/sdk/android/DirectEventProcessor.java Co-authored-by: Todd Anderson <127344469+tanderson-ld@users.noreply.github.com>
Recording an event and encoding a batch of them were sharing one monitor. Encoding is the dominant cost on the path and runs on the events executor; recording runs on whichever thread evaluated a flag, which on Android is usually the main one. An evaluation could therefore wait out a whole batch encode before it was allowed to add a counter. The list of full events, the capacity counted against it and the drop count move to DirectEventProcessor. A delivery takes the run under pendingLock and encodes it after releasing, so a recording thread waits only for another thread's memory operation. OutboundEventBuffer keeps the counters and the wire format, and takes the summaries under its own lock for the same reason. A run that cannot be serialized is now discarded rather than put back. Nothing the encoder can fail on is transient -- it writes to a byte array -- so a restored run would have failed again on every later flush and no events would ever have been delivered again. EventProcessorTestBase now waits out the delivery that coming online schedules. It was previously free to land in the middle of a test's recording loop and take a batch the test had not asked to send, which at capacity reads as the limit having been exceeded. Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DirectEventProcessor.java
Nothing in it logs any more. The capacity warning went to the processor with the capacity check, and a failed encode is thrown to the caller rather than reported here. Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java # launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DirectEventProcessor.java # launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/OutboundEventBuffer.java
Moving these back to where tier 2 has them leaves the capacity fields and their assignment as unchanged lines, so this branch's diff shows the commit machinery it adds rather than a reshuffle of what it inherited. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # example/src/main/java/com/launchdarkly/example/MainActivity.java # example/src/main/res/layout/activity_main.xml
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # example/src/main/java/com/launchdarkly/example/MainActivity.java # example/src/main/res/layout/activity_main.xml
Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush
…y/event-durability-tier3-persistence
…-durability-tier2-bounded-flush * andrey/event-durability-tier1-buffer: unserializable unit test
…y/event-durability-tier3-persistence * andrey/event-durability-tier2-bounded-flush: unserializable unit test
…-durability-tier2-bounded-flush Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DirectEventProcessorTest.java
…y/event-durability-tier3-persistence Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java # launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/DirectEventProcessor.java # launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/OutboundEventBuffer.java
Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush
It reports its progress in that text, and the Apple test app puts it in the same place. Co-authored-by: Cursor <cursoragent@cursor.com>
…the network Flushes asked for while a retry was queued took that retry as theirs. When its own post began a wait, as the second 400 that drops a batch does for the ones behind it, the retry stopped at that wait and queued nothing, so what those flushes committed stayed put until a flush from outside, often the periodic one. A batch the backoff keeps without an attempt is now HELD rather than RETRYABLE, and a retry that ends on one queues itself again for when the wait is up. A retry ending on a post that failed still queues nothing. Co-authored-by: Cursor <cursoragent@cursor.com>
The periodic flush went straight to delivery, which returns at the offline gate before it commits. A requested flush commits first for exactly that case; the periodic one did not, so evaluations short of a full pending run, with no track or identify among them, stayed in memory for as long as an outage lasted, and a process killed meanwhile lost them. It now goes through the same commit-then-deliver path, as do the deliveries that going online and startup start for a previous run's batches. The commit is still only where events persist. Co-authored-by: Cursor <cursoragent@cursor.com>
close()'s last delivery went through the same remainingWaitNanos() gate as every other, and the retry it queued was then canceled by shutdown. After a refusal, a wait of minutes to an hour, every close() inside it posted nothing; with persistence off, the default, those events went with the process, uncounted and unlogged. The final delivery now posts whatever wait the backoff is in, which shutting down cuts short. Only for its own length: its retry, and anything after it, are gated as before, and offline still keeps it off the network. Co-authored-by: Cursor <cursoragent@cursor.com>
…te names as references (#415) **Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [ ] I have validated my changes against all supported platform versions **Related issues** Stacked on `andrey/event-durability-tier1-buffer`, which moved java-sdk-internal to 1.12.1. Picks up [java-core#228](launchdarkly/java-core#228), released in java-sdk-internal 1.12.2. The iOS counterpart is [ios-client-sdk#544](launchdarkly/ios-client-sdk#544). **Describe the solution you've provided** `_meta.redactedAttributes` is a list of attribute references. When every attribute is private (`allAttributesPrivate`, or an anonymous context in a feature event), java-sdk-internal 1.12.1 listed each attribute's raw name. A name that starts with a slash, such as `/ssn`, reads back as the path to an attribute named `ssn`, so the list named an attribute other than the one that was removed. 1.12.2 lists `AttributeRef.fromLiteral(name).toString()` instead: `/~1ssn` for that name, and the name itself for any name without a leading slash. The Go and JavaScript SDKs write the same form. 1.12.2 contains only that fix. Both `launchdarkly-android-client-sdk` and `shared-test-code` move to it. `EventProcessorPrivacyTest.allAttributesPrivateReportsEachNameAsTheReferenceThatNamesIt` checks the posted payload for attributes named `/ssn`, `/a~b` and `c/d`. It expects `/~1ssn`, `/~1a~0b` and `c/d`, and fails on 1.12.1 with `[/a~b, /ssn, c/d]`. **Describe alternatives you've considered** None. The redaction is java-sdk-internal's, which the event processor reuses through `OutboundEventBuffer`. **Additional context** Event output only, and only for attribute names that start with a slash. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Bumps **`launchdarkly-java-sdk-internal`** from **1.12.1** to **1.12.2** in `launchdarkly-android-client-sdk` and `shared-test-code`, picking up the fix where **`_meta.redactedAttributes`** uses proper **attribute references** when every attribute is private (`allAttributesPrivate`). > > With **1.12.1**, literal names starting with `/` (e.g. `/ssn`) were listed as raw strings, which could be misread as paths to different attributes; **1.12.2** reports the encoded refs (e.g. `/~1ssn`, `/~1a~0b`) while ordinary names like `c/d` stay unchanged. > > Adds **`EventProcessorPrivacyTest.allAttributesPrivateReportsEachNameAsTheReferenceThatNamesIt`** to assert the posted identify payload matches that behavior on the wire. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1737441. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Cursor <cursoragent@cursor.com>
A log a previous run left was deleted as empty whenever no frame of it read back, which is also what a first frame failing its checksum, or a format this version does not read, looks like: a log full of events went with nothing logged or counted. A log that read back partway was recovered up to the damage, and what came after it went silently too. Recovery now tells the two apart. A log holding nothing is still deleted quietly. A damaged one, or one in another format, is deleted, logged at error and counted lost, and the frames before damage in a partly readable one are still recovered, with the rest counted lost too. Nothing in a damaged log says how many events the damage took, so it counts one, the fewest it can have been. The processor counts that as it does a damaged batch, so the next flush a caller waits on hears of it. Co-authored-by: Cursor <cursoragent@cursor.com>
… the test app Posts a 20 MB payload of valid custom events straight to the events service on a connection of its own, with the headers the SDK's sender uses, and shows the status, headers and body that come back. Neither the event store, the batch size limit nor the retry state is involved, so the answer to a body that size is seen directly. The body is generated as it is written, a chunk at a time: on Android 6 the platform connection buffered a 20 MB write whole, which with the array behind it exhausted a 48 MB heap. Co-authored-by: Cursor <cursoragent@cursor.com>
A 413 was unexpected: the batch was kept under the extended backoff and, being the oldest, held back every batch behind it until it passed the maximum age. The events service answers 413 to a body over its limit, which posting it again never changes, and 415 and 422 are refusals of the payload in the same way. All four now count together toward the drop that 400 had: two in a row for the same batch, in any mix, remove it and count its events as lost, and they wait the normal backoff a 400 does rather than the extended one. LDUtil's classification, which the data sources share, is unchanged; the events-only check is LDUtil.isEventPayloadRefused. Co-authored-by: Cursor <cursoragent@cursor.com>
EventDirectory says it decides what the files in the directory are called, but the log's two names, pending- and new-, were DiskEventLog's, which also built them. They are EventDirectory's now, beside ready-, and DiskEventLog takes a new log's name, and the one it is opened under, from there. Co-authored-by: Cursor <cursoragent@cursor.com>
// MARK: is Xcode's convention, carried over from the Swift side. The Java sources mark sections with // ---- title ----, so these do too. Co-authored-by: Cursor <cursoragent@cursor.com>
A file past twice the batch size cannot be this version's, and read anyway it is held in memory whole. One large enough failed expiry, which runs first in every delivery, on every pass, without ever being deleted. Such a file is now unreadable before anything is allocated: a batch is dropped and counted lost by delivery, a log from a previous run is deleted and counted lost by recovery, and expiry discards it as one event. Memory short at the allocation is a read that may pass later. Expiry and recovery leave a file that throws for a later pass rather than ending the pass. Co-authored-by: Cursor <cursoragent@cursor.com>
Batches are named ready-v1-<payloadId>.<count>. Nothing is taken from a ready- name of another version: it is counted by reading the file, whose header decides whether this version can deliver it, and given a payload ID made from its name, the same for every attempt from every process. A later version can then change its names however it likes, and an earlier one pays a read for it rather than taking some other number for the count. Co-authored-by: Cursor <cursoragent@cursor.com>
A listing that reads a batch named without a count took its size from a stat of its own afterwards, which answers 0 where it cannot look. With no earlier size to fall back on, a batch of up to 2 MiB was then counted as a header's six bytes against the byte capacity. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This is the first version, so a ready- batch not named v1- can only be a later version's, after a downgrade. Reading it is that version's to do: it is no longer listed, counted against capacity or delivered under a payload ID made up for it. Expiry still deletes it once it is past the maximum age, without reading or counting it, so it cannot stay on the device for good. Co-authored-by: Cursor <cursoragent@cursor.com>
…-durability-tier2-bounded-flush
…y/event-durability-tier3-persistence
| * process, its events lost without being counted. | ||
| */ | ||
| private static byte[] readAll(FileInputStream in) throws IOException { | ||
| byte[] contents = new byte[(int) Math.min(in.getChannel().size(), Integer.MAX_VALUE - 8)]; |
There was a problem hiding this comment.
Do we still need this Math.min check if we have in.getChannel().size?
Semi related, where do we impose a maximum size we are willing to read that matches the 2MB max file size we're expecting these to be?
There was a problem hiding this comment.
try (SeekableByteChannel sbc = Files.newByteChannel(path);
InputStream in = Channels.newInputStream(sbc)) {
long size = sbc.size();
if (size > (long)MAX_BUFFER_SIZE)
throw new OutOfMemoryError("Required array size too large");
return read(in, (int)size);
}```
The test listed in a tight loop with a yield between listings. A monitor is not fair, and a yield returns at once when the closing thread is blocked on it rather than runnable, so the lister took the lock straight back. On a busy CI runner the 150 closes ran past the 30 second timeout. A 200 microsecond pause lets each close in while a listing still overlaps it. Co-authored-by: Cursor <cursoragent@cursor.com>
ConnectivityManager reports, to an event processor that listens, the start of each data source's first flag request and how it ended: loaded, failed, or stopped before it answered. A poll a fresh cache put off makes no request at launch and reports nothing. Each source reports once. Groundwork for the launch gate and pacing (spec D16-D20). Co-authored-by: Cursor <cursoragent@cursor.com>
Where events persist: - What a previous run left waits for the launch gate: at least 2 s after first going online, and until the first flag request settles, there was none, or 10 s have passed. The diagnostic init event waits for it too. - Deliveries the processor starts on its own (the launch delivery and the timer) send one batch at a time, at least 1 s after the last answered, and hold while a flag request is in flight, for up to 10 s. - A flush the application asks for, going offline, and close are never held, and end the pacing of a delivery under way. - Recovery of a previous run's logs runs at the lowest thread priority. At DISABLED nothing changes. Spec D16-D20. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 33af63a. Configure here.
capacityBytes was each environment's, so an application configured for several environments could keep that many times the bytes it set. It is now for all of them together, each environment's store getting an equal share. The event capacity is still each environment's in full. Co-authored-by: Cursor <cursoragent@cursor.com>
…aunch cap is not waited twice Co-authored-by: Cursor <cursoragent@cursor.com>
…e it answers Co-authored-by: Cursor <cursoragent@cursor.com>
…post Co-authored-by: Cursor <cursoragent@cursor.com>
| flagRequestAnswered = answered; | ||
| if (requestsFlags && eventProcessor instanceof FlagRequestListener) { | ||
| ((FlagRequestListener) eventProcessor).flagRequestStarted(); | ||
| } |
There was a problem hiding this comment.
This seems hacky, and also a bit of a lie since it is saying a request was answered, but in this case no request was sent.
| private final LDContext context; | ||
| private final DataSourceUpdateSink dataSourceUpdateSink; | ||
| final long initialDelayMillis; // visible for testing | ||
| final long initialDelayMillis; // visible for testing, and to ConnectivityManager |
There was a problem hiding this comment.
This is a hack to use in real code.
| } | ||
|
|
||
| /** A data source started loading flags: at launch, at {@code identify}, or reconnecting. */ | ||
| void flagRequestStarted(); |
There was a problem hiding this comment.
I'm not sure this should be communicating the request/response information to other parts of the code if avoidable.
Why not just communicate that one component wants network layer priority? Then the component can hold network layer priority across multiple requests and we don't need to have the concept of loaded, failed, stopped permeating out of this API into other parts of the code.
| /** Whether the delivery running is one this processor started on its own, and so paced. */ | ||
| private boolean pacedPass; | ||
| private int pacedPostsInPass; | ||
| private long lastPacedPostEndedNanos; |
There was a problem hiding this comment.
This seems like way too much logic / pollution of concerns to be embedded in the direct event processor itself. This is approaching a QoS system and that is too much to be in this class.
|
|
||
| private final Object lock = new Object(); | ||
| // Guarded by lock. | ||
| private boolean inFlight; |
There was a problem hiding this comment.
We should ideally not reveal from one component needing network priority that it has requests in flight. It really should just be that it needs the network layer bandwidth and what it will do with that bandwidth is up to it. It may make a thousand requests, who knows, other portions of our code shouldn't need to know.

Requirements
Related issues
Tier 3 of the Event Durability spec, "Tier 3 — persistence". Stacked on #403 (tier 2) and targets that branch, so this diff shows only tier 3. It will be retargeted once the branches below it merge. The iOS counterpart is launchdarkly/ios-client-sdk#538, and both write the same on-disk format.
Opened as a draft: a multi-threading and readability review turned up items still being worked through (see Known open items).
Describe the solution you've provided
Recorded events go to an append-only log on disk rather than only an in-memory buffer, so an application that dies shortly after recording an event still reports it: the next run delivers what the previous one left behind. That covers what tier 2 cannot:
SIGKILL, ANR kills, native crashes, and the system reclaiming a backgrounded process.EventPersistence(new public enum) andEventProcessorBuilder.eventPersistence(...):DISABLED(default): events stay in memory and the SDK does not touch the filesystem for events.DEFERRED: writes happen on the SDK's commit thread, a moment after recording.IMMEDIATE:trackandidentifywrite on the caller's thread before returning, so there is no window.EventStore: an append-only log under the no-backup files directory, one directory per mobile key. It handles framing, capacity, torn-tail recovery of a log interrupted by a crash, and a fallback to memory when a write fails (for example, a full disk), so a disk failure never takes the application down. Each process appends to its ownopen-<process>log; closedready-<payloadId>batches are shared, so a batch left by a process that never runs again is still delivered. The byte format lives inEventLogFormatand the log this process appends to inDiskEventLog, mirroring the iOS split.DirectEventProcessor): an evaluation appends to a pending run and updates a counter, with no encoding on the caller's thread. A commit encodes the run and its summaries together and stages them in the store.trackandidentifyare commit points. Evaluations commit once a threshold accumulates, on flush, and before delivery.flushAsync()answersfalsewhen events the SDK accepted were lost for good: an event or summary that could not be serialized, or that was too large to store. Each loss is reported to exactly one caller, the one whose delivery closed its batch after the loss, as on iOS. Events dropped for capacity are counted in diagnostics, not reported as losses.close()commits first, then delivers with what is left of one two-second budget. Neither step waits on the caller's thread past the budget, so a stalled disk write cannot become an ANR. Work that outlasts the budget finishes in the background.AnalyticsEventSenderposts a pre-assembled body under an explicit payload ID.DEFAULT_CAPACITYgoes from 100 to 1000, matching iOS, since capacity now also bounds what is held on disk.Describe alternatives you've considered
SQLite. Measured as a store and rejected: it costs more per event than an append to a log the SDK already frames, for durability properties the log gets from
writealone. Seesqlite-event-persistence-findings.mdin the spec folder.Fsync on every commit. What durability is defending against is the process dying, not the device losing power. Bytes handed to the kernel survive the former, and an fsync per event would dominate the cost of
track.One shared log for all processes. That is simpler, but one process's startup recovery could rename a log another live process is still appending to. A log per process avoids that without file locks.
Additional context
Tests. 879 unit tests pass locally (
testDebugUnitTest). New coverage includesEventStoreMultiProcessTest(several stores on one directory, recovery, transient read failures, events still counted against capacity while being written),DirectEventProcessorTest(commit points, retry, capacity across memory and disk, persistence failing partway through, oversized frames reported as losses, a loss reported to the right flush across a retry,close()within budget on a stalled disk write),OutboundEventBufferSerializationTest, andSameContextPerContextEventSummarizerTest.Benchmarks.
EventPersistenceBenchmark(JVM and instrumented) measures recording cost per mode. It runs only whenLD_EVENT_BENCH=1, so ordinary test runs are unaffected.Test app. Now configures
EventPersistence.IMMEDIATE, so the events recorded by Eval+Kill now, which tier 2 loses, are recovered and delivered on the next launch. It also gains Over-Refresh Eval, a main-thread stress loop that compares evaluation cost while filling capacity and after it is full.Known open items (from the threading review, to be resolved before this leaves draft):
flushAsync(), and soflushAndWait, does not commit on the caller's thread asflush()does, so aflushAndWaitthat times out can leave events only in memory, even atIMMEDIATE.Version.
EventPersistenceandEventProcessorBuilder.eventPersistencehave no@sincetag yet; one should be added once the release version is known.Note
Overview
Tier 3 event durability replaces the in-memory
DefaultEventProcessorwrapper with a newDirectEventProcessorstack that writes to an on-diskEventStore(append-only logs undergetNoBackupFilesDir(), per mobile key), so events survive process death and are delivered on the next launch with a stable payload ID viaAnalyticsEventSender.Applications opt in through
EventPersistence(DISABLED/DEFERRED/IMMEDIATE) onEventProcessorBuilder.IMMEDIATEcommitstrack/identifyon the caller thread; delivery adds retry/backoff, launch gating and pacing that defer to in-flight flag loads (FlagRequestListenerwired fromConnectivityManager), andflushAsync/flushAndWaitsemantics (including multi-env parallel flush and crash-handler delivery tests).Supporting pieces include
BatchLocks/DiskEventLog, recovery of interrupted logs,DeliveryRetryState, default event capacity raised to 1000,launchdarkly-java-sdk-internal1.12.2, optionalLD_EVENT_BENCHdevice benchmarks, and the example app switching to blockingLDClient.init(..., timeout).Reviewed by Cursor Bugbot for commit 51aa35e. Bugbot is set up for automated code reviews on this repo. Configure here.