Repository navigation
fix: give prepared compaction the next merge admission turn - #324
Merged
Merged
Conversation
beinan
marked this pull request as ready for review
October 6, 2026 06:41
beinan
added a commit
that referenced
this pull request
Oct 6, 2026
Keep explicitly configured owned tables under continuous master catch-up coverage, including low-count tails after successful slices. Persist the canonical target set in the shared policy so failover cannot silently discard desired coverage; fresh stats, cooldowns, fencing and global capacity still gate every new Job. If a previously confirmed Job disappears, exact terminal child-container evidence can retire its inventory reservation as a failure. Missing or running children and identity mismatches remain unresolved. Execution/claim/target-lock records are retained for normal storage recovery before successor writes. Rebased onto merged #321; this PR contains only the continuous coverage and missing-Job changes. New cases cover continued service after restart, mismatched replica policy, stale post-success stats and missing-Job identity/terminal checks. Formatting passed. Local catchup suite: 33/33 passed with real etcd and native storage. Combined with #321, #322 and #324: all 170 master library tests passed, including real-etcd tests. CI remains in progress. Existing external publisher adoption and production activation remain separate, qualified per-target migrations; no owner keys are reset by this change. 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. 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>
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.3, backlog C4 and A2). ## Problem Three scheduling questions had no metric: how long tasks wait from enqueue to claim; whether prepared compactions/indexes get their commit turn or expire and rebuild (#324's hint is honoured only by upgraded binaries and nothing records a miss); and how busy each dispatch pool is — the resident-only slot added in #331 was not covered by any gauge, so `MERGE_WAL_CONCURRENCY=N` read as N while N+1 could run. ## Approach | Metric | Labels | Meaning | |---|---|---| | `master_task_schedule_to_bind_seconds` | `kind` | `enqueued_at → started_at`. Growth with idle pools = admission blocking; with full pools = capacity. | | `master_commit_turn_wait_seconds` | `kind, result` | How long a prepared compaction/index waited for its write turn. | | `master_commit_turn_requests_total` | `kind, result` | `served` vs `expired` (preparation discarded and rebuilt). Rising `expired` with flat `master_merge_yield_to_compaction_total` ⇒ a contender ignores the hint. | | `master_task_pool_in_use` | `pool` ∈ general/compact/merge/resident_merge | Permits in use, set every poll. | No scheduling behaviour changes. `docs/compaction-commit-fairness.md` explains how to read the new counters. ## Validation `cargo clippy --all-targets -D warnings`, `fmt`, `typos` clean. Metrics are recorded at existing code points with no new control flow, so behaviour coverage is the existing scheduler suite (CI). 🤖 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
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.
A persistent publisher can repeatedly claim another merge pass before a prepared compaction obtains commit ownership, causing the compactor to exhaust its wait budget and rebuild files. Publish a commit-ready hint only after nonempty output is prepared; subsequent merge claims yield to the matching live preparer.
The hint shares the preparation claim lease and is checked in the existing admission snapshot and final claim CAS. Preparation stays concurrent, admitted merges are not cancelled, and unresolved merge executions can still be claimed for storage recovery. Normal exclusive commit ownership and the existing wait budget remain in force.
Validation: formatting/diff checks passed. All three local real-etcd tests passed for native pass handoff, lease-loss cleanup/stale publisher rejection, and recovery bypass. Combined with #321, #322 and #323: all 170 master library tests passed, including real-etcd tests. All 11 CI checks passed before merge. Every scheduling/catchup contender needs the updated binary to honor this fairness hint; production rollout is separate.