[GitHub] [ozone] sodonnel commented on a change in pull request #2148: HDDS-5090. make Decommission work under SCM HA.

[GitBox <gi...@apache.org>]

sodonnel commented on a change in pull request #2148:
URL: https://github.com/apache/ozone/pull/2148#discussion_r611667522



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -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.

Review comment:
       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 `processHeartBeat` with the correct DatanodeDetails to trigger a command.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org