Repository navigation
fix: retry stale maintenance preparations without one-hour cooldown - #328
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 (P0.2, backlog C1). ## Problem `merge/failure.rs` recognised stale index/compaction preparations by matching exact prose plus the literal path `crates/lance-context-core/src/store_base.rs:` in the string Lance renders for `InvalidInput`. Moving the check, renaming the crate, or a change in Lance's wrapper format silently reverts a 2 s retry to a 1 h cooldown with `needs_attention` — the production failure #328 fixed. (Line numbers are *not* the fragility; any digits are accepted.) ## Approach - `lance-context-api`: `MaintenanceErrorCode::StalePreparation` → `[LC_STALE_PREPARATION]`, with `tag()` / `parse()`. The api crate hosts it so core, merge and master share it without a dependency cycle. - `store_base.rs`: all six stale-preparation sites tag their message (index ×3, compaction ×2, merge watermark overlap ×1). - `failure.rs`: `classify` checks ownership first, then the code, then falls back to the legacy text matcher for records written by old binaries. `corrected()` accepts either form. The legacy matcher is marked removable once no stored record lacks a code. - `docs/concurrent-indexing.md` updated. ## Alternatives A typed error enum through `lance::Error` is cleaner, but errors cross worker HTTP and etcd task records as strings; a code inside the string is the smallest change that survives every boundary. Leaving the text matcher as the only path: rejected, it is the bug's root cause. ## Validation - New tests: code round-trip through wrappers and arbitrary paths; ownership precedence; code does not launder a 403; old `DataOrConfiguration` record with a code is corrected without resetting attempts/clock. - `lance-context-merge`: 11 unit + 10 etcd-backed tests pass locally; `lance-context-core` prepared/stale tests pass; `clippy --all-targets -D warnings`, `fmt`, `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.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.
Production index preparation was rejected after a concurrent index replacement or fragment rewrite, but Lance wrapped the conflict in
Invalid user input. The generic classifier treated the first failure as a permanent data/configuration error and delayed the next attempt for an hour. The current production failure inventory contains 15 such index records.Recognize the exact stale index/compaction preparation errors as retryable. Interpret previously misclassified records using the corrected policy while preserving failure timestamps and consecutive attempts. Repeated failures still exhaust the existing retry budget; ownership, fencing, and permanent storage/data errors retain their existing protection. The paginated failure inventory reports the same effective policy as admission. Old masters continue to use the stored cooldown during a rolling upgrade.
Validation: all 19 merge crate tests passed (10 unit and 9 isolated etcd tests), and Rust 1.99 Clippy passed. Tests cover the production error strings, old-record recovery without writes, persistence across reconnect, escalating retry budgets, and revoked claims that cannot reset failures. Full CI runs before merge.
An isolated Pod running the current production image reproduced the old behavior: the stale-preparation record remained unqueued across multiple recovery sweeps, while failure bytes and target locks were unchanged. The same probe is prepared for the new image before rollout.