zookeeper-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [zookeeper] insumity commented on a change in pull request #1081: ZOOKEEPER-3537: Leader election - Use of out of election messages
Date Tue, 24 Sep 2019 20:37:37 GMT
insumity commented on a change in pull request #1081: ZOOKEEPER-3537: Leader election - Use
of out of election messages
URL: https://github.com/apache/zookeeper/pull/1081#discussion_r327822590
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FastLeaderElection.java
 ##########
 @@ -1032,9 +1032,9 @@ public Vote lookForLeader() throws InterruptedException {
                          * together.
                          */
                         if (n.electionEpoch == logicalclock.get()) {
-                            recvset.put(n.sid, new Vote(n.leader, n.zxid, n.electionEpoch,
n.peerEpoch));
+                            recvset.put(n.sid, new Vote(n.leader, n.zxid, n.electionEpoch,
n.peerEpoch, n.state));
                             voteSet = getVoteTracker(recvset, new Vote(n.version, n.leader,
n.zxid, n.electionEpoch, n.peerEpoch, n.state));
-                            if (voteSet.hasAllQuorums() && checkLeader(outofelection,
n.leader, n.electionEpoch)) {
+                            if (voteSet.hasAllQuorums() && checkLeader(recvset, n.leader,
n.electionEpoch)) {
 
 Review comment:
   @lvfangmin in the [general case](https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FastLeaderElection.java#L999)
(i.e., received notification is in a `LOOKING` state), `n.state` is not needed. 
   
   However, note that we add `n.state` in the case that `FOLLOWING` or `LEADING` notifications
are [received](https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FastLeaderElection.java#L1029).
Assume that in such a scenario a notification from the leader is received and hence `n.state
== LEADING`. If we do not add `n.state` in this case, `n.state` will be the default value
of `LOOKING`. The problem with this is that when [`checkLeader`](https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FastLeaderElection.java#L1037)
is called, and the state of the leader vote is subsequently [checked](https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FastLeaderElection.java#L793)
it will not be in a `LEADING` state but it will be in a `LOOKING` state. Therefore, the `checkLeader`
test will fail.
   
   This was not a problem before because `checkLeader` was called in the `outofelection` set
and the votes added in `outofelection` contained `n.state` as can be seen [here](https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FastLeaderElection.java#L1049).

----------------------------------------------------------------
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


With regards,
Apache Git Services

Mime
View raw message