fix(master): a MergeWal that errors on a worker and merges nothing is a failure - #268
Merged
beinan merged 1 commit intoSep 26, 2026
Conversation
… a failure run_merge_wal reported success whenever at least one worker answered, so a store whose base table is broken -- the shard owner 500s with 'Not found: data/<frag>.lance', the other workers hold no data and reclaim 0 -- completed as 'merged 0 generations across 1/2 workers'. The failure cooldown (lance-format#267) never saw a failure and never engaged; staging reproduced this with a deliberately broken store that racked up 56 'done' tasks. Fail the task when any worker errored and nothing was merged. Partial progress with some errors stays a success; the next sweep retries. 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>
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.
Problem
run_merge_walreturnsOkwhenever 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 asSo #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
merge_wal_with_an_erroring_worker_and_no_progress_fails: one stub worker returns{reclaimed: 0}, the other returns 500; the task reachesFailedwith "nothing was merged".merge_wal_broadcasts_and_sums_reclaimed(all-ok) andmerge_wal_without_endpoints_failsunchanged; master suite 27/27 etcd-backed + 42 default, clippy clean.Refs #266, #267.
🤖 Generated with Claude Code