Skip to content

feat(events): persist events to disk so they survive the process (tier 3) - #408

Open
abelonogov-ld wants to merge 279 commits into
mainfrom
andrey/event-durability-tier3-persistence
Open

abelonogov-ld wants to merge 279 commits into
mainfrom
andrey/event-durability-tier3-persistence

Conversation

@abelonogov-ld

@abelonogov-ld abelonogov-ld commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

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) and EventProcessorBuilder.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: track and identify write 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 own open-<process> log; closed ready-<payloadId> batches are shared, so a batch left by a process that never runs again is still delivered. The byte format lives in EventLogFormat and the log this process appends to in DiskEventLog, mirroring the iOS split.
  • Recording and commits (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. track and identify are commit points. Evaluations commit once a threshold accumulates, on flush, and before delivery.
  • Delivery reads batches oldest first. A batch keeps its payload ID across retries and process restarts, so LaunchDarkly discards a repeated delivery instead of double counting. Delivery stops at the first retryable failure, schedules a single retry instead of sleeping on the events thread, and removes a batch only once it has been accepted or permanently refused. A batch whose read fails transiently is kept for a later attempt.
  • Loss reporting: flushAsync() answers false when 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.
  • AnalyticsEventSender posts a pre-assembled body under an explicit payload ID.
  • DEFAULT_CAPACITY goes 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 write alone. See sqlite-event-persistence-findings.md in 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 includes EventStoreMultiProcessTest (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, and SameContextPerContextEventSummarizerTest.

Benchmarks. EventPersistenceBenchmark (JVM and instrumented) measures recording cost per mode. It runs only when LD_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 so flushAndWait, does not commit on the caller's thread as flush() does, so a flushAndWait that times out can leave events only in memory, even at IMMEDIATE.
  • Batches left by a previous run are not counted against capacity until the first listing, which an offline start postpones.
  • The store and the processor both schedule commits, so a threshold commit is followed by a redundant no-op pass.

Version. EventPersistence and EventProcessorBuilder.eventPersistence have no @since tag yet; one should be added once the release version is known.


Note

Overview
Tier 3 event durability replaces the in-memory DefaultEventProcessor wrapper with a new DirectEventProcessor stack that writes to an on-disk EventStore (append-only logs under getNoBackupFilesDir(), per mobile key), so events survive process death and are delivered on the next launch with a stable payload ID via AnalyticsEventSender.

Applications opt in through EventPersistence (DISABLED / DEFERRED / IMMEDIATE) on EventProcessorBuilder. IMMEDIATE commits track/identify on the caller thread; delivery adds retry/backoff, launch gating and pacing that defer to in-flight flag loads (FlagRequestListener wired from ConnectivityManager), and flushAsync / flushAndWait semantics (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-internal 1.12.2, optional LD_EVENT_BENCH device benchmarks, and the example app switching to blocking LDClient.init(..., timeout).

Reviewed by Cursor Bugbot for commit 51aa35e. Bugbot is set up for automated code reviews on this repo. Configure here.

abelonogov-ld and others added 30 commits September 18, 2026 16:01
…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>
…/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>
…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

* 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>
abelonogov-ld and others added 17 commits October 27, 2026 09:50
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>
* 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)];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

        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>
abelonogov-ld and others added 2 commits October 27, 2026 18:40
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>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ 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.

abelonogov-ld and others added 5 commits October 27, 2026 20:45
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();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a hack to use in real code.

}

/** A data source started loading flags: at launch, at {@code identify}, or reconnecting. */
void flagRequestStarted();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants