Repository navigation
fix: demand pipeline at production scale - #364
Merged
Merged
Conversation
Review round 5 brought production numbers: 13,405 tables, 354 with
pending WAL, 318 generic. Six findings, all confirmed and fixed here.
1. Full prefix reads fail past the etcd client's 4 MiB cap (~27k shard
records). Every planner read (demand-events, demand, executors,
assignments, queue, running) is now paged at 512 keys at one revision.
Reconcile runs detached from the leader loop so a slow pass cannot
starve the lease keepalive; one pass in flight, skipped ticks counted.
2. The scan read only sealed, so a lost merge-completion event left a
fully merged shard pending forever and an old table's first contact
read generation 1001 as 1001 pending. all_shard_watermarks now also
reads the base table's merged watermark per shard and the scan event
carries it.
3. Scan events for generic stores were published under
generic:generic:<name>; entry.name already carries the prefix.
4. Shadow placement reserved 1 byte when pending_bytes was unknown
(every real publisher reports 0). An unknown now reserves the full
per-pass cost (2 x ROLLOUT_APPEND_MAX_BYTES) and is counted.
5. One "disagreement" counter mixed discovery, capacity and ownership
effects. Replaced by planner_shadow_comparison{outcome} with
discovery_miss, placement_gap, phantom_task and agree, each a precise
question, documented as observations and not a takeover gate; the old
counter remains as their sum.
6. Age after merging past the kept entries: 128 pending, merge 64 gave
"unknown" in memory and generation 128's time after rebuild. The
times map is now bounded by cap only (pruning by merged made the kept
set order dependent), keeps the lowest 63 plus the newest, and is
filtered by merged on read. The answer carries a bound - Exact when
the oldest pending generation's time is kept, Upper otherwise - and
only an Exact age drives scoring and promotion. Executor events no
longer seed flush times (their timestamp is observation time).
Tests: a production-shape test seeds 13,405 tables x 2 shards, asserts
the unpaged read fails, the paged read returns all 26,810 records at one
revision, the fold counts them, a leader completes a reconcile while its
lease survives twelve seconds past the pass, and cleans up after itself
(gated on PLANNER_SCALE_TEST=1; run alone). Demand tests cover the
128/64 case as a flagged Upper bound identical after rebuild and in
reverse order, scan-with-merged folding, and the randomized
rebuild-equals-fold property still holds with the new bookkeeping.
Scoring asserts a bound never promotes or boosts. 16 demand, 5 scoring,
17 planner/executor/scan/resident etcd tests and 9 sweeper tests pass
on a clean etcd. Clippy -D warnings, fmt, typos clean. Spec updated.
Refs lance-format#333. Review round 5.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
17 of 18 tasks
beinan
added a commit
that referenced
this pull request
Oct 9, 2026
Part of #333. Follow-up to #364. Review round 5 measured production: **13,405 tables, ~354 with pending, 318 generic, 6 masters**. The design assumed "tens → low hundreds active" — wrong by two orders of magnitude — and that assumption directly produced the unpaged etcd reads and 1-byte reservations fixed in #364. Corrects §2 (workload table, with the error acknowledged), §4.5 (per-cycle cost now stated with paging and measured at production shape), §6 (when the single leader would need watch-driven folding or sharding), and marks backlog open question 1 answered. Docs only; typos 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.
Part of #333. Fixes all six findings of review round 5 (credit: production review), which brought real numbers: 13,405 tables, 354 with pending, 318 generic.
sealed; a lost completion event left a merged shard pending forever; old table's first contact → gen 1001 = 1001 pendingall_shard_watermarksreads the base MemWAL index's merged watermark per shard; scan events carry it.generic:generic:<name>entry.namealready has the prefix.pending_bytes == 0(all real publishers)2 × ROLLOUT_APPEND_MAX_BYTES); counted.planner_shadow_comparison{outcome ∈ discovery_miss, placement_gap, phantom_task, agree}, each a precise question; documented as observations. Old counter kept as the sum.Exact/Upper; only Exact promotes. Executor events no longer seed flush times. Keepalive: see #1.Validation
PLANNER_SCALE_TEST=1(run alone; it starves a shared single-node etcd).Upperbound, same after rebuild and in reverse; randomizedrebuild == foldstill holds; scan-with-merged folds.-D warnings, fmt, typos clean. Spec §1.1/§1.2a/§1.3/§2.3 updated.🤖 Generated with Claude Code