You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ratis.apache.org by GitBox <gi...@apache.org> on 2020/11/03 17:44:33 UTC
[GitHub] [incubator-ratis] szetszwo opened a new pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
szetszwo opened a new pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252
See https://issues.apache.org/jira/browse/RATIS-1112
----------------------------------------------------------------
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
[GitHub] [incubator-ratis] runzhiwang commented on a change in pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252#discussion_r517348198
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java
##########
@@ -88,6 +93,20 @@ void stopRunning() {
this.isRunning = false;
}
+ boolean lostMajorityHeartbeats() {
Review comment:
how about rename to `isTimeoutSinceLostMajorityHeartbeats` ?
----------------------------------------------------------------
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
[GitHub] [incubator-ratis] runzhiwang commented on a change in pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252#discussion_r517386278
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java
##########
@@ -88,6 +93,20 @@ void stopRunning() {
this.isRunning = false;
}
+ boolean lostMajorityHeartbeats() {
Review comment:
Okay, `lostMajorityHeartbeatsRecently` is better.
----------------------------------------------------------------
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
[GitHub] [incubator-ratis] szetszwo commented on a change in pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252#discussion_r517400181
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java
##########
@@ -88,6 +93,20 @@ void stopRunning() {
this.isRunning = false;
}
+ boolean lostMajorityHeartbeats() {
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo merged pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252
----------------------------------------------------------------
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
[GitHub] [incubator-ratis] runzhiwang commented on a change in pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252#discussion_r517348198
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java
##########
@@ -88,6 +93,20 @@ void stopRunning() {
this.isRunning = false;
}
+ boolean lostMajorityHeartbeats() {
Review comment:
how about rename to `isTimeoutSinceLostMajorityHeartbeats` and return elapsed.compareTo(waitTime) >= 0?
----------------------------------------------------------------
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
[GitHub] [incubator-ratis] runzhiwang commented on a change in pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252#discussion_r517180695
##########
File path: ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java
##########
@@ -96,6 +96,27 @@ public void testChangeLeader() throws Exception {
cluster.shutdown();
}
+ @Test
+ public void testLostMajorityHeartbeats() throws Exception {
+ runWithNewCluster(3, this::runTestLostMajorityHeartbeats);
+ }
+
+ void runTestLostMajorityHeartbeats(CLUSTER cluster) throws Exception {
+ final RaftServerImpl leader = waitForLeader(cluster);
+ try {
+ isolate(cluster, leader.getId());
+ Thread.sleep(leader.getMaxTimeoutMs());
+ Thread.sleep(leader.getMaxTimeoutMs());
Review comment:
`Thread.sleep(leader.getMaxTimeoutMs())` duplicated call ?
----------------------------------------------------------------
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
[GitHub] [incubator-ratis] szetszwo commented on a change in pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252#discussion_r517384200
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java
##########
@@ -88,6 +93,20 @@ void stopRunning() {
this.isRunning = false;
}
+ boolean lostMajorityHeartbeats() {
Review comment:
I got that you wanted to invert the boolean. However, "isTimeoutSinceLostMajorityHeartbeats" is hard to understand. So, let's keep "lostMajorityHeartbeats"? Or I can change it to ""lostMajorityHeartbeatsRecently".
----------------------------------------------------------------
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
[GitHub] [incubator-ratis] szetszwo commented on a change in pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252#discussion_r517231573
##########
File path: ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java
##########
@@ -96,6 +96,27 @@ public void testChangeLeader() throws Exception {
cluster.shutdown();
}
+ @Test
+ public void testLostMajorityHeartbeats() throws Exception {
+ runWithNewCluster(3, this::runTestLostMajorityHeartbeats);
+ }
+
+ void runTestLostMajorityHeartbeats(CLUSTER cluster) throws Exception {
+ final RaftServerImpl leader = waitForLeader(cluster);
+ try {
+ isolate(cluster, leader.getId());
+ Thread.sleep(leader.getMaxTimeoutMs());
+ Thread.sleep(leader.getMaxTimeoutMs());
Review comment:
Yes. The test may fail if it only sleeps once.
----------------------------------------------------------------
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
[GitHub] [incubator-ratis] runzhiwang commented on a change in pull request #252: RATIS-1112. Ensure a node doesn't get reelected as a leader if it vol…
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #252:
URL: https://github.com/apache/incubator-ratis/pull/252#discussion_r517348198
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java
##########
@@ -88,6 +93,20 @@ void stopRunning() {
this.isRunning = false;
}
+ boolean lostMajorityHeartbeats() {
Review comment:
how about rename to `isTimeoutSinceLostMajorityHeartbeats` and `return elapsed.compareTo(waitTime) >= 0`?
----------------------------------------------------------------
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