[GitHub] [helix] kaisun2000 commented on a change in pull request #1066: Prevent parallel controller pipelines run causing two master replicas

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

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



##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -2119,8 +2126,41 @@ private String getHexSessionId() {
    * 2. create mode is EPHEMERAL or EPHEMERAL_SEQUENTIAL
    */
   private boolean isSessionAwareOperation(String expectedSessionId, CreateMode mode) {
-    return expectedSessionId != null && !expectedSessionId.isEmpty() && (
-        mode == CreateMode.EPHEMERAL || mode == CreateMode.EPHEMERAL_SEQUENTIAL);
+    return expectedSessionId != null && !expectedSessionId.isEmpty() && mode.isEphemeral();
+  }
+
+  private ZooKeeper getExpectedZookeeper(final String expectedSessionId) {

Review comment:
       How do we reason this is correct? Note, previously, we have `acquireEventLock` when checking sessionId is equal to expected sessionId.
   ```
   if (isSessionAwareOperation(expectedSessionId, mode)) {
             acquireEventLock();
             try {
               final String actualSessionId = Long.toHexString(zooKeeper.getSessionId());
               if (!actualSessionId.equals(expectedSessionId)) {
                 throw new ZkSessionMismatchedException(
                     "Failed to create ephemeral node! 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.
                */
               zooKeeper = ((ZkConnection) getConnection()).getZookeeper();
   ```




----------------------------------------------------------------
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: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org