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/04 02:09:41 UTC
[GitHub] [incubator-ratis] dengziming opened a new pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
dengziming opened a new pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241
## What changes were proposed in this pull request?
In the leader election approach, the candidate will change to leader in advance if the majority voters vote for it. we can also terminate the approach if majority voters reject it.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1120
## How was this patch tested?
unit test.
----------------------------------------------------------------
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 pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#issuecomment-721519997
@dengziming Thanks the first patch. I have merged it. Looking forward to your more contribution.
----------------------------------------------------------------
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 merged pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang merged pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241
----------------------------------------------------------------
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 #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#discussion_r517021344
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/PeerConfiguration.java
##########
@@ -94,6 +94,16 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
return num > size() / 2;
}
+ boolean majorityImpossibleWithout(Collection<RaftPeerId> rejected) {
Review comment:
Do you think rename majorityImpossibleWithout to majorityRejectVote maybe 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] runzhiwang commented on a change in pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#discussion_r517042861
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/PeerConfiguration.java
##########
@@ -94,6 +94,16 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
return num > size() / 2;
}
+ boolean majorityImpossibleWithout(Collection<RaftPeerId> rejected) {
Review comment:
If you have better name, I also can accept it.
----------------------------------------------------------------
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 pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#issuecomment-721471780
reopen pr to trigger ci.
----------------------------------------------------------------
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 closed pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang closed pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241
----------------------------------------------------------------
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 #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#discussion_r517024988
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfiguration.java
##########
@@ -206,6 +206,12 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
(oldConf == null || oldConf.hasMajority(others, selfId));
}
+ /** @return true if the rejects are in the half(impossible for others in majority). */
+ boolean majorityImpossibleWithout(Collection<RaftPeerId> rejects) {
+ return conf.majorityImpossibleWithout(rejects) &&
+ (oldConf == null || oldConf.majorityImpossibleWithout(rejects));
+ }
Review comment:
I think this check it not totally reverse of hasMajority. The reverse of hasMajority is `!conf.hasMajority(others, selfId) || !(oldConf == null || oldConf.hasMajority(others, selfId))` i.e.` !conf.hasMajority(others, selfId) || (oldConf != null && !oldConf.hasMajority(others, selfId))``. i.e. ` conf.majorityImpossibleWithout(rejects) || (oldConf != null && !oldConf.majorityImpossibleWithout(rejects))``
----------------------------------------------------------------
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 closed pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang closed pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241
----------------------------------------------------------------
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 closed pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang closed pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241
----------------------------------------------------------------
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] dengziming commented on a change in pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
dengziming commented on a change in pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#discussion_r517048881
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfiguration.java
##########
@@ -206,6 +206,12 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
(oldConf == null || oldConf.hasMajority(others, selfId));
}
+ /** @return true if the rejects are in the half(impossible for others in majority). */
+ boolean majorityImpossibleWithout(Collection<RaftPeerId> rejects) {
+ return conf.majorityImpossibleWithout(rejects) &&
+ (oldConf == null || oldConf.majorityImpossibleWithout(rejects));
+ }
Review comment:
This is right! I just forgot this.
----------------------------------------------------------------
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 closed pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang closed pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241
----------------------------------------------------------------
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 pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#issuecomment-719439401
@dengziming Thanks for the first PR. Please fill What changes were proposed in this pull request?, What is the link to the Apache JIRA?, How was this patch tested?.
----------------------------------------------------------------
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 #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#discussion_r517042861
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/PeerConfiguration.java
##########
@@ -94,6 +94,16 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
return num > size() / 2;
}
+ boolean majorityImpossibleWithout(Collection<RaftPeerId> rejected) {
Review comment:
You are right, so if you have better name, I also can accept it.
----------------------------------------------------------------
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] dengziming commented on a change in pull request #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
dengziming commented on a change in pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#discussion_r517042470
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/PeerConfiguration.java
##########
@@ -94,6 +94,16 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
return num > size() / 2;
}
+ boolean majorityImpossibleWithout(Collection<RaftPeerId> rejected) {
Review comment:
OK, firstly I assume rejected votes is not necessarily a majority, maybe half is enough. But I think majorityRejectVote is more self-explanatory even it is not very accurate.
----------------------------------------------------------------
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 #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#discussion_r517024988
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfiguration.java
##########
@@ -206,6 +206,12 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
(oldConf == null || oldConf.hasMajority(others, selfId));
}
+ /** @return true if the rejects are in the half(impossible for others in majority). */
+ boolean majorityImpossibleWithout(Collection<RaftPeerId> rejects) {
+ return conf.majorityImpossibleWithout(rejects) &&
+ (oldConf == null || oldConf.majorityImpossibleWithout(rejects));
+ }
Review comment:
I think this check it not totally reverse of hasMajority. The reverse of hasMajority is `!conf.hasMajority(others, selfId) || !(oldConf == null || oldConf.hasMajority(others, selfId))` i.e.` !conf.hasMajority(others, selfId) || (oldConf != null && !oldConf.hasMajority(others, selfId))`. i.e. ` conf.majorityImpossibleWithout(rejects) || (oldConf != null && oldConf.majorityImpossibleWithout(rejects))`
----------------------------------------------------------------
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 #241: RATIS-1120. Terminate election in advance if majority peers reject
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #241:
URL: https://github.com/apache/incubator-ratis/pull/241#discussion_r517024988
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfiguration.java
##########
@@ -206,6 +206,12 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
(oldConf == null || oldConf.hasMajority(others, selfId));
}
+ /** @return true if the rejects are in the half(impossible for others in majority). */
+ boolean majorityImpossibleWithout(Collection<RaftPeerId> rejects) {
+ return conf.majorityImpossibleWithout(rejects) &&
+ (oldConf == null || oldConf.majorityImpossibleWithout(rejects));
+ }
Review comment:
I think this check it not totally reverse of hasMajority. The reverse of hasMajority is `!conf.hasMajority(others, selfId) || !(oldConf == null || oldConf.hasMajority(others, selfId))` i.e.` !conf.hasMajority(others, selfId) || (oldConf != null && !oldConf.hasMajority(others, selfId))``. i.e. ` conf.majorityImpossibleWithout(rejects) || (oldConf != null && !oldConf.majorityImpossibleWithout(rejects))`
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfiguration.java
##########
@@ -206,6 +206,12 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
(oldConf == null || oldConf.hasMajority(others, selfId));
}
+ /** @return true if the rejects are in the half(impossible for others in majority). */
+ boolean majorityImpossibleWithout(Collection<RaftPeerId> rejects) {
+ return conf.majorityImpossibleWithout(rejects) &&
+ (oldConf == null || oldConf.majorityImpossibleWithout(rejects));
+ }
Review comment:
I think this check it not totally reverse of hasMajority. The reverse of hasMajority is `!conf.hasMajority(others, selfId) || !(oldConf == null || oldConf.hasMajority(others, selfId))` i.e.` !conf.hasMajority(others, selfId) || (oldConf != null && !oldConf.hasMajority(others, selfId))`. i.e. ` conf.majorityImpossibleWithout(rejects) || (oldConf != null && !oldConf.majorityImpossibleWithout(rejects))`
----------------------------------------------------------------
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