codings-dan commented on code in PR #673:
URL: https://github.com/apache/ratis/pull/673#discussion_r918886954
##########
ratis-common/src/main/java/org/apache/ratis/protocol/RaftPeer.java:
##########
@@ -133,10 +136,24 @@ public Builder setPriority(int priority) {
return this;
}
+ public Builder setStartupRole(RaftProtos.RaftPeerRole startupRole) {
+ if (startupRole != RaftProtos.RaftPeerRole.FOLLOWER
Review Comment:
ok, ignore my personal habits
##########
ratis-common/src/main/java/org/apache/ratis/util/ProtoUtils.java:
##########
@@ -118,6 +119,7 @@ static RaftPeer toRaftPeer(RaftPeerProto p) {
.setClientAddress(p.getClientAddress())
.setAdminAddress(p.getAdminAddress())
.setPriority(p.getPriority())
+ .setStartupRole(p.hasStartupRole() ? p.getStartupRole() :
RaftProtos.RaftPeerRole.FOLLOWER)
Review Comment:
Good Catch! I forget the forward compatibility
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java:
##########
@@ -951,9 +951,11 @@ private void yieldLeaderToHigherPriorityPeer() {
for (LogAppender logAppender : senders.getSenders()) {
final FollowerInfo followerInfo = logAppender.getFollower();
final RaftPeerId followerID = followerInfo.getPeer().getId();
- final RaftPeer follower = conf.getPeer(followerID);
+ final RaftPeer follower = conf.getPeer(followerID,
RaftPeerRole.FOLLOWER);
Review Comment:
But when designing this method parameter, we thought that the variable will
only be specified when the Listener is obtained. Also, this kind of call is a
lot in ratis, see below, so I think we should remove this change
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java:
##########
@@ -951,9 +951,11 @@ private void yieldLeaderToHigherPriorityPeer() {
for (LogAppender logAppender : senders.getSenders()) {
final FollowerInfo followerInfo = logAppender.getFollower();
final RaftPeerId followerID = followerInfo.getPeer().getId();
- final RaftPeer follower = conf.getPeer(followerID);
+ final RaftPeer follower = conf.getPeer(followerID,
RaftPeerRole.FOLLOWER);
if (follower == null) {
- LOG.error("{} the follower {} is not in the conf {}", this,
server.getId(), conf);
+ if (conf.getPeer(followerID, RaftPeerRole.LISTENER) == null) {
Review Comment:
Thanks for letting me know, I forgot about 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
