[GitHub] [iotdb] OneSizeFitsQuorum commented on a change in pull request #5377: Add isLeaderOf to Consensus

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

OneSizeFitsQuorum commented on a change in pull request #5377:
URL: https://github.com/apache/iotdb/pull/5377#discussion_r839242445



##########
File path: consensus/src/main/java/org/apache/iotdb/consensus/IConsensus.java
##########
@@ -56,4 +56,6 @@
   ConsensusGenericResponse transferLeader(ConsensusGroupId groupId, Peer newLeader);
 
   ConsensusGenericResponse triggerSnapshot(ConsensusGroupId groupId);
+
+  boolean isLeaderOf(ConsensusGroupId groupId);

Review comment:
       Maybe just `isLeader`

##########
File path: consensus/src/main/java/org/apache/iotdb/consensus/standalone/StandAloneConsensus.java
##########
@@ -168,4 +168,9 @@ public ConsensusGenericResponse transferLeader(ConsensusGroupId groupId, Peer ne
   public ConsensusGenericResponse triggerSnapshot(ConsensusGroupId groupId) {
     return ConsensusGenericResponse.newBuilder().setSuccess(false).build();
   }
+
+  @Override
+  public boolean isLeaderOf(ConsensusGroupId groupId) {
+    return false;

Review comment:
       Maybe `True` as it only has one replica

##########
File path: consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java
##########
@@ -413,6 +439,20 @@ public ConsensusGenericResponse transferLeader(ConsensusGroupId groupId, Peer ne
     return ConsensusGenericResponse.newBuilder().setSuccess(reply.isSuccess()).build();
   }
 
+  @Override
+  public boolean isLeaderOf(ConsensusGroupId groupId) {
+    RaftGroupId raftGroupId = Utils.toRatisGroupId(groupId);
+
+    boolean isLeader;
+    try {
+      isLeader = server.getDivision(raftGroupId).getInfo().isLeader();
+    } catch (IOException exception) {
+      // if the query fails, simply return not leader

Review comment:
       Maybe report a log?

##########
File path: consensus/src/main/java/org/apache/iotdb/consensus/ratis/Utils.java
##########
@@ -69,13 +69,18 @@ public static String groupFullName(ConsensusGroupId consensusGroupId) {
     return String.format("%s-%d", groupTypeAbbr, consensusGroupId.getId());
   }
 
-  public static RaftPeer toRaftPeer(Endpoint endpoint) {
+  // priority is used as ordinal of leader election
+  public static RaftPeer toRaftPeer(Endpoint endpoint, int priority) {
     String Id = String.format("%s-%d", endpoint.getIp(), endpoint.getPort());
-    return RaftPeer.newBuilder().setId(Id).setAddress(IP_PORT(endpoint)).build();
+    return RaftPeer.newBuilder()
+        .setId(Id)
+        .setAddress(IP_PORT(endpoint))

Review comment:
       I suggest changing the name of this function `IP_PORT`...
   BTW, it seems that you can use `Id` for address directly?




-- 
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@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org