[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #13404: [fix-13315] fix ZookeeperRegistry get lock failed

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

ruanwenjun commented on code in PR #13404:
URL: https://github.com/apache/dolphinscheduler/pull/13404#discussion_r1082148716


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/registry/ServerNodeManager.java:
##########
@@ -235,12 +235,13 @@ public void notify(Event event) {
     }
 
     private void updateMasterNodes() {
-        currentSlot = 0;
-        totalSlot = 0;
-        this.masterNodes.clear();
+
         String nodeLock = Constants.REGISTRY_DOLPHINSCHEDULER_LOCK_MASTERS;
         try {
             registryClient.getLock(nodeLock);
+            currentSlot = 0;

Review Comment:
   It's better to store the pre value of `currentSlot`, totalSlot, masterNodes, and rollback to the pre value when catch an exception.



##########
dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-zookeeper/src/main/java/org/apache/dolphinscheduler/plugin/registry/zookeeper/ZookeeperRegistry.java:
##########
@@ -238,7 +238,7 @@ public boolean acquireLock(String key) {
 
     @Override
     public boolean releaseLock(String key) {
-        if (null == threadLocalLockMap.get().get(key)) {
+        if (threadLocalLockMap.get() == null || null == threadLocalLockMap.get().get(key)) {

Review Comment:
   ```suggestion
           Map<String, InterProcessMutex> threadLocalMutexMap = threadLocalLockMap.get();
           if ( threadLocalMutexMap == null || threadLocalMutexMap.get(key) == null) {
   ```



-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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