Repository navigation
Reserve resident WAL task capacity beyond legacy merge RPCs - #331
Merged
Merged
Conversation
beinan
marked this pull request as ready for review
October 8, 2026 08:08
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
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.
Resident append tasks can remain queued while every ordinary merge slot is held by an unfinished legacy worker RPC. Add one configurable resident-only task slot per opted-in master (
ROLLOUT_APPEND_LOCAL_TASK_CONCURRENCY, default 1) so local WAL work can run while those RPCs remain outstanding. The ordinary merge pool retains its existing capacity; both pools share the existing local Arrow memory budget.Filter eligibility before dependency probes or claim mutation. Only configured, owned, non-draining rollout targets qualify; canonical claims, external catch-up ownership, fencing, and joined process drain are unchanged. Existing legacy ownership of the same table is never bypassed.
Regression coverage holds a legacy HTTP request open, queues another legacy task before real resident WAL work, and requires the resident merge to complete with correct readback while the two legacy tasks remain running/queued. Additional real-etcd coverage verifies filtering, external owners, draining/generic targets, wildcard opt-in, and replica deduplication. All 10 focused local tests passed against isolated real etcd, including the saturated legacy-pool end-to-end regression, replica deduplication, memory-budget separation, and resident continuation. All 11 CI checks passed on ebc210e, including full Rust unit/integration/real-etcd HA, coverage, Clippy, Python, and platform wheels.