Repository navigation
fix: prepare compaction files without blocking WAL merges - #308
Merged
beinan merged 3 commits intoOct 5, 2026
Merged
Conversation
beinan
marked this pull request as ready for review
October 5, 2026 17:28
beinan
added a commit
that referenced
this pull request
Oct 5, 2026
Production compaction still retained table writer ownership while waiting up to 10 seconds for the global `stats-writer` lock. After #308, nine production commit-phase observations averaged 12.51 seconds despite file encoding already running outside ownership. Finish successful prepared compactions durably before reopening a read handle and refreshing statistics. This releases the existing task/table keys through the normal completion transaction; it does not bypass fencing, drain guards, or dedicated publishers. Keep the caller's task and compaction permits through refresh to bound concurrency, and report its latency separately as `stats_refresh`. No-op preparations and failed publication/completion skip refresh. A crash after completion may omit the best-effort compaction counter update; scans still refresh table counts. Validation: add an etcd-backed regression covering owned and legacy publication with a competing stats writer. The Compact must become Done and the next MergeWal must acquire the table while refresh is blocked; the local compaction permit remains occupied until refresh finishes. Validation passed locally: all 139 master library tests including the full etcd HA suite (0 failures); focused prepared-compaction tests 4/4; master Clippy all targets with warnings denied; formatting and diff whitespace checks. GitHub CI is running. Co-authored-by: Beinan Wang <>
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>
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
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.
Large compactions retain table ownership while reading and encoding old fragments, preventing WAL merge publication for the entire rewrite. Prepare immutable compaction files under a separate task lease, then acquire the existing table writer protocol only for validation and publication. Append-only merges can publish during preparation; changes to source fragments invalidate the prepared rewrite.
COMPACTION_PREPARE_TARGETSenables selected rollouts or*, default off. Preparation handles reject manifest commits. Publication opens a fresh handle, retains original Lance read versions, and uses the existing fencing/recovery path for owned targets. Failed preparation cannot release another task’s merge ownership. Compaction capacity is reserved before claiming, within the existing general and compaction budgets. Ready output yields its local slot afterCOMPACTION_COMMIT_WAIT_SECS(120 by default) without cancelling the current writer.Validation: local workspace Clippy passed with warnings denied. All 17 append regression tests passed (two pre-existing benchmarks ignored), including both append/compact commit orders with physical rows, full blobs and WAL watermarks, deletion conflicts, and revoked commit authority. All 138 final master tests passed against local etcd, including lease replacement, dedicated ownership, capacity-before-claim, bounded admission waiting, and owned/unowned publication configurations. Full core regression passed: 290 tests, zero failures, 14 pre-existing ignored benchmarks/etcd tests. CI remains running. Isolated Azure overlap/process-loss validation is prepared for the merged-source image before activation.
No production behavior has been enabled. See
docs/concurrent-compaction.mdfor configuration, recovery and unreferenced-file behavior.