You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/01/10 02:52:36 UTC

[GitHub] [helix] kaisun2000 commented on a change in pull request #677: Add method to wait and return established session's ID

kaisun2000 commented on a change in pull request #677: Add method to wait and return established session's ID
URL: https://github.com/apache/helix/pull/677#discussion_r365045275
 
 

 ##########
 File path: helix-core/src/main/java/org/apache/helix/manager/zk/client/HelixZkClient.java
 ##########
 @@ -182,6 +183,28 @@ void asyncSetData(final String path, Object datat, final int version,
   // ZK state control
   boolean waitUntilConnected(long time, TimeUnit timeUnit);
 
+  /**
+   * Waits for SyncConnected state and returns a valid session ID(non-zero). The implementation of
+   * this method should wait for SyncConnected state and ZK session to be established, and should
+   * guarantee the established session's ID is returned before keeper state changes.
+   *
+   * Please note: this default implementation may have race condition issue and return an unexpected
+   * session ID that is zero or another new session's ID. The default implementation is for backward
+   * compatibility purpose.
+   *
+   * @param timeout Max waiting time for connecting to ZK server.
+   * @param timeUnit Time unit for the timeout.
+   * @return A valid ZK session ID which is non-zero.
+   */
+  default long waitForEstablishedSession(long timeout, TimeUnit timeUnit) {
+    if (!waitUntilConnected(timeout, timeUnit)) {
+      throw new ZkTimeoutException(
+          "Failed to get established session because connecting to ZK server has timed out in "
+              + timeout + " " + timeUnit);
+    }
+    return getSessionId();
+  }
+
 
 Review comment:
   Why we need this default implementation that still have race condition?  Why default can't be the correct one? Or the default implementation just throw exception telling people they should implement the correct version?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org