[GitHub] [dubbo] CrazyHZM commented on a diff in pull request #11036: Fix zookeeper cas failed when create

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

CrazyHZM commented on code in PR #11036:
URL: https://github.com/apache/dubbo/pull/11036#discussion_r1033082143


##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/zookeeper/ZookeeperClient.java:
##########
@@ -33,7 +33,7 @@ public interface ZookeeperClient {
      * @param path path to ZNode
      * @param ephemeral specify create mode of ZNode creation. true - EPHEMERAL, false - PERSISTENT.
      */
-    void create(String path, boolean ephemeral);
+    void create(String path, boolean ephemeral, boolean faultTolarance);

Review Comment:
   Description of the addition of the `faultTolarance` parameter, other LGTM



##########
dubbo-remoting/dubbo-remoting-zookeeper/src/main/java/org/apache/dubbo/remoting/zookeeper/curator/CuratorZookeeperClient.java:
##########
@@ -119,60 +119,79 @@ public List<ACL> getAclForPath(String path) {
     }
 
     @Override
-    public void createPersistent(String path) {
+    public void createPersistent(String path, boolean faultTolerant) {
         try {
             client.create().forPath(path);
         } catch (NodeExistsException e) {
-            logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "ZNode " + path + " already exists.", e);
+            if (!faultTolerant) {
+                logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "ZNode " + path + " already exists.", e);
+                throw new IllegalStateException(e.getMessage(), e);
+            }
         } catch (Exception e) {
             throw new IllegalStateException(e.getMessage(), e);
         }
     }
 
     @Override
-    public void createEphemeral(String path) {
+    public void createEphemeral(String path, boolean faultTolerant) {
         try {
             client.create().withMode(CreateMode.EPHEMERAL).forPath(path);
         } catch (NodeExistsException e) {
-            logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "ZNode " + path + " already exists, since we will only try to recreate a node on a session expiration" +
-                ", this duplication might be caused by a delete delay from the zk server, which means the old expired session" +
-                " may still holds this ZNode and the server just hasn't got time to do the deletion. In this case, " +
-                "we can just try to delete and create again.", e);
-            deletePath(path);
-            createEphemeral(path);
+            if (faultTolerant) {
+                logger.info("ZNode " + path + " already exists, since we will only try to recreate a node on a session expiration" +
+                    ", this duplication might be caused by a delete delay from the zk server, which means the old expired session" +
+                    " may still holds this ZNode and the server just hasn't got time to do the deletion. In this case, " +
+                    "we can just try to delete and create again.");
+                deletePath(path);
+                createEphemeral(path, true);

Review Comment:
   There is no `createOrUpdate` function here, so why do you need to recreate it?



-- 
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: notifications-unsubscribe@dubbo.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org