You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by chen-hongqin <ch...@163.com> on 2012/03/08 10:25:31 UTC

two things about FastLeaderElection.java

hi, i am a deleloper from China, when i am reading code "FastLeaderElection.java", i am a bit puzzled.
First one :
i am thiking if this code  
return ((newEpoch > curEpoch) || ((newEpoch == curEpoch) && (newZxid > curZxid)) || ((newZxid == curZxid) && (newId > curId)));

can be replaced with 
return ((newEpoch > curEpoch) || ((newEpoch == curEpoch) && (newZxid > curZxid)) || ((newZxid == curZxid) && (newId > proposedLeader)));

i think when replaced, it may be running more efficiently.


Second one:
And when looking at this following part, i am still puzzled
 case LEADING:
      /*
       * Consider all notifications from the same epoch
       * together.
       */
      if (n.electionEpoch == logicalclock) {
       recvset.put(n.sid, new Vote(n.leader, n.zxid, n.electionEpoch,
         n.peerEpoch));
       if (termPredicate(recvset, new Vote(n.leader, n.zxid, n.electionEpoch,
         n.peerEpoch, n.state))
         && checkLeader(outofelection, n.leader, n.electionEpoch)) {
        self.setPeerState((n.leader == self.getId()) ? ServerState.LEADING
          : learningState());

        Vote endVote = new Vote(n.leader, n.zxid, n.peerEpoch);
        leaveInstance(endVote);
        return endVote;
       }
      }
i think outofelection have nothing to do in this part, is that so?

and can I replace it like this? 
checkLeader(recvset, n.leader, n.electionEpoch)) 


i hope to hear from you! Thank you!^_^






2012-03-08



chen-hongqin

Re: two things about FastLeaderElection.java

Posted by Flavio Junqueira <fp...@yahoo-inc.com>.
Hi Chen-hongqin, In general it is best to propose code changes using jiras. 

On Mar 8, 2012, at 10:25 AM, chen-hongqin wrote:

> hi, i am a deleloper from China, when i am reading code "FastLeaderElection.java", i am a bit puzzled.
> First one :
> i am thiking if this code  
> return ((newEpoch > curEpoch) || ((newEpoch == curEpoch) && (newZxid > curZxid)) || ((newZxid == curZxid) && (newId > curId)));
> 
> can be replaced with 
> return ((newEpoch > curEpoch) || ((newEpoch == curEpoch) && (newZxid > curZxid)) || ((newZxid == curZxid) && (newId > proposedLeader)));
> 
> i think when replaced, it may be running more efficiently.
> 

I don't see how it makes the election process more efficient. As presented, I don't see any significant gain here.

> 
> Second one:
> And when looking at this following part, i am still puzzled
> case LEADING:
>      /*
>       * Consider all notifications from the same epoch
>       * together.
>       */
>      if (n.electionEpoch == logicalclock) {
>       recvset.put(n.sid, new Vote(n.leader, n.zxid, n.electionEpoch,
>         n.peerEpoch));
>       if (termPredicate(recvset, new Vote(n.leader, n.zxid, n.electionEpoch,
>         n.peerEpoch, n.state))
>         && checkLeader(outofelection, n.leader, n.electionEpoch)) {
>        self.setPeerState((n.leader == self.getId()) ? ServerState.LEADING
>          : learningState());
> 
>        Vote endVote = new Vote(n.leader, n.zxid, n.peerEpoch);
>        leaveInstance(endVote);
>        return endVote;
>       }
>      }
> i think outofelection have nothing to do in this part, is that so?
> 

It does because the notification comes from a server that is leading and consequently out of election.

> and can I replace it like this? 
> checkLeader(recvset, n.leader, n.electionEpoch)) 
> 
> 

I don't think so, it sounds wrong to me.



-Flavio


flavio
junqueira
senior research scientist
 
fpj@yahoo-inc.com
direct +34 93-183-8828
 
avinguda diagonal 177, 8th floor, barcelona, 08018, es
phone (408) 349 3300    fax (408) 349 3301