fix(scorch): purge obsolete segments under sustained load - #1
Merged
ajroetker merged 6 commits intoJul 24, 2026
Merged
Conversation
…er idles removeOldData() - the only path that deletes obsolete segment files and prunes old bolt snapshots - runs from just two call sites: the persister's idle-wait block and the merger-catch-up pause. An index under sustained mutation (e.g. a continuous enrichment backfill feeding batches) never reaches either: the persist loop takes 'continue OUTER' on every pass, so obsolete .zap files accumulate at churn rate, unbounded, and survive process restarts (observed in production: root.bolt referencing 3 snapshots while the store directory held 15,703 segment files / 28GB, growing 17GB/h). Add ForcedPurgeInterval (default 1m, same package-var style as NumSnapshotsToKeep; 0 disables): the persister loop head now fires a purger check and removeOldData pass at least that often, regardless of load. Cleanup becomes a scheduled duty instead of an idle-time courtesy. Same goroutine as the existing call sites - no new concurrency. Suite: index/scorch green except pre-existing TestIndexReader vector assertion failure, identical on the unpatched v2.5.8-antfly002 tag.
The nap loop blocks on persisterNotifier waiting for merger progress. If the merger is starved (observed: merge workers grinding a vellum FST merge of a 17k-tiny-segment pile for hours under a 1-vCPU container limit), that signal never comes - and since the napping persister is the only goroutine that runs removeOldData, nothing deletes obsolete files while the file count that keeps the nap alive only grows: a livelock. Observed in production twice in one day, including once at 4 vCPUs. Add a ForcedPurgeInterval ticker case to the nap select: cleanup no longer requires merger permission, and a successful sweep drops numFilesOnDisk below the nap threshold, exiting the loop on its own. Complements the loop-head forced pass (previous commit), which covers the sub-threshold idle-starvation regime. Suite: green except pre-existing TestIndexReader (identical on unpatched v2.5.8-antfly002).
…er sustained mutation Two guarantees: (1) compile fence - the test references ForcedPurgeInterval, so any merge that drops the forced-purge patch breaks the build rather than silently reverting GC to idle-only; (2) behavioral pin - unsafe batches from a writer goroutine keep the persister behind the root epoch (never idle) and the .zap census is sampled mid-churn: with the 20ms purge the observed max stays at the live set (17); with the interval disabled (pre-patch idle-only semantics) nothing prunes old bolt snapshots either, every persisted epoch pins its files, and the census reaches 868 within 2s (red-run verified). An earlier version asserted after a terminal sleep, which let the idle-wait purge run during quiescence - it passed even with the patch disabled. The assertion must happen while the writer is still running; slowdown only shrinks the census, so the green arm is timing-safe in CI.
The persister parks in three places. The loop head purges every iteration and the merger-catch-up nap got a purge ticker previously — but the introducer-wait select had neither, on the assumption that introductions always arrive. A merge plan whose later task fails after an earlier task succeeded aborts before any introduction, so under a persistent merge failure nothing ever wakes the persister: the aborted plans' unmarked outputs (one per replan, observed at ~9/s) accumulate with no sweeper. Give that select the same forced-purge ticker arm, bounding orphaned outputs to one ForcedPurgeInterval's worth.
The behavioral test no longer references ForcedPurgeInterval, so it compiles against a scorch without the knob and fails there by accumulation instead of by compile error (853 mid-churn .zap files vs the 150 bound on the pre-patch persister). The interval shortening moves behind a hook wired by forced_purge_fence_test.go, which keeps the direct symbol reference and pins the non-zero default.
A wrapper segment plugin lets the first file merge of a plan succeed and fails the rest, so a multi-task plan aborts before introduction with the first task's output orphaned on disk. With the persister parked in its introducer-wait, the orphan must disappear within a bounded number of forced-purge intervals and without any merge introduction. Fails on the pre-patch persister (orphan still on disk after 20 intervals).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root cause
Scorch cleanup was effectively an idle-time task. Under sustained mutation, the persister could remain at the loop-head continue path indefinitely. It could also park waiting for merger catch-up or for an introduction that never arrives after a later merge task fails. In each case, obsolete or aborted-plan segment files accumulated without a runnable sweeper.
ForcedPurgeInterval defaults to one minute and may be set to zero to disable the behavior. Cleanup remains on the existing persister goroutine; this adds no concurrent remover.
The three covered paths are:
Validation
Related incident: antflydb/antfly#381