-
Notifications
You must be signed in to change notification settings - Fork 593
HDDS-5090. make Decommission work under SCM HA. #2148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -413,38 +413,56 @@ boolean opStateDiffers(DatanodeDetails dnDetails, NodeStatus nodeStatus) { | |
| } | ||
|
|
||
| /** | ||
| * If the operational state or expiry reported in the datanode heartbeat do | ||
| * not match those store in SCM, queue a command to update the state persisted | ||
| * on the datanode. Additionally, ensure the datanodeDetails stored in SCM | ||
| * match those reported in the heartbeat. | ||
| * This method should only be called when processing the | ||
| * heartbeat, and for a registered node, the information stored in SCM is the | ||
| * source of truth. | ||
| * This method should only be called when processing the heartbeat. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add a test for this new logic - Command fired if leader, but not fired if follower? I checked in TestSCMNodeManager, and there is a test "testSetNodeOpStateAndCommandFired" which I earlier set to ignore as it became invalid when I developed decommission. I then forgot to go back and fix it. You could use that tests as a starting point, and change it to call
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the hint. |
||
| * | ||
| * On leader SCM, for a registered node, the information stored in SCM is | ||
| * the source of truth. If the operational state or expiry reported in the | ||
| * datanode heartbeat do not match those store in SCM, queue a command to | ||
| * update the state persisted on the datanode. Additionally, ensure the | ||
| * datanodeDetails stored in SCM match those reported in the heartbeat. | ||
| * | ||
| * On follower SCM, datanode notifies follower SCM its latest operational | ||
| * state or expiry via heartbeat. If the operational state or expiry | ||
| * reported in the datanode heartbeat do not match those stored in SCM, | ||
| * just update the state in follower SCM accordingly. | ||
| * | ||
| * @param reportedDn The DatanodeDetails taken from the node heartbeat. | ||
| * @throws NodeNotFoundException | ||
| */ | ||
| protected void updateDatanodeOpState(DatanodeDetails reportedDn) | ||
| throws NodeNotFoundException { | ||
| NodeStatus scmStatus = getNodeStatus(reportedDn); | ||
| if (opStateDiffers(reportedDn, scmStatus)) { | ||
| LOG.info("Scheduling a command to update the operationalState " + | ||
| "persisted on {} as the reported value does not " + | ||
| "match the value stored in SCM ({}, {})", | ||
| reportedDn, | ||
| scmStatus.getOperationalState(), | ||
| scmStatus.getOpStateExpiryEpochSeconds()); | ||
| if (scmContext.isLeader()) { | ||
| LOG.info("Scheduling a command to update the operationalState " + | ||
| "persisted on {} as the reported value does not " + | ||
| "match the value stored in SCM ({}, {})", | ||
| reportedDn, | ||
| scmStatus.getOperationalState(), | ||
| scmStatus.getOpStateExpiryEpochSeconds()); | ||
|
|
||
| try { | ||
| SCMCommand<?> command = new SetNodeOperationalStateCommand( | ||
| Time.monotonicNow(), | ||
| try { | ||
| SCMCommand<?> command = new SetNodeOperationalStateCommand( | ||
| Time.monotonicNow(), | ||
| scmStatus.getOperationalState(), | ||
| scmStatus.getOpStateExpiryEpochSeconds()); | ||
| command.setTerm(scmContext.getTermOfLeader()); | ||
| addDatanodeCommand(reportedDn.getUuid(), command); | ||
| } catch (NotLeaderException nle) { | ||
| LOG.warn("Skip sending SetNodeOperationalStateCommand," | ||
| + " since current SCM is not leader.", nle); | ||
| return; | ||
| } | ||
| } else { | ||
| LOG.info("Update the operationalState saved in follower SCM " + | ||
| "for {} as the reported value does not " + | ||
| "match the value stored in SCM ({}, {})", | ||
| reportedDn, | ||
| scmStatus.getOperationalState(), | ||
| scmStatus.getOpStateExpiryEpochSeconds()); | ||
| command.setTerm(scmContext.getTermOfLeader()); | ||
| addDatanodeCommand(reportedDn.getUuid(), command); | ||
| } catch (NotLeaderException nle) { | ||
| LOG.warn("Skip sending SetNodeOperationalStateCommand," | ||
| + " since current SCM is not leader.", nle); | ||
| return; | ||
|
|
||
| setNodeOperationalState(reportedDn, reportedDn.getPersistedOpState(), | ||
| reportedDn.getPersistedOpStateExpiryEpochSec()); | ||
| } | ||
| } | ||
| DatanodeDetails scmDnd = nodeStateManager.getNode(reportedDn); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add a test for this - ie ensure that nodes are added to the decommission workflow when the onBecomeLeader() event is fired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done