[GitHub] [helix] xyuanlu commented on a diff in pull request #2558: Metaclient leader election - basics

["xyuanlu (via GitHub)" <gi...@apache.org>]

xyuanlu commented on code in PR #2558:
URL: https://github.com/apache/helix/pull/2558#discussion_r1266099720


##########
meta-client/src/main/java/org/apache/helix/metaclient/datamodel/DataRecord.java:
##########
@@ -35,4 +36,8 @@ public DataRecord(String znodeId) {
   public DataRecord(ZNRecord record) {
     super(record);
   }
+
+  public DataRecord(DataRecord record, String id) {

Review Comment:
   `ZNRecord` format is actually pretty generic format. Only the name indicating it is for ZK. We would like to still keep the format and use another name. 
   
   Having DataRecord extends ZNRecord would achieve this goal.



##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/leaderelection/LeaderElectionClient.java:
##########
@@ -53,36 +80,49 @@ public class LeaderElectionClient {
    * @param metaClientConfig The config used to create an metaclient.
    */
   public LeaderElectionClient(MetaClientConfig metaClientConfig, String participant) {
-
+    this._participant = participant;
+    if (metaClientConfig == null) {
+      throw new IllegalArgumentException("MetaClientConfig cannot be null.");
+    }
+    LOG.info("Creating MetaClient for LeaderElectionClient");
+    if (MetaClientConfig.StoreType.ZOOKEEPER.equals(metaClientConfig.getStoreType())) {

Review Comment:
   The store type in config is designed to be used to determine witch underlying storage user specified. 



##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/leaderelection/LeaderElectionClient.java:
##########
@@ -91,10 +131,46 @@ public boolean joinLeaderElectionParticipantPool(String leaderPath) {
    *
    * @param leaderPath The path for leader election.
    * @param userInfo Any additional information to associate with this participant.
-   * @return boolean indicating if the operation is succeeded.
+   * @throws RuntimeException if the operation is not succeeded.
    */
-  public boolean joinLeaderElectionParticipantPool(String leaderPath, Object userInfo) {
-    return false;
+  public void joinLeaderElectionParticipantPool(String leaderPath, LeaderInfo userInfo) {
+    // TODO: create participant entry with info
+    subscribeAndTryCreateLeaderEntry(leaderPath);
+  }
+
+  private void subscribeAndTryCreateLeaderEntry(String leaderPath) {
+    _metaClient.subscribeDataChange(leaderPath + LEADER_ENTRY_KEY, _reElectListener, false);
+    LeaderInfo leaderInfo = new LeaderInfo(LEADER_ENTRY_KEY);
+    leaderInfo.setLeaderName(_participant);
+
+    try {
+      // try to create leader entry, assuming leader election group node is already there
+      _metaClient.create(leaderPath + LEADER_ENTRY_KEY, leaderInfo, MetaClientInterface.EntryMode.EPHEMERAL);
+    } catch (MetaClientNodeExistsException ex) {
+      LOG.info("Already a leader for group {}" , leaderPath);
+    } catch (MetaClientNoNodeException ex) {
+      try {
+        // try to create leader path root entry
+        _metaClient.create(leaderPath, null);

Review Comment:
   The ephemeral node representing leader would have node path of `/parent_path/user_passed_leader_election_group_name/LEADER`. 
   
   `_metaClient.create(leaderPath + LEADER_ENTRY_KEY, leaderInfo, MetaClientInterface.EntryMode.EPHEMERAL);`
   try to create `/parent_path/user_passed_leader_election_group_name/LEADER. ` directly assuming `/parent_path/user_passed_leader_election_group_name/` is already there.
   
   If we get `MetaClientNoNodeException`, meaning `/parent_path/user_passed_leader_election_group_name/` is not there so we create one. This is what the second `create` for.
   
   If we still get `MetaClientNoNodeException`, meaning `/parent_path/' is not created, so we throw exception.
   
   I am not sure if transaction would help here as logically creating `/parent_path/user_passed_leader_election_group_name/` and `/parent_path/user_passed_leader_election_group_name/LEADER` should not be all or nothing operation. 



-- 
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: reviews-unsubscribe@helix.apache.org

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