feat(master): cool down targets that fail repeatedly so sweeps stop re-enqueueing them - #267
Merged
Merged
Conversation
…e-enqueueing them A store whose base-table manifest names a fragment that no longer exists fails every merge and every compaction at the same point, forever. The scheduler had no memory of that: each auto-sweep re-enqueued it, and each attempt held a task slot through a full serial fan-out before failing. In production five such stores (one with 7,985 pending generations) consumed roughly half the fleet's merge capacity while healthy stores queued behind them; the merge queue climbed from 0 to 109 in an hour. The task store now records consecutive failures per (kind, target) under an etcd key leased for the cooldown duration. Past TASK_COOLDOWN_AFTER_FAILURES (default 3) the sweeps skip the target for TASK_COOLDOWN_BASE_SECS (default 10 min), doubling per further failure up to TASK_COOLDOWN_MAX_SECS (6 h). A success clears it; a manual POST /tasks is not gated. Entering cooldown logs a warn with the last error and increments master_task_cooldowns_total, and GET /api/v1/scheduler/cooldowns lists what the master has given up on. Test (etcd-backed): two failures on a target cross a threshold of 2, the sweep then enqueues nothing for it, list_cooldowns reports it with until_ms set, and a manual enqueue still succeeds. Also asserts a target below the threshold is not treated as cooling down. Closes lance-format#266. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Sep 26, 2026
… a failure (#268) ## Problem `run_merge_wal` returns `Ok` whenever at least one worker responded (`ok_workers > 0`), tolerating errors on the rest so a single slow or restarting worker doesn't fail the whole task. That tolerance also swallows the one failure shape we most need to see: a store whose **base table is broken**. Its shard owner returns 500 (`Not found: .../data/<frag>.lance`), the other workers hold no shard and reclaim 0, and the task completes as ``` merged 0 generations across 1/2 workers ``` So #267's failure cooldown never engages — staging reproduced this with a deliberately broken store that accumulated **56 "done" tasks** and zero cooldowns, while production's five broken stores have been "succeeding" the same way. ## Fix Fail the task when **any worker errored and nothing was merged**. Partial progress with some errors is still success (the next sweep retries the stragglers); all-workers-tolerated-404 with 0 reclaimed is still success (the store is simply empty). Only "someone broke and we achieved nothing" becomes a failure, which is exactly what the cooldown needs. ## Verification - New etcd-backed test `merge_wal_with_an_erroring_worker_and_no_progress_fails`: one stub worker returns `{reclaimed: 0}`, the other returns 500; the task reaches `Failed` with "nothing was merged". - Existing `merge_wal_broadcasts_and_sums_reclaimed` (all-ok) and `merge_wal_without_endpoints_fails` unchanged; master suite 27/27 etcd-backed + 42 default, clippy clean. Refs #266, #267. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Beinan Wang <> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Sep 26, 2026
…ouble (#269) ## Problem #267 leased the cooldown record for the window's own duration. When the window lapsed, the failure count went with it, so the next failure started again from the threshold and the window never grew. On staging with 6 masters: the instant a broken store's 10-minute window closed, every master's sweep re-probed it (a burst of one task per master), three failures later it was back in a fresh 10-minute window — forever, never doubling. Observed: failures at 09:19:33/37/38 → window to 09:29 → failures at 09:29:40/41/42 → window to 09:39. `failures` stayed at 3. ## Fix Lease the record for `TASK_COOLDOWN_MAX_SECS` regardless of the window, so the count survives the window closing. The first failure after a window is N+1 and gets a doubled window, as the policy intends. `is_cooling_down` / `list_cooldowns` now check `until_ms > now` rather than treating the record's presence as the window. A success still clears the record. ## Verification Extended `repeated_failures_cool_the_target_down_and_sweeps_skip_it`: after two failures (threshold 2, base 1 h) the manual enqueue fails as failure **3** and the record's window is >1.5 h, i.e. doubled — not a fresh 1 h. Master suite 27/27 etcd-backed + 42 default, clippy clean. Refs #266, #267, #268. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Beinan Wang <> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Sep 26, 2026
…270) ## Problem `record_failure` (#267) bumped the per-target failure count with a plain get → +1 → put. Several masters finish a failing task for the same target within seconds of one another (they all swept it at the same instant), so two could read the same count and both write `count+1` — a lost update. Staging showed three masters failing the same store at 10:34:49/:51/:52 with the record under-counting; a broken store then takes an extra window or two to reach the threshold. ## Fix `get_cooldown_versioned` returns the key's `mod_revision`; `record_failure` writes the new record in a `Txn` conditioned on `Compare::mod_revision == observed`, and on conflict revokes its lease, backs off 5–25 ms with jitter, and retries (up to 64 attempts — contention is bounded by the master count, a handful). The success path is unchanged. ## Verification New etcd-backed test `concurrent_failure_records_are_not_lost`: 12 concurrent `record_failure` calls on one target, asserts `failures == 12`. Under the pre-fix get/put it lost updates; with CAS but a tight 8-retry loop it exhausted retries; with jittered backoff all 12 land. Master suite 28/28 etcd-backed + 42 default, clippy clean. Refs #266, #267, #269. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Beinan Wang <> 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.
Closes #266.
Problem
A store whose base-table manifest names a fragment that no longer exists fails every
MergeWalandCompactat the same point, forever:The scheduler has no memory of failure.
sweep_merge_wal_inner/sweep_candidates_innerre-enqueue every over-threshold store on every tick (enqueueonly de-dupes against queued/running), so a task that just failed is back in the queue ten minutes later, and each attempt holds aTASK_CONCURRENCYslot through a full serial worker fan-out before failing. Production 2026-09-26: five such stores (one at 7,985 pending generations) consumed roughly half of 6 masters' merge slots; the merge queue for healthy stores went 0 → 109 in an hour.Fix
TaskStorerecords consecutive failures per(kind, target)in etcd under<prefix>/cooldown/<kind>/<target>, leased for the cooldown duration so it ages out on its own.TASK_COOLDOWN_AFTER_FAILURES(default 3), the sweeps skip the target forTASK_COOLDOWN_BASE_SECS(default 600), doubling per further failure up toTASK_COOLDOWN_MAX_SECS(default 21600).0disables.POST /tasksis not gated — operators can always force an attempt.max, so a slow trickle of unrelated failures doesn't accumulate forever) and is not a cooldown.warn!with the last error andmaster_task_cooldowns_total{kind};GET /api/v1/scheduler/cooldownsreturnsVec<TaskCooldown>(kind, target, failures, until_ms, last_error) so a broken store is visible instead of silently eating capacity.Verification
cargo clippy --workspace --all-targets -D warningsclean.ETCD_TEST_ENDPOINTSagainst local etcd 3.7), including newrepeated_failures_cool_the_target_down_and_sweeps_skip_it: threshold 2, no worker endpoints so every MergeWal fails; asserts the target is not cooling after one failure, is after two, the sweep then enqueues 0,list_cooldownsreports it withuntil_msset, and a manual enqueue succeeds.is_cooling_downrequiresuntil_ms).Refs #264, #261.
🤖 Generated with Claude Code