fix: the pending-generation read cap counts every shard, not the worst one - #279
Merged
Merged
Conversation
…t one A read opens every pending generation of every writer shard, so the cap introduced in lance-format#278 must hold for the store as a whole. A store with 20 shards at ~1,400 generations each (25,588 total) passed the per-shard check and one point lookup opened all of them, OOMKilling the worker. The per-shard check stays (it fails fast, before the other shards are even listed); the total is checked once the snapshots are collected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Sep 30, 2026
…on is unchanged (#281) ## Problem The scanner's unchanged-version shortcut reuses the previous stats row wholesale. WAL flushes never bump the base-table version, so a store that no worker or master has ever merged keeps its version forever while its shards fill up. In production `mai3_bigclimb_run6p5_77b_t0r1` sat at "3 pending" in the stats table while its 20 shards accumulated 17,254 generations; the merge sweep reads that row, so it never scheduled a merge, and after #278/#279 every read on the store was refused (503) by the pending-generation cap. ## Fix Both `observe_generic` and `observe_one` still skip the base-table row count on an unchanged version, but recount `pending_wal_generations` (one small shard-manifest read per shard, under the existing observe timeout) and write it into the reused row. ## Verification - `skipped_round_recounts_pending_generations`: 1 → 4 flushed generations with the base version unchanged; the skipped row reports 4. - `generic_store_is_observed_with_pending_wal` extended the same way (3 → 5). - Master suite incl. etcd `--include-ignored` passes; clippy clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <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
1 participant
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.
Problem
The read cap from #278 (
pending_generations_max) is checked per writer shard. A read opens every pending generation of every shard, so a store with 20 shards at ~1,400 generations each (25,588 total) passes the check and one point lookup still opens all 25k datasets — it OOMKilled a 32 GiB worker in production an hour after #278 shipped.Fix
Keep the per-shard check (fails fast, before other shards are listed) and additionally refuse once the collected snapshots' total exceeds the cap, with the same
PENDING_GENERATIONS_EXCEEDEDprefix the server maps to503 OVERLOADED. Docs updated to say "per shard or in total".Verification
pending_generation_cap_counts_every_shard: cap 3, shards with 2+1 generations read fine; 2+2 is refused with "across 2 shards"; merging one shard brings reads back. Existing per-shard test unchanged; core lib suite passes.🤖 Generated with Claude Code