You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/03/30 05:21:44 UTC
[GitHub] [iotdb] SzyWilliam opened a new pull request #5377: Add isLeaderOf to Consensus
SzyWilliam opened a new pull request #5377:
URL: https://github.com/apache/iotdb/pull/5377
## Description
In this PR, I add a new interface method `isLeaderOf` to IConsensus to determine whether current peer is the leader of a given consensus group.
Also, I fixed a bug related to previous transferLeader method
## Details
Ratis assign every peer a default priority 0. When transferring leadership, it won't allow any peer with (peer.priority <= currentLeader.priority) to become the new leader. Thus before call transferLeader, we should first upgrade newLeader's priority and degrade every others.
--
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
[GitHub] [iotdb] SzyWilliam commented on a change in pull request #5377: Add isLeaderOf to Consensus
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on a change in pull request #5377:
URL: https://github.com/apache/iotdb/pull/5377#discussion_r839485689
##########
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:
done
--
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
[GitHub] [iotdb] OneSizeFitsQuorum commented on a change in pull request #5377: Add isLeaderOf to Consensus
Posted by 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
[GitHub] [iotdb] SzyWilliam commented on a change in pull request #5377: Add isLeaderOf to Consensus
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on a change in pull request #5377:
URL: https://github.com/apache/iotdb/pull/5377#discussion_r839477354
##########
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:
done
--
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
[GitHub] [iotdb] OneSizeFitsQuorum merged pull request #5377: Add isLeaderOf to Consensus
Posted by GitBox <gi...@apache.org>.
OneSizeFitsQuorum merged pull request #5377:
URL: https://github.com/apache/iotdb/pull/5377
--
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
[GitHub] [iotdb] SzyWilliam commented on a change in pull request #5377: Add isLeaderOf to Consensus
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on a change in pull request #5377:
URL: https://github.com/apache/iotdb/pull/5377#discussion_r839477738
##########
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:
done
--
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
[GitHub] [iotdb] SzyWilliam commented on a change in pull request #5377: Add isLeaderOf to Consensus
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on a change in pull request #5377:
URL: https://github.com/apache/iotdb/pull/5377#discussion_r839482412
##########
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:
Change IP_PORT to ipAddress. Id is IP-Port and address is IP:Port, thus has a little difference. I use the slash instead of colon because the ratis won't allow a : appears in the naming scheme.
--
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