Repository navigation
fix(master): page by bytes, chunk writes by bytes, promote on wait floors - #366
Merged
Merged
Conversation
…oors Review round 6, five findings, all confirmed. 1. The 512-key page limited count, not bytes: 512 tables x 8 shards with full time maps returned ~5.6 MB and failed the 4 MiB cap, and a reconcile would retry the same page forever. Pages are now adaptive: start at 512 keys, halve on a response over 1 MiB or on a message-size error, grow back when small, still at one revision. 2. Cache writes chunked only by op count; 100 twenty-shard records (~2.7 MB) exceeded etcd's 1.5 MiB request cap. Writes are now chunked by op count and by bytes (768 KiB per txn), sized per op. 3. A long-lived tail could never promote for age: the first 63 timestamps occupy the bounded map forever, newer ones are dropped, and Upper bounds were excluded from promotion. An Upper bound on the flush time is a floor on the wait (flushed no later than that), which is exactly what "waited at least max_turn_wait" needs. Promotion now uses the wait floor from either bound; score aging still uses exact times only so a bound cannot inflate a score. 4. The comparison conflated queued with running, missed discovery on tables the planner could not place, and reported zeros when the queue read failed. It now reads queued and running as separate paged sets, compares per due table (discovery_miss / queued_only / running / placement_gap / phantom_task), and records nothing on a read failure. 5. The byte estimate capped at 2 x max_bytes, but the append path reads a whole generation before checking the target, so an oversized generation sets the real floor. Estimate is now 2 x max(max_bytes, largest generation seen); with no size data the whole local budget. Tests: chunk_ops unit test (count-bound, byte-bound, oversized op); scale test at the reviewer's shapes - 512 x 8 with full maps (fixture asserted > 4 MiB; paged read returns 4,096), and a 612-table reconcile with 20-shard records writing within request limits (gated on PLANNER_SCALE_TEST=1, run alone, drop-guard cleanup); demand test for the 70-merged-then-two-pending tail asserting a usable day-long wait floor identical after rebuild; scoring test that a bound promotes on wait but adds no age boost. 16 demand, 5 scoring, 19 planner/executor/ scan/resident etcd tests and 39 merge-crate tests pass on a clean etcd. Clippy -D warnings, fmt, typos clean. Spec updated. Refs lance-format#333. Review round 6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
17 of 18 tasks
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.
Part of #333. Fixes all five findings of review round 6 (credit: production review).
discovery_miss / queued_only / running / placement_gap / phantom_task; skip-and-count on read failure2 × max(max_bytes, largest generation seen); whole local budget when unknownValidation
chunk_ops: count-bound, byte-bound, oversized-op-alone.PLANNER_SCALE_TEST=1, run alone, drop-guard cleanup): 512×8 full maps — fixture asserted > 4 MiB total and unpaged read fails, paged read returns 4,096; 612-table reconcile incl. 100×20-shard records writes within limits.-D warnings, fmt, typos clean. Spec §1.1/§1.2a/§2.3 updated.🤖 Generated with Claude Code