Repository navigation
fix(master): read merge progress at one etcd revision with a typed response - #341
Merged
beinan merged 1 commit intoOct 8, 2026
Merged
Conversation
…sponse `/merge-progress` did four independent etcd reads (execution, progress, work report, execution again) and, when they disagreed, returned `progress_changed_retry` or `execution_changed_retry`, pushing read consistency onto the caller. The response was hand-assembled JSON with no schema. Add `Coordinator::progress_snapshot`: one read for the execution id, then one transaction guarded on the execution bytes that fetches progress and work report together, so all three come from a single revision (bounded to three attempts; a target flipping executions faster than two etcd reads is not a status-endpoint concern). A work report whose sequence trails the current progress sequence is dropped rather than rendered beside a newer heartbeat. The handler returns `MergeProgressReport`, a serde struct with a fixed field set and a `status` enum of four values; the two retry statuses are gone. Absent fields are omitted so existing consumers that only read `status`/`task_id` are unaffected. `revision` is new. Validation: merge-crate test extended for the snapshot, stale-report dropping and the absent-target case; routes test asserts on the typed struct and its wire form. Both pass against real etcd. Clippy -D warnings, fmt and typos clean. Refs lance-format#333. 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.6, backlog O2/O3). ## What `docs/operations/maintenance-runbook.md`: symptom-driven guide for WAL merge / compaction / index / repair scheduling. Sections: where to look (table of question → endpoint/metric/key), backlog growing, deciding whether a running execution is stuck, demand but nothing scheduled (seven admission reasons, cheapest first), compaction never commits on a hot table, executor died holding ownership, draining, known gaps. Leading rule: **progress decides whether something is stuck; leases, heartbeats, Running state, retry countdowns, Ready pods and elapsed time do not.** Recovery is fenced and automatic; never delete ownership keys by hand. Uses the metrics from #339 and the typed `/merge-progress` from #341. All numeric claims (15 s retry loop, 15-failure attention, 120 s fence, 30 s lease TTL, 3-failure cooldown, 256/4096 thresholds) verified against source. Docs only; `typos` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
17 of 18 tasks
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.
Part of #333 (P0.5, backlog C5).
Problem
GET /api/v1/merge-progress(#330) did four independent etcd reads — execution, progress, work report, execution again — and returnedprogress_changed_retry/execution_changed_retrywhen they disagreed, leaving the caller to retry. The body wasserde_json::json!with no schema.Approach
Coordinator::progress_snapshot(target): read the execution id, then one txn guarded on the execution bytes that fetches progress and work report together. All three fields come from a single revision, exposed asrevision. Bounded to three attempts. A work report whosesequencetrails the current progress sequence is dropped instead of being shown next to a newer heartbeat.MergeProgressReport(serde struct) withstatus ∈ {reported, progress_details_unavailable, progress_unknown, no_active_record}. The two*_retrystatuses are removed. Absent fields are omitted, so consumers reading onlystatus/task_idsee no change.docs/merge-work-progress.mdupdated with the schema and single-revision guarantee.Alternatives
Reading all keys by target in one range: progress and work keys are per execution id, not per target, so a first read is unavoidable. Using
WithRevisionon separate gets: works, but the guarded txn also detects an execution swap for free.Validation
lance-context-merge:detailed_work_keeps_legacy_progress_cas_…extended — snapshot agrees with individual reads, stale report dropped after a heartbeat, absent target →None.lance-context-master:merge_progress_never_calls_a_live_legacy_task_healthyasserts on the typed struct and its serialised form.-D warnings, fmt, typos clean.🤖 Generated with Claude Code