Repository navigation
fix: preserve merge completion outcomes across lease cleanup - #310
Merged
Merged
Conversation
This was referenced Oct 5, 2026
beinan
added a commit
that referenced
this pull request
Oct 5, 2026
Master shutdown currently drains HTTP but can drop detached scheduler tasks while they are still writing. Add executor-local admission tracking before claim RPCs and keep each reservation until its task finishes. An identity-checked admin drain endpoint stops new local claims, scans and catch-up controller operations; SIGTERM/SIGINT uses the same gate and waits for admitted work before runtime exit. Other masters continue consuming the durable queue. Regression coverage checks in-flight reservations, late subscribers, repeated drain, wrong process identity, a held worker merge finishing normally, and a second master completing work left queued. Validation: 142 master tests passed with real etcd. After the final HTTP identity/controller additions, all six drain-related regressions passed from a fresh build verified to contain the final branch tests; Clippy also passed. Existing progress watchdogs/fences are unchanged; drain does not impose a writer cancellation timeout. The operator guide specifies exact-Pod/identity polling and Kubernetes grace-period limits. This protocol only exists in new images: it cannot establish that an old auxiliary executor has drained. Dedicated one-table mode continues to use its supervisor's joined pass lifecycle. Isolated Azure integration validation also passed on staging candidate `975550f` combining #310–314: wrong executor identity returned 409, local drain held a newly queued task, a second master completed that task, and the drained master exited with code 0. Continuous ingestion/native publishing and compact/index maintenance completed beforehand; all 125 full payloads were verified and cgroup OOM/max events remained zero. This bounded functional test is not a production-size benchmark. Co-authored-by: Beinan Wang <>
beinan
added a commit
that referenced
this pull request
Oct 5, 2026
Persistent native catch-up ownership currently excludes Compact and IndexId even between successful merge passes. Add an explicit, default-empty target opt-in for publishers verified to use the common fenced task/target/merge-claim protocol. Immutable compact preparation may overlap a native pass; compact publication and index maintenance still acquire exclusive ownership. Every admission compares the exact dedicated identity, and a changed identity invalidates compaction promotion. Completion preserves the publisher owner. All 142 master tests passed with real etcd, including regressions for native/maintenance exclusion during publication, subsequent native drainage, stale-owner rejection, and legacy/generic/draining/wildcard protection. Clippy passed. Ordinary MergeWal and Repair do not receive this exception. Production enablement still requires complete writer coverage, including rejecting direct legacy worker merge HTTP calls for each opted-in target, followed by a production canary. The operator guide documents these requirements and the remaining bounded-wait/fairness limitation. No targets are enabled by default. Isolated Azure staging passed on candidate `975550f` combining #310–314, using the unchanged production supervisor with local etcd and test-owned active/operator records. During continuous ingestion, a native pass reclaimed 31 generations, compact completed 32 fragments into one, IndexId completed, and a subsequent native pass reclaimed 29 generations. Persistent ownership was unchanged, final pending was zero, and all 125 full payloads matched. The 4 CPU / 8 GiB test container recorded no cgroup OOM/max events. This verifies integration; it does not establish fairness under sustained saturation or production-sized throughput. Co-authored-by: Beinan Wang <>
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
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.
Dedicated merges can report a lease-revoke failure after publishing data, hiding whether the task-completion transaction committed and triggering an unnecessary failed pass. Preserve the completion transaction's committed, claim-lost, or unknown outcome independently of lease cleanup. Revoking an already absent lease is idempotent; failed history pruning after completion is also best-effort. Real claim loss and ambiguous completion responses remain errors.
Add task/target/lease-correlated completion diagnostics and bounded outcome counters. Regression coverage verifies that cleanup failures cannot replace the transaction result, and that an expired executor cannot finish or release its replacement's claim and table lock. Validation: all 141 master library tests passed with real etcd (including ignored HA tests); Clippy passed with warnings denied. The focused completion-outcome regression also passed independently.