mxq-151 commented on code in PR #13404:
URL: 
https://github.com/apache/dolphinscheduler/pull/13404#discussion_r1106853954


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -237,14 +237,16 @@ public void notify(Event event) {
     private void updateMasterNodes() {
 
         String nodeLock = Constants.REGISTRY_DOLPHINSCHEDULER_LOCK_MASTERS;
+
         try {
-            registryClient.getLock(nodeLock);
-            currentSlot = 0;
-            totalSlot = 0;
-            this.masterNodes.clear();
-            Collection<String> currentNodes = 
registryClient.getMasterNodesDirectly();
-            List<Server> masterNodeList = 
registryClient.getServerList(NodeType.MASTER);
-            syncMasterNodes(currentNodes, masterNodeList);
+            if (registryClient.getLock(nodeLock)) {
+                currentSlot = 0;
+                totalSlot = 0;
+                this.masterNodes.clear();
+                Collection<String> currentNodes = 
registryClient.getMasterNodesDirectly();

Review Comment:
   > When you get the master node failed, it's better to rollback these fields 
to the previous state. Otherwise, the logic will look like the below:
   > 
   > * When get lock failed, these fields are at the previous state.
   > * When get master nodes failed, these fields are not in the previous state.
   
   the logic should be this:
   > * When get lock failed, these fields are at the previous state.
   > * When get lock success, these fields will update .
   
   i don't get your words,can you make it clear?



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

Reply via email to