pkuwm commented on a change in pull request #1066:
URL: https://github.com/apache/helix/pull/1066#discussion_r443106477



##########
File path: 
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -2114,13 +2088,43 @@ private String getHexSessionId() {
   }
 
   /*
-   * Session aware operation needs below requirements:
-   * 1. the session id is NOT null or empty
-   * 2. create mode is EPHEMERAL or EPHEMERAL_SEQUENTIAL
+   * Gets the zookeeper instance that ensures its session ID matches the 
expected session ID.
+   * It is used for write operations that suppose the znode to be created by 
the expected session.
    */
-  private boolean isSessionAwareOperation(String expectedSessionId, CreateMode 
mode) {
-    return expectedSessionId != null && !expectedSessionId.isEmpty() && (
-        mode == CreateMode.EPHEMERAL || mode == 
CreateMode.EPHEMERAL_SEQUENTIAL);
+  private ZooKeeper getExpectedZookeeper(final String expectedSessionId) {
+    ZooKeeper zk = ((ZkConnection) getConnection()).getZookeeper();
+
+    if (expectedSessionId == null || expectedSessionId.isEmpty()) {
+      return zk;
+    }
+
+    /*
+     * 1. If operation is session aware, we have to check whether or not the
+     * passed-in(expected) session id matches actual session's id.
+     * If not, znode creation is failed. This validation is
+     * critical to guarantee the znode is created by the expected ZK session.
+     *
+     * 2. Otherwise, the operation is NOT session aware.
+     * In this case, we will use the actual zookeeper session to create the 
node.
+     */
+    acquireEventLock();
+    try {
+      final String actualSessionId = Long.toHexString(zk.getSessionId());
+      if (!actualSessionId.equals(expectedSessionId)) {
+        throw new ZkSessionMismatchedException(
+            "Failed to get expected zookeeper instance! There is a session id 
mismatch. Expected: "
+                + expectedSessionId + ". Actual: " + actualSessionId);
+      }
+
+      /*
+       * Cache the zookeeper reference and make sure later zooKeeper.create() 
is being run
+       * under this zookeeper connection. This is to avoid locking 
zooKeeper.create() which
+       * may cause potential performance issue.
+       */
+      return ((ZkConnection) getConnection()).getZookeeper();

Review comment:
       Thanks for confirming. Simplified 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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to