fix(cache): close cache-maintenance races behind top-of-feed loss - #336
Merged
Conversation
Three fixes from the 2026-07-23 cache-race audit (prod symptom: transient
top-of-feed loss on cached bucket + reactionCount entries, self-healing at
bucket_entry_ttl).
1. Stale-min remove race (confirmed bug). Async cache maintenance captures
min_tracked_value at collect and evaluates removes unlocked; a concurrent
pagination expand() lowers the floor and appends members before apply, so
apply dropped the freshly-added legitimate members. Removes are now tagged by
origin: filter-mismatch removes (slot no longer matches the filter — from
evaluate_filter_work, and from evaluate_sort_work's qualifies-but-unmatched
branch) go in `removes` and are ALWAYS applied; sort-bound removes (value fell
below the captured floor) go in `sort_bound_removes` and are the only ones
subject to the guard. A sort-bound candidate was, by construction, non-
qualifying under the captured floor, so the guard reduces to a single live-
bound test: apply the remove only when the value still does not qualify under
the current floor (a concurrent expand that lowered the floor to include it
means keep it). Origin tagging is required: a filter-mismatch remove can carry
a value that qualifies under the live floor, so an origin-blind guard would
drop it and leave a no-longer-matching slot in the result.
2. No-op sort-set suppression. process_set_op and recompute_computed_sorts_for
_slot full-overwrote sort layers unconditionally, so the re-emitter's steady
-state same-value sets became real sort mutations and flooded bucket
-membership maintenance. Both now reconstruct the slot's current value from
the published snapshot (~num_bits contains) and skip the clear+set plus the
mutated-slot registration when unchanged. Within-batch staleness is covered by
the upstream dedup_ops LIFO; the cross-batch return-to-published-value shape
is a known narrow, self-healing residual (documented at the call site). Dump/
test mode (no engine) keeps the unconditional overwrite.
3. Page-1 divergence canary. Config-gated (cache.page1_canary_sample_rate,
default 0 = off, hot-reloadable via PATCH) sampling of first-page bucket-sort
cache hits: re-derives the first page from the slow path (bounded to the query
limit, no doc fetches), diffs IDs, and on divergence increments
bitdex_cache_page1_divergence_total{sort_field} and warns once per entry per
interval with the key, min_tracked_value, capacity, needs_rebuild, bucket
cutoff.
Tests: deterministic stale-min interleaving test (fails under unconditional
apply), origin-gap test proving filter-mismatch removes still apply when the
value qualifies under the live bound (fails under origin-blind apply), same
-value sort/computed no-op suppression tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JustMaier
force-pushed
the
fix/cache-maint-races
branch
from
July 23, 2026 16:46
e25c5d6 to
968dfdd
Compare
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
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
Three fixes from the 2026-07-23 cache-race audit. Prod symptom: transient top-of-feed loss on cached
bucket + reactionCountentries that self-heals atbucket_entry_ttl.1. Stale-min remove race (CONFIRMED bug)
The async cache worker captures
min_tracked_valueat collect, evaluates removes unlocked, and applies later. A concurrent paginationexpand()(4K→64K) lowers the floor and appends members between collect and apply, so apply removed the freshly-added legitimate members.Fix: removes are tagged by origin.
removes): the slot no longer matches the entry's filter. Produced byevaluate_filter_workand byevaluate_sort_work's qualifies-but-unmatched branch. Always applied (membership-guarded).sort_bound_removes): the slot's value fell below the floor captured at collect. Only these get the stale-min guard.A sort-bound candidate was, by construction, non-qualifying under the captured floor, so the guard reduces to a single live-bound test: apply the remove only when the value still does not qualify under the current floor (a concurrent expand that lowered the floor to include it ⇒ keep the member). No captured-bound value needs to be carried.
Origin tagging is required (not just cleaner): a filter-mismatch remove can carry a sort value that qualifies under the live floor (e.g. a slot that stopped matching the filter but sorts inside the band expand opened). An origin-blind guard drops it and leaves a no-longer-matching slot in the result = transient false inclusion. (This was the review-caught gap in the first revision, where the guard was applied to all removes via a two-bound
captured_min_trackeddiscriminator.)Tests:
test_apply_maintenance_keeps_members_added_by_concurrent_expand— collect→expand→apply; the expand-added member survives, a genuine below-floor drop still applies. Fails under unconditional apply.test_apply_maintenance_filter_mismatch_remove_always_applies— a filter-mismatch remove whose value qualifies under the live floor MUST still evict. Verified it fails under the origin-blind guard and passes after.2. No-op sort-set suppression (flood amplifier)
process_set_op's sort branch andrecompute_computed_sorts_for_slot's computed-target branch full-overwrote every bit layer unconditionally. The steady-state re-emitter's same-value sets thus became real sort mutations →bucket_changed_slots→maintain_bucket_membershipflood.Fix: both reconstruct the slot's currently-published value (
ConcurrentEngine::reconstruct_sort_value, ~num_bitscontains) and skip the clear+set — and the mutated-slot registration — when unchanged.Noneengine (dump/test mode) keeps the unconditional overwrite.Staleness (documented at the call site): reconstruct reads the published snapshot (base + unmerged diffs). Within-batch re-sets of the same slot+field are collapsed upstream by
dedup_ops(LIFO), so intra-batch staleness can't drop a genuine change. The one known residual is cross-batch: a value that moves away and back to its published value across batches has the return set suppressed and keeps its published sort position until the next genuine change — a narrow, self-healing window accepted in exchange for killing the flood.Tests:
test_set_op_suppresses_same_value_sort_write,test_recompute_suppresses_same_value_computed_sort— same-value set/recompute emit zero sort mutations; changed value still writes the full overwrite.3. Built-in page-1 divergence canary
Config-gated (
cache.page1_canary_sample_rate, default0.0= off, hot-reloadable viaPATCH /indexes/{name}/config). On a sampled first-page bucket-sort cache hit it re-derives the first page from the slow path — bounded to the query limit, no doc fetches — diffs IDs, and on divergence incrementsbitdex_cache_page1_divergence_total{sort_field}andtracing::warns once per entry per 60s with theUnifiedKey,min_tracked_value,capacity,needs_rebuild, and bucket cutoff. Sampling is restricted to first-page queries (no cursor, no offset) — the top-of-feed case the prod symptom hit and the only offset-free fast/slow comparison. Counter registration and bridge wiring are#[cfg(feature = "server")]; the detection+warn path runs in any build.Q4 / known-test note
test_min_tracked_value_after_expansion(the known-deadlocked test, skipped in the suite run) collides nominally with this area (expand +min_tracked_value) but exercises only single-threadedexpand()bound bookkeeping — not the apply-time race this PR fixes. Fix #1 touchesapply_maintenance_results/ the evaluators, notexpand()ormin_tracked_value(), so it does not make that test fixable/relevant; its hang is environmental (orphaned-cargo target lock per the known-hangs reference), not a code deadlock in a path this PR changes. The newtest_apply_maintenance_keeps_members_added_by_concurrent_expandcovers the actual race directly.Testing
cargo test --lib --features pg-sync(skipping the known-deadlockedtest_min_tracked_value_after_expansion): 1255 passed, 8 ignored.cargo check --features server,pg-sync --libclean, no new warnings.🤖 Generated with Claude Code