Repository navigation
Prepare key indexes outside table write ownership - #327
Merged
Merged
Conversation
This was referenced Oct 8, 2026
beinan
added a commit
that referenced
this pull request
Oct 8, 2026
Part of #333. Companion to #334. ## What Adds `docs/design/scheduler-improvement-backlog.md`: the defects in the current maintenance scheduling layer, grouped by severity with file:line references, and the ordered P0–P4 plan to fix them. Highlights: - **Architectural**: no single owner of "what runs next"; capacity as semaphores without memory; 7-key etcd snapshot per candidate per claim; discovery duplicated five ways; dependencies as task DAG edges; stats scanner mutating tables directly; six allowlists. - **Correctness**: error classification by message text incl. source line numbers (#328); potential retry storm across #327/#330/#328 for slow-but-alive index builds; starvation of queued legacy tasks accepted as expected in #331's test; fairness hint (#324) without version or metric; `/merge-progress` pushing read consistency onto clients; random-id queue order; backpressure as a cliff at 4096 generations. - **Plan**: P0 is zero behaviour change (pure eligibility function, typed errors, metrics, e2e test, runbook); P1 shadow planner; P2–P4 per-table migration and deletion, each with mixed-version rules. ## Validation Docs only. `typos` clean locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Oct 8, 2026
beinan
added a commit
that referenced
this pull request
Oct 8, 2026
Part of #333. Addresses the review comments on #334/#335. ## Changes to the design | Review point | Change | |---|---| | One assignment per table re-serialises merge and compaction | Invariant 1 is now *one write-turn holder* per table; preparation units run concurrently with the turn holder and each other (§4.3, §4.5, §7). Explicit: never re-serialise merge behind preparation (#308, #327). | | Strict class order + in-class aging still starves commit-ready compaction under a hot merger | Two hard bounds in §4.2: `max_consecutive_turns[kind]` (applies across classes, incl. class 1) and `max_turn_wait_secs[kind]` promoting to class 1. Invariant 5 rewritten. | | Delta demand events race and get overwritten; stale snapshots can hide progress | Per-shard watermarks (`sealed_through_seq`), max-merge per shard, sum across shards; snapshot lowers a watermark only with a newer observed revision; new invariant 5a (idempotent folding). | | `bytes_free` heartbeat is a sample, not a reservation | Assignments carry `reserved_bytes`; headroom = `bytes_total − Σ reserved` over live assignments, rebuilt from etcd on failover; executor still enforces local budget (§4.4). | | Shadow phase would leave a scheduling gap | P1 keeps all loops running; scanner *additionally* writes demand; per-table switch only after the fleet is homogeneous (§8, backlog P1/P2 and mixed-version rules). | | Isolating legacy RPCs is not recovery | Stated explicitly in §4.4 and backlog C3: isolation bounds blast radius; stall detection stays on the execution's actual read/encode/commit progress. | ## Factual corrections - `generate_id()` is UUIDv7 (`core/src/id.rs:28`): the queue is approximately enqueue-time ordered; what it lacks is priority. Backlog §0 and C6 corrected. - `failure.rs:74-79` parses line/column as `u32`, so renumbering is safe. What breaks it is a path move, crate rename, or a change in how Lance wraps `InvalidInput`. C1 corrected. Docs only; `typos` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Oct 8, 2026
Part of #333 (P0.1). ## Problem `claim_next` in `task_store.rs` decides per queued candidate whether this poller may attempt the claim transaction through a chain of inlined `continue`s: kind filter, resident-only filter (#331), preparation shape (#308/#327), commit-ready yield (#324), the seven-key ownership predicate, and the worker-execution rule. None of it was testable without etcd, the `preparing` predicate existed twice, and no skip had a name. ## Approach New `eligibility.rs` with three pure functions: `pre_dependency_filter`, `claim_shape`, `admission_filter`, each returning `Result<(), Skip>`. `claim_next` performs the same etcd reads in the same order and feeds them in; `YieldToPreparedCommit` and `OwnershipBlocked` map to the existing counters, every other skip is traced with its reason. **No behaviour change.** This is the foundation for the planner's filter phase and for the "why not now" surface. ## Alternatives Testing through etcd integration only (status quo): too slow and too coarse to pin down individual predicates. A trait over the store: more machinery than needed; plain inputs are enough. ## Validation - 7 new etcd-free unit tests: resident filtering incl. wildcard, preparation opt-in excluding draining/generic, catch-up sharing exact-match, commit-ready yield incl. recovery bypass and stale-hint ignore, each ownership predicate, worker-execution rule. - `cargo clippy --all-targets -D warnings`, `cargo fmt --check`, `typos` clean. - Locally against real etcd: `task_store` 23/23, `scheduler` 31/31. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Oct 8, 2026
) Part of #333 (P0.4, backlog C2). ## Problem `watch_maintenance_preparation` cancels a compaction/index preparation after `MAINTENANCE_IDLE_TIMEOUT_SECS` without a completed step. That boundary had no direct test. A regression to wall-clock cancellation, or a change that stopped counting real index IO as a step, would cancel every long index build, classify it retryable (#328) and rebuild it every few seconds — the #327 + #330 + #328 interaction flagged in the backlog. ## Approach One etcd-backed test, 3 s idle timeout, three cases: 1. **Slow but alive** — 8 s preparation completing one step per second finishes. 2. **Silent** — cancelled within `[IDLE, IDLE+4)` with the exact `"maintenance preparation made no progress"` error. 3. **Real index build** under the same short timeout completes, proving `data/` and `_indices/` IO is observed as progress through the preparation store wrapper. Mutation check: removing `checkpoint()` from case 1 fails the test with `alive preparation was cancelled` after ~4 s. ## Validation Test passes locally against real etcd (~12 s). Clippy `-D warnings`, fmt, typos clean. Test-only change. 🤖 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.
IndexId currently holds table write ownership while scanning keys and building index files, so a long index build blocks WAL publication. Opted-in rollout tables now prepare BTree/ZoneMap files outside write ownership, then acquire the existing fenced writer protocol only to validate and publish the result.
Preserve source fragments, same-name index metadata, schema and the original read version; reject invalidated output while allowing append merges and uncovered-fragment reads.
Reuse compaction's lease, commit-turn and recovery protocol, including mixed-version wire keys. Keep dedicated-owner exclusions and existing concurrency limits.
INDEX_PREPARE_TARGETSdefaults to disabled.Observe completed data/index IO during preparation. Lance 9's basic scalar trainers ignore fine-grained progress callbacks; response headers, metadata polling and failed IO must not mask stalls.
Update
async-traitto 0.1.92, which removes the redundant generatedmust_useattribute rejected by current Rust 1.99 Clippy. No lint suppression.Validation: 473 local tests passed (core library, master unit and all 98 etcd-backed master tests); Clippy for core/master libraries/tests passed with warnings denied. Focused regressions cover both index types and append orders, watermarks/reads, stale source/index/schema rejection, denied publication, concurrent admission, lease-loss replacement and isolated cloud/local IO progress. GitHub CI must pass before merge.