Skip to content

fix(warm_registry): record original filters, not post-prefilter-substitution - #228

Merged
JustMaier merged 1 commit into
mainfrom
fix/warm-registry-persist
Apr 25, 2026
Merged

fix(warm_registry): record original filters, not post-prefilter-substitution#228
JustMaier merged 1 commit into
mainfrom
fix/warm-registry-persist

Conversation

@JustMaier

Copy link
Copy Markdown
Contributor

Summary

Warm registry persist had been silently failing on every diverse-shape query batch. server.rs:2787 overwrites query.filters with the post-prefilter-substitution form when a prefilter matches, prepending a FilterClause::BucketBitmap { bitmap: Arc<RoaringBitmap>, .. } clause. Later at server.rs:2817 we recorded &query.filters — the now-substituted version — into the warm registry. When the merge thread tried to serialize, serde rejected BucketBitmap (#[serde(skip)]) and the entire batch was dropped:

warm registry persist failed: the enum variant
FilterClause::BucketBitmap cannot be serialized

Symptom

887,596 queries over 5 min in the post-PR-#224 regression-resolution run → zero successful warm persists, warm.json stayed at the previous session's 1-entry state, auto-warm on next boot restored 1 stale entry instead of a meaningful warm window.

Fix

Capture original_filters_for_warm = query.filters.clone() BEFORE the substitution call, pass that to warm_registry().record(...). The substituted form is still used for execution; only the shape recorded into the persistent registry comes from the user's input.

14 insertions, 2 deletions. No behavior change on the executor or planner.

Verification

Same replay corpus that exposed the bug:

Metric Pre-fix Post-fix
warm registry: persisted N shapes events 0 / 5 min 10+ events, fires every merge cycle
warm registry persist failed: BucketBitmap 9+ / 5 min 0
warm.json size 221 B (1 stale entry) 410 KB (187 unique shapes)
Top entry frequency in warm.json 1 6,248

Stacking

Test plan

  • Build clean (cargo build --profile fast --features server,pg-sync)
  • Live verification with 273K-query replay → warm.json populated correctly with 187 entries
  • Time-to-first-warm-restore measurement post-merge (separate task)

🤖 Generated with Claude Code

…itution

The query handler at server.rs:2787 overwrites `query.filters` with the
post-prefilter-substitution form when a registered prefilter matches:

    if let Cow::Owned(ref new_clauses) = substituted_clauses {
        query.filters = new_clauses.clone();
    }

The substituted form prepends a `FilterClause::BucketBitmap { bitmap:
Arc<RoaringBitmap>, .. }` clause containing the prefilter's pre-computed
bitmap. That variant is `#[serde(skip)]` — it's an internal optimization
shape never meant to leave the executor.

Later, server.rs:2817 records `&query.filters` into the warm registry.
By then `query.filters` may contain `BucketBitmap`. When the merge
thread's periodic warm-persist serializes the recorded entries to
`warm.json`, serde returns:

    warm registry persist failed: the enum variant
    FilterClause::BucketBitmap cannot be serialized

…and the entire batch is dropped on the floor. Result observed in the
post-PR-#224 measurement run: 887,596 queries over 5 min → zero
successful warm persists, warm.json stays at the previous run's 1-entry
state, auto-warm on next boot is effectively useless.

Fix: capture `original_filters_for_warm = query.filters.clone()` BEFORE
the substitution call, then pass that to `warm_registry().record(...)`.
The substituted form is still used for execution; only the shape we
record into the persistent registry comes from the user's input.

Verified locally with the same replay corpus that exposed the bug:

    Pre-fix:
      warm registry: persisted N shapes — 0 events / 5 min
      warm registry persist failed: BucketBitmap … — 9+ events / 5 min
      warm.json — stale 1-entry file from earlier session

    Post-fix:
      warm registry: persisted N shapes — fires every merge cycle
      Final: persisted 187 shapes (289,912 total recorded, 187 unique)
      warm.json — 410 KB, 187 entries, top frequency 6,248

This unblocks the time-to-first-warm-restore measurement (originally
filed as part of Scarlet's regression-resolution check). With a
populated warm.json, cold-pod restart can run a meaningful auto-warm
pass instead of restoring 1 stale entry in 100 ms.

The fix has no behavior change on the executor or planner — only the
warm-registry input source.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JustMaier

Copy link
Copy Markdown
Contributor Author

Reviewed. Approved.

Surgical fix. Capture original_filters_for_warm = query.filters.clone() before prefilter substitution at line 2787, then thread it through the record call at line 2823 instead of the post-substitution query.filters. Two-comment WHY captures the trap (BucketBitmap serde-skip → serde panic → silent batch drop) — exactly the kind of comment that will save someone hours when they look at this code in 6 months.

Verification table is striking — 0 → every cycle persist firing, warm.json 221 B → 410 KB, 1 → 187 unique shapes. Bug was eating every diverse-shape persist for who-knows-how-long.

No correctness risk on the executor side — substitution still happens AFTER capture, so the planner/executor still sees the optimized form. Only the warm registry sees the original.

One observation (not blocker): clone() on the filter Vec is per-query overhead. Filters are usually shallow (few clauses), so cheap. If a future profiler shows it as hot, swap to recording the canonical form before substitution + reuse the canonical (which is already serializable). Out of scope for this fix.

Approved. Same self-approve caveat — treat this comment as approval. Merging next + tagging v1.0.171-jemalloc.

@JustMaier
JustMaier merged commit 17ba2d7 into main Apr 25, 2026
4 of 5 checks passed
@JustMaier
JustMaier deleted the fix/warm-registry-persist branch April 25, 2026 10:09
JustMaier added a commit that referenced this pull request Apr 25, 2026
Captures the full causal chain + measurement data for the post-#224
apply-mean regression and its resolution via PRs #226/#227/#228:

- Bisect (mut_us 4-15μs vs lock_us 8-235ms in [remove_bulk_slow])
  showed apply was waiting for a long-held FilterField read.
- bitmap_memory_cache::scan_tick → FilterField::bitmap_bytes() iterating
  769K userId entries under one read lock was the holder.
- Pre-#224 the merge thread's mark_stale calls were unreachable, so
  the scanner had nothing to re-read and bitmap_bytes was effectively
  dormant. Post-#224 it ran at full cadence.
- PR #227 chunked bitmap_bytes to bound the read-lock-hold; lock_us
  dropped 80-235ms → 5-11ms.
- PR #228 fixed a separate latent bug in the warm-persist path where
  query.filters had been overwritten with the post-prefilter-substitution
  form (containing FilterClause::BucketBitmap, #[serde(skip)]) and
  serialization was silently failing every batch.
- Final time-to-first-warm-restore: 163s total (159s dump restore
  + 4.6s auto-warm of 187 shapes), down from the handoff's reported
  30-60min — a 100-300x reduction.

Full session ship list captured at the bottom (v1.0.165 through
v1.0.171 — seven perf PRs all stacked cleanly).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant