qzsee opened a new pull request, #67322:
URL: https://github.com/apache/doris/pull/67322
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
**This caused two issues:**
1. Wrong guard flag. The block checked isDecommissioned() (the final
decommissioned state) instead of isDecommissioning() (the in-progress state).
As long as the BE had not yet reached the terminal DECOMMISSIONED state, every
scheduling round of
[CloudClusterChecker](copilot://navigate?keyword=CloudClusterChecker) would
re-enter this branch and call registerWaterShedTxnId(be.getId()) again. The
watershed txn id is only meaningful to be registered once per decommissioning
session; repeatedly registering it on every check interval produces redundant
RPCs/log entries and pollutes the upgrade manager's internal state.
2. Inconsistent state on RPC failure. be.setDecommissioning(true) was
executed after the try/catch block, meaning even if registerWaterShedTxnId
threw a UserException and the watershed txn id was never successfully
registered, the BE would still be marked as decommissioning=true. Subsequent
checks would then skip the retry path entirely, leaving the BE in an
inconsistent state where it is flagged as decommissioning but has no associated
watershed txn id.
**Fix**
Change the guard from `!be.isDecommissioned()` to `!be.isDecommissioning()`,
so the branch is only entered on the first transition into the decommissioning
state.
Move `be.setDecommissioning(true)` inside the try block, right after
registerWaterShedTxnId succeeds. This guarantees the in-memory decommissioning
flag is only flipped when the watershed txn id has been registered
successfully; if the RPC fails, the BE stays in its previous state and will be
retried on the next check round.
**Impact**
Eliminates repeated registerWaterShedTxnId calls for a BE that is stuck in
DECOMMISSIONING state.
Makes the decommissioning state transition atomic with respect to the
watershed txn id registration: either both succeed, or neither takes effect
(and the operation is retried next round).
No behavior change for BEs that have already reached
NODE_STATUS_DECOMMISSIONED.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
