Skip to content

Commit 6ed3309

Browse files
runningcodeclaude
andcommitted
perf(core): Coalesce scope writes on submit instead of a timer (JAVA-628)
Scope-persistence flushes were debounced 100ms behind a scheduled task. The debounce was unnecessary: the Sentry executor is single-threaded, so a submitted flush task already sits in the queue long enough for mutations arriving behind it to be folded into the same write. That is exactly the window that matters, since the queue is deepest during startup. Submit the flush instead of scheduling it. Coalescing now tracks executor load rather than a fixed delay, which closes the data-loss window the debounce introduced. Guard against the executor rejecting the task without throwing once its queue is full, which would otherwise leave the pending flag set and stop scope persistence for the rest of the process. Also record why resetCache() deliberately leaves pending mutations alone: they only ever hold values from the current process, so dropping them would lose scope state set during init rather than clearing the previous run's data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 73afe6e commit 6ed3309

3 files changed

Lines changed: 43 additions & 25 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
### Performance
1515

1616
- Batch and coalesce scope-persistence disk writes to reduce startup cost ([#5791](https://github.com/getsentry/sentry-java/pull/5791))
17-
- Scope mutations are now coalesced (latest value per field) and breadcrumbs are appended in batches behind a single fsync, instead of one synchronous disk write per mutation. Persisted scope data (used to enrich crash/ANR events on the next launch) may now lag real-time changes by up to ~100 ms.
17+
- Scope mutations are now coalesced (latest value per field) and breadcrumbs are appended in batches behind a single fsync, instead of one synchronous disk write per mutation.
1818
- Reduce the number of SDK threads: `LifecycleWatcher` now schedules the session-end task on the shared timer executor instead of creating a dedicated `java.util.Timer` thread ([#5819](https://github.com/getsentry/sentry-java/pull/5819))
1919

2020
## 8.50.1

sentry/src/main/java/io/sentry/cache/PersistingScopeObserver.java

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.util.Queue;
3535
import java.util.concurrent.ConcurrentHashMap;
3636
import java.util.concurrent.ConcurrentLinkedQueue;
37+
import java.util.concurrent.Future;
3738
import java.util.concurrent.atomic.AtomicBoolean;
3839
import org.jetbrains.annotations.NotNull;
3940
import org.jetbrains.annotations.Nullable;
@@ -43,15 +44,6 @@ public final class PersistingScopeObserver extends ScopeObserverAdapter {
4344

4445
private static final Charset UTF_8 = Charset.forName("UTF-8");
4546

46-
/**
47-
* How long scope mutations are coalesced before being flushed to disk. Rather than writing on
48-
* every mutation, we keep only the latest value per file (and buffer breadcrumbs) and flush them
49-
* together after this delay. This trades a small data-loss window (mutations from the last
50-
* ~{@value #FLUSH_AFTER_MS} ms before the process dies) for far fewer disk writes and fsyncs,
51-
* which is significant during startup.
52-
*/
53-
static final long FLUSH_AFTER_MS = 100;
54-
5547
/** Sentinel value marking a file that should be deleted rather than written on the next flush. */
5648
private static final Object DELETE_MARKER = new Object();
5749

@@ -136,7 +128,7 @@ public void toStream(Breadcrumb value, OutputStream sink) throws IOException {
136128
// Breadcrumbs buffered since the last flush, appended together behind a single fsync.
137129
private final @NotNull Queue<Breadcrumb> pendingBreadcrumbs = new ConcurrentLinkedQueue<>();
138130
private final @NotNull AtomicBoolean pendingBreadcrumbsClear = new AtomicBoolean(false);
139-
private final @NotNull AtomicBoolean hasScheduledFlush = new AtomicBoolean(false);
131+
private final @NotNull AtomicBoolean hasPendingFlush = new AtomicBoolean(false);
140132

141133
public PersistingScopeObserver(final @NotNull SentryOptions options) {
142134
this.options = options;
@@ -153,7 +145,7 @@ public void addBreadcrumb(@NotNull Breadcrumb crumb) {
153145
return;
154146
}
155147
pendingBreadcrumbs.offer(crumb);
156-
scheduleFlush();
148+
requestFlush();
157149
}
158150

159151
@Override
@@ -167,7 +159,7 @@ public void setBreadcrumbs(@NotNull Collection<Breadcrumb> breadcrumbs) {
167159
// drop breadcrumbs buffered before the clear; anything added after it is enqueued again
168160
pendingBreadcrumbs.clear();
169161
pendingBreadcrumbsClear.set(true);
170-
scheduleFlush();
162+
requestFlush();
171163
}
172164
}
173165

@@ -226,31 +218,42 @@ private void enqueue(final @NotNull String fileName, final @Nullable Object enti
226218
}
227219
// latest value wins; a null entity means the file should be deleted on the next flush
228220
pendingWrites.put(fileName, entity == null ? DELETE_MARKER : entity);
229-
scheduleFlush();
221+
requestFlush();
230222
}
231223

232-
@SuppressWarnings("FutureReturnValueIgnored")
233-
private void scheduleFlush() {
234-
if (!hasScheduledFlush.compareAndSet(false, true)) {
235-
// a flush is already scheduled; it will pick up the latest pending state
224+
/**
225+
* Queues a flush unless one is already queued. Coalescing comes from the flush task sitting in
226+
* the executor queue: every mutation that arrives before it runs is folded into the same write.
227+
* The executor is single-threaded, so during startup — when mutations are frequent and the queue
228+
* is deep — that window covers many mutations.
229+
*/
230+
private void requestFlush() {
231+
if (!hasPendingFlush.compareAndSet(false, true)) {
232+
// a flush is already queued; it will pick up the latest pending state
236233
return;
237234
}
238235
try {
239-
options.getExecutorService().schedule(this::flushOnExecutor, FLUSH_AFTER_MS);
236+
final @NotNull Future<?> future = options.getExecutorService().submit(this::flushOnExecutor);
237+
if (future.isCancelled()) {
238+
// the executor rejects tasks without throwing once its queue is full, so clear the flag or
239+
// no later mutation would ever be able to queue a flush again
240+
hasPendingFlush.set(false);
241+
}
240242
} catch (Throwable e) {
241-
hasScheduledFlush.set(false);
242-
options.getLogger().log(ERROR, "Scope persistence flush could not be scheduled", e);
243+
hasPendingFlush.set(false);
244+
options.getLogger().log(ERROR, "Scope persistence flush could not be submitted", e);
243245
}
244246
}
245247

246248
private void flushOnExecutor() {
247249
runSafely(this::flushPending);
248-
hasScheduledFlush.set(false);
249-
// reschedule if mutations arrived while we were flushing
250+
// clear the flag before re-checking, otherwise a mutation landing between the drain and the
251+
// clear would see a flush still queued and be left with nobody to write it
252+
hasPendingFlush.set(false);
250253
if (!pendingWrites.isEmpty()
251254
|| !pendingBreadcrumbs.isEmpty()
252255
|| pendingBreadcrumbsClear.get()) {
253-
scheduleFlush();
256+
requestFlush();
254257
}
255258
}
256259

@@ -353,6 +356,9 @@ public static <T> void store(
353356
* I/O and should be called from a background thread.
354357
*/
355358
public void resetCache() {
359+
// NOTE: pending mutations are deliberately left alone. They only ever hold values from the
360+
// current process, which is exactly what this reset is clearing the way for; dropping them
361+
// would lose scope state set during init.
356362
// since it keeps a reference to the file and we cannot delete it, breadcrumbs we just clear
357363
try {
358364
final @NotNull ObjectQueue<Breadcrumb> queue = breadcrumbsQueue.getValue();

sentry/src/test/java/io/sentry/cache/PersistingScopeObserverBatchingTest.kt

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class PersistingScopeObserverBatchingTest {
3030
read(options, BREADCRUMBS_FILENAME, List::class.java) as List<Breadcrumb>
3131

3232
@Test
33-
fun `defers writes until the scheduled flush runs`() {
33+
fun `defers writes until the flush runs`() {
3434
val executor = DeferredExecutorService()
3535
val sut = getSut(executor)
3636

@@ -80,6 +80,18 @@ class PersistingScopeObserverBatchingTest {
8080
assertThat(sut.readBreadcrumbs().map { it.message }).containsExactly("kept")
8181
}
8282

83+
@Test
84+
fun `resetCache keeps pending mutations from the current process`() {
85+
val executor = DeferredExecutorService()
86+
val sut = getSut(executor)
87+
88+
sut.setTransaction("SetDuringInit")
89+
sut.resetCache()
90+
executor.runAll()
91+
92+
assertThat(sut.readTransaction()).isEqualTo("SetDuringInit")
93+
}
94+
8395
@Test
8496
fun `flush writes pending state synchronously`() {
8597
val executor = DeferredExecutorService()

0 commit comments

Comments
 (0)