[Fix-18644][Registry] Reconcile HA roles using instance ownership - #18645
michaellx1057 wants to merge 5 commits into
Conversation
18a0527 to
fee692b
Compare
|
Will there be any issues if the service restarts quickly and assumes the role of the previous leader? |
Thanks for asking. Yes—in our reproduction, the restarted Master became ACTIVE without acquiring ownership of the existing coordinator selector. That selector still belonged to the previous process's etcd lease and was subsequently removed. A matching address does not transfer the old process's lease to the new process. Deployment and operational contextWe run a self-hosted DolphinScheduler 3.4.2 deployment with:
We encountered scheduling problems during Master maintenance, restarts and replica-count changes. The controlled reproduction kept DS at 3.4.2 throughout; it did not involve a mixed-version upgrade. We temporarily reduced the deployment to 1 Master afterward. The proposed HA patch has not been deployed to this environment. Restarting a Master at the same addressIn a separate controlled run on 2026-09-16 (all times below are UTC+8):
The recreated Master 0 became ACTIVE while the coordinator selector was still attached to the predecessor's lease. Both Masters logged ACTIVE and started their coordinators: The etcd watch recorded deletion of the old selector, followed by Master 1 writing a new selector. Selected decoded fields: More than 7 minutes later, both coordinators were still processing the same workflow instance: There are recurring entries between these points, so this was sustained coordinator overlap. These logs do not establish duplicate business writes. The overlap also involved the etcd REMOVE payload issue and failure to demote after losing an election; address reuse was the initial ownership problem, not the entire failure chain. Related scale-down observationIn an earlier round, we scaled from 2 Masters to 1. This exposed the separate REMOVE-event problem tracked in #18640/#18641; it is not the same experiment as the restart above. Before recovery, the database snapshot showed an unassigned workflow waiting to run: The remaining Master logged: We initiated a restart of the remaining Master at approximately 15:00:47, using the same DS 3.4.2 image. The delayed instance then completed: Database readback confirmed instance 111167 was SUCCESS at 15:03:15.105, and the command queue was empty at 15:04:21. Instance 111168 also completed at 15:03:24.923. Their elapsed times were approximately 195 and 85 seconds, compared with about 5 seconds for adjacent executions. These durations include the recovery process. This screenshot shows the final SUCCESS records after recovery. The fault-time SERIAL_WAIT state is established by the database snapshot; we did not capture a UI screenshot at that point. Supplementary MySQL checkWe also tested the original HA implementation with a real MySQL 8.0.46 Registry. The successor became ACTIVE, but selector row 1 still belonged to the predecessor client, with its value and update time unchanged. This was an isolated fixture deliberately retaining the predecessor, not a production JDBC restart. This is why I think the new process should acquire its own selector rather than adopt the predecessor's role based on its address. Adding an instance timestamp addresses that ownership ambiguity. Handling a lost election and stopping the old coordinator are separate parts of the fix. |
|
While checking the demotion and re-election path, I found a related lifecycle issue in the existing Master coordinators. The exception → standby → retry path in this PR can exercise that sequence, so I have pushed a separate follow-up commit (3bb22b2) rather than treating the role-transition tests as sufficient. The change keeps the existing dedicated-thread structure. Each run has its own stop flag. If the previous run is still finishing, The added tests use real coordinators with deliberately blocked DAO/handler calls, including repeated role changes through This addresses overlap within the same coordinator instance. It does not cancel an already-issued database operation or provide fencing between different Masters. If the old database call never returns, its replacement remains pending rather than running concurrently. |
|
I tested a smaller candidate to check which changes are needed for this fix. 7fab651 now narrows this PR to the instance timestamp, ownership-based role reconciliation, election/role-update synchronization, and the coordinator thread handoff protection from 3bb22b2. It removes immediate demotion on election exceptions, the One clarification to my earlier coordinator comment: the overlap does not require the exception → standby → retry path. Without the handoff protection, tests also reproduced it during normal loss and reacquisition of leadership. A blocked DAO call was allowed to return after reactivation, and both the old and new threads continued polling. This is why I am keeping the handoff protection while removing the broader lifecycle changes. ValidationThe combined candidate passed 26 targeted tests on JDK 8. The final upstream revision was also clean-built through the 132-module Master/Alert reactor on actual Zulu JDK 8 We also tested a 3.4.2 backport in the same Kubernetes / etcd deployment described above, including a leader restart at the same address and scaling from 2 Masters to 1. The backport also covers the serial-coordinator restartability fix already present in dev (#18623). It is not a deployment of dev jars into 3.4.2. Selected fields from the validation logs follow. Times are UTC+8 on 2026-09-16; thread-dump details are omitted for readability. The thread lists contain only the serial and task-group coordinators. The rebuilt Master-0 remained standby while Master-1 owned the selector. After Master-1 was removed, Master-0 acquired the selector and started both coordinators. Natural scheduled executions also completed successfully. For example, selected database fields show: The error-command count remained at its baseline of 12 throughout the recorded checkpoints. The original image and single-Master topology were restored afterward. An earlier attempt restored the original deployment before completing the restart check because the test's 90-second wait was shorter than the configured 120-second termination grace period. The results above are from the subsequent run with the corrected timeout. These observations support the reduced scope for the reported restart and scale-down scenarios. The snapshots do not prove that no transient overlap occurred between observations, and the patch does not provide cross-Master fencing. The separate etcd REMOVE-value fix remains tracked in #18641. |
|
The current code looks too messy and feels overly AI-driven, making it difficult to tell exactly what problem it’s meant to solve. |
@ruanwenjun Thanks for the suggestions. I've updated the implementation to use a single election worker and simplified the lifecycle logic. Demotion and normal Master shutdown now share the same cleanup path: both coordinators are asked to stop before we wait for either worker to finish. |
|
I've updated the PR description to match the current implementation and completed another deployment test of I used a minimal backport to our DolphinScheduler 3.4.2 deployment, running Java 8, Kubernetes 1.33.13 and a 3-member etcd 3.6.14 cluster. The sequence was 1→3 Masters, restarting the elected Master at the same address, then scaling back to 1. The restart reached the retained-selector window. Selected observations from September 17, UTC+8: The recreated process did not inherit its predecessor’s role. Post-restart thread dumps showed both coordinator workers only on master-1. After scale-down, master-0 became the owner and its slots converged 3→2→1 without restarting that surviving Pod. Three SERIAL_WAIT canary instances completed at each stage: 9 workflow runs / 27 Shell tasks, with one actual output marker per task log. The original image and complete StatefulSet configuration were restored afterward. This supports the tested maintenance sequence. Complete live Java shutdown completion was not captured; controlled-DAO tests cover the coordinator shutdown interleavings separately. This was not a partition or all-backend test. One separate issue remained visible: the broad member-prefix subscription also delivers selector events to the heartbeat parser. With the restored DELETE payload, selector REMOVE logs a caught parse error. Member removal and election still completed in this run. |
f4c272e to
3afd40f
Compare
Run ownership reconciliation on a single election worker. Request both coordinator workers to stop before joining either on demotion and normal Master shutdown. Add deterministic lifecycle and shutdown regression coverage.
3afd40f to
3753a84
Compare

Was this PR generated or assisted by AI?
YES — Codex assisted with source analysis, implementation, regression tests and this description. The contributor reviewed the changes and validation evidence.
Purpose of the pull request
Closes #18644.
A restarted HA server can adopt its predecessor’s selector because the address is unchanged. A server that loses ownership can also retain its ACTIVE role. This can leave multiple Masters running their coordinators.
This PR distinguishes server instances, reconciles roles with current selector ownership, and prevents a coordinator from restarting before its previous worker has finished.
This follows the identity-reuse discussion in #18624. The serial-coordinator restart fix #18623 and the etcd REMOVE-value fix #18641 address related but separate parts of the behavior.
Brief change log
serverIdentify.Registry paths and the Registry SPI are unchanged. No new lifecycle enum is introduced.
Verification
Current revision:
3afd40fTargeted tests passed on JDK 8
1.8.0_504: 47 cases in total, with no failures, errors or skips:The concurrency tests use controlled DAO calls to exercise shutdown and restart while a worker is still busy. They cover specific interleavings; they are not a guarantee against every possible race. The full repository test suite was not run.
Controlled deployment verification
A minimal backport of
3afd40f, together with #18641 (b47fc0b), was tested on DolphinScheduler 3.4.2 with Java 8, Kubernetes 1.33.13 and a 3-member etcd 3.6.14 cluster. This used version-appropriate 3.4.2 changes, not dev JARs deployed into 3.4.2.The sequence was:
The recreated Master remained STAND_BY while etcd still contained its predecessor’s selector. At the recorded post-restart checkpoints, both coordinator workers were present only on the elected Master.
After scale-down, membership slots converged from 3 to 1 without restarting the surviving Pod. Three SERIAL_WAIT canary instances were run at each stage: 9 workflow runs and 27 Shell tasks completed successfully, with one actual output marker per task log.
The original image and complete StatefulSet configuration were restored afterward.
Complete live Java shutdown completion was not captured. Controlled-DAO tests provide separate coverage for the coordinator shutdown sequence. Network partitions, production deployments with other Registry backends, the full Alert lifecycle and long-soak behavior were not tested.
Earlier evidence
The earlier reproduction and validation comments remain useful historical context:
Those comments describe earlier revisions. The implementation and current validation are summarized above.
Compatibility and limits
Pull Request Notice
Pull Request Notice