codings-dan commented on code in PR #673:
URL: https://github.com/apache/ratis/pull/673#discussion_r917716420
##########
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
+ && startupRole != RaftProtos.RaftPeerRole.LISTENER) {
+ throw new IllegalArgumentException(
+ "At startup the role can only be set to FOLLOWER or LISTENER, the
current value is " +
+ startupRole);
+ }
+ this.startupRole = startupRole;
+ return this;
+ }
+
public RaftPeer build() {
+ if (startupRole == null) {
Review Comment:
Please remove the if statement
##########
ratis-common/src/main/java/org/apache/ratis/protocol/RaftPeer.java:
##########
@@ -75,6 +77,7 @@ public static class Builder {
private String clientAddress;
private String dataStreamAddress;
private int priority;
+ private RaftProtos.RaftPeerRole startupRole;
Review Comment:
set `startupRole` defalut value `RaftProtos.RaftPeerRole.FOLLOWER`
##########
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:
There is no need to increase this variable, the default is to take FOLLOWER
##########
ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroup.java:
##########
@@ -75,6 +77,13 @@ public Collection<RaftPeer> getPeers() {
return peers.values();
}
+ public Collection<RaftPeer> getPeers(RaftProtos.RaftPeerRole role) {
+ return peers.values()
+ .stream()
+ .filter(peer -> peer.getStartupRole() == role)
Review Comment:
I think we can't use `startupRole` as the difference between raftpeer,
because the peer will change its identity
##########
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:
Why do you need to add this judgment here? In the election process, it seems
that listeners do not need to participate
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -28,6 +28,7 @@ message RaftPeerProto {
string dataStreamAddress = 4; // address of the data stream server
string clientAddress = 5; // address of the client RPC server
string adminAddress = 6; // address of the admin RPC server
+ optional RaftPeerRole startupRole=7; // peer start up role
Review Comment:
typo. `optional RaftPeerRole startupRole = 7`
##########
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:
Since the default value is set above, redundant judgment logic can be
deleted here
##########
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:
I think `!=` can be replaced by `!...equals` here
--
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]
