feat(config): allow idle eviction on single_value per_value_lazy filter fields [GATED — HOLD] - #318
Draft
JustMaier wants to merge 1 commit into
Draft
feat(config): allow idle eviction on single_value per_value_lazy filter fields [GATED — HOLD]#318JustMaier wants to merge 1 commit into
JustMaier wants to merge 1 commit into
Conversation
…er fields [GATED]⚠️ HOLD — do NOT enable in prod config before v1.1.46 (activation-orphan fix) and a review of the eviction over-eviction path for postId (see caveats below). This commit is INERT: it only relaxes a validator; no field gains eviction until a config opts in. postId / postedToId are single_value per_value_lazy filter fields with NO eviction, so their per-value bitmaps accumulate unbounded on a serving pod (postedToId grew 1,833 → 102,663 resident VBs in 10h on prod bitdex-0). They are the only genuinely monotonic term in the RSS warm-growth. The config validator hard-rejected `eviction:` on any non-multi_value field, so bounding them required a code change. The rest of the eviction machinery is already field-type-agnostic and safe for these fields: - the idle sweep skips DIRTY VBs (concurrent_engine.rs ~3419) — unpersisted activation inserts are never dropped; - `eviction_configs` is built from any `eviction.is_some()` (not gated on type); - stamp-on-read fires for any eviction-enabled field (ensure_fields_loaded); - postId/postedToId are already in `lazy_value_fields` (via per_value_lazy), so an evicted value re-loads from ShardStore on next query, and every lazy-load apply provably PRESERVES diffs (load_base = base |= disk). So this change only relaxes the validator to `MultiValue || per_value_lazy`, mirroring the exact `lazy_value_fields` inclusion condition. Eager single_value fields (e.g. userId) stay rejected — they load whole and can't reload one value. Tests: validator accepts eviction on single_value+per_value_lazy, still rejects eager single_value, still accepts multi_value (regression). 14 passed. CAVEATS gating prod activation (config, not code): 1. Ships only AFTER v1.1.46 orphan fix. Enabling postId eviction adds evict+reload churn on postId during the exact activation window whose whole-batch op-loss is still unresolved — don't widen that race before it's closed. 2. Latent over-eviction: the sweep's `unwrap_or(true)` treats never-query-stamped values as evictable once clean. A replay/activation-loaded postId VB that hasn't been query-stamped becomes evictable on the next clean sweep. Before enabling, either stamp values on lazy-load (not just on query) or confirm this churn is acceptable — otherwise postId thrashes evict→disk-reload on the query hot path. 3. Pick a LONG idle for postId/postedToId (>= modelVersionIds' 7200s), since a re-load is a disk read on the query path; short idles like tagIds' 300s would over-evict a high-cardinality FK. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
This is the real fix for the only unbounded term in the prod RSS warm-growth (P2), but its config activation is gated. The code here is inert: it only relaxes a validator; no field gains eviction until a config opts in. See the gate below.
Why
postId/postedToIdaresingle_value per_value_lazyfilter fields with no eviction, so their per-value bitmaps accumulate unbounded on a serving pod (postedToIdgrew 1,833 → 102,663 resident bitmaps in 10h on prod bitdex-0). They are the only genuinely monotonic term in the RSS ratchet. The config validator hard-rejectedeviction:on any non-multi_valuefield, so bounding them required a code change.What
One change: relax the validator from
field_type == MultiValuetoMultiValue || per_value_lazy(config.rs), mirroring the exactlazy_value_fieldsinclusion condition inConcurrentEngine(concurrent_engine.rs:584). Eagersingle_valuefields (e.g.userId) stay rejected — they load whole and can't reload one evicted value.Everything else already handles these fields safely:
concurrent_engine.rs ~3419) — unpersisted activation inserts are never dropped (the load-bearing correctness property);eviction_configsand stamp-on-read are not field-type-gated;postId/postedToIdare already inlazy_value_fields, and every lazy-load apply provably preserves diffs (load_base = base |= disk), so an evicted value re-loads correctly.Tests
config::tests: accepts eviction onsingle_value + per_value_lazy, still rejects eagersingle_value, still acceptsmulti_value(regression). 14 passed.🔒 Gate — activate the config ONLY after all three
unwrap_or(true)(concurrent_engine.rs ~3430) currently makes never-query-stamped values evictable once clean. A replay/activation-loaded postId that hasn't been queried would be evicted immediately. B assumes 2a's fixed behavior (a never-stamped value is not evicted before its idle window).All three are also in the commit message. Convergence note: gate (2) is independently the same prerequisite as fixer's 2a.
🤖 Generated with Claude Code