szetszwo commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1513554496


##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -554,6 +561,6 @@ message GroupInfoReplyProto {
   RaftGroupProto group = 2;
   RoleInfoProto role = 3;
   bool isRaftStorageHealthy = 4;
-  repeated CommitInfoProto commitInfos = 5;
+  repeated PeerInfoProto peerInfos = 5;

Review Comment:
   Add the new proto at the end.
   ```
   message GroupInfoReplyProto {
     RaftRpcReplyProto rpcReply = 1;
     RaftGroupProto group = 2;
     RoleInfoProto role = 3;
     bool isRaftStorageHealthy = 4;
     repeated CommitInfoProto commitInfos = 5;
     RaftConfigurationProto conf = 6;
     LogInfoProto logInfo = 7;
   }
   ```



##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -175,6 +175,13 @@ message CommitInfoProto {
   uint64 commitIndex = 2;
 }
 
+message PeerInfoProto {
+  CommitInfoProto commitInfo = 1;
+  uint64 appliedIndex = 2;
+  uint64 currentTerm = 3;
+  uint64 snapshotIndex = 4;
+}

Review Comment:
   Since the word `PeerInfo` is already used, let's call it `LogInfoProto`.  
BTW, let's use `TermIndexProto` instead of `uint64`.  Then, everything will 
have a term.
   ```
   message LogInfoProto {
     TermIndexProto lastSnapshot = 1;
     TermIndexProto applied = 2;
     TermIndexProto committed = 3;
     TermIndexProto lastEntry = 4;
   }
   ```



##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -418,7 +425,7 @@ message RaftClientReplyProto {
   }
 
   uint64 logIndex = 14; // When the request is a write request and the reply 
is success, the log index of the transaction
-  repeated CommitInfoProto commitInfos = 15;
+  repeated PeerInfoProto peerInfos = 15;

Review Comment:
   We should not rename the proto.  Otherwise, it is an incompatible change.
   
   BTW, we should change only `GroupInfoReplyProto` and leave other protos 
unchanged.



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

Reply via email to