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



##########
File path: helix-core/src/main/java/org/apache/helix/HelixManager.java
##########
@@ -419,6 +420,17 @@ void 
addExternalViewChangeListener(org.apache.helix.ExternalViewChangeListener l
    */
   Long getSessionStartTime();
 
+  /**
+   * Checks whether the cluster manager is leader and returns the session ID 
associated to the
+   * connection of cluster data store, if and only if it is leader.
+   *
+   * @return {@code Optional<String>} session ID is present inside the {@code 
Optional} object
+   * if the cluster manager is leader. Otherwise, returns an empty {@code 
Optional} object.
+   */
+  default Optional<String> getSessionIdIfLead() {

Review comment:
       manager.getSessionIdIfLead checks whether the cluster manager is leader 
and returns the session ID associated to the connection of cluster data store, 
if and only if it is leader. It combines isLeader() and getSessionId() in one 
method to ensure the session id returning is what we check leadership in 
isLeader().
   
   Without it, we may do this:
   
   if (isLeader()) {
     sessionId = manager.getSessionId();
   }
   The problem is race condition. sessionId may change after isLeader().
   
   So we need manager.getSessionIdIfLead to check leadership and get sessionId 
at the same time.




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