You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ratis.apache.org by GitBox <gi...@apache.org> on 2020/09/14 10:24:08 UTC

[GitHub] [incubator-ratis] mukul1987 opened a new pull request #199: RATIS-769. Ratis RaftGroupId should also include number of nodes in the log output.

mukul1987 opened a new pull request #199:
URL: https://github.com/apache/incubator-ratis/pull/199


   ## What changes were proposed in this pull request?
   This change adds the numbet of nodes in the group in the GroupMemberId toString
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/RATIS-769
   
   ## How was this patch tested?
   The output looks like s2@group-DC32E8D50A70@3
   
   


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

[GitHub] [incubator-ratis] szetszwo commented on pull request #199: RATIS-769. Ratis RaftGroupId should also include number of nodes in the log output.

Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #199:
URL: https://github.com/apache/incubator-ratis/pull/199#issuecomment-694400112


   Please don't include size in RaftGroupId or RaftMemberGroupId since the actual group size can be changed.  There is no way to update the "size" stored in RaftGroupId or RaftMemberGroupId.


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

[GitHub] [incubator-ratis] runzhiwang commented on pull request #199: RATIS-769. Ratis RaftGroupId should also include number of nodes in the log output.

Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #199:
URL: https://github.com/apache/incubator-ratis/pull/199#issuecomment-692070484


   +1, we can trigger the CI again.


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

[GitHub] [incubator-ratis] amaliujia commented on pull request #199: RATIS-769. Ratis RaftGroupId should also include number of nodes in the log output.

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #199:
URL: https://github.com/apache/incubator-ratis/pull/199#issuecomment-694397555


   +1
   
   LGTM


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

[GitHub] [incubator-ratis] amaliujia commented on a change in pull request #199: RATIS-769. Ratis RaftGroupId should also include number of nodes in the log output.

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #199:
URL: https://github.com/apache/incubator-ratis/pull/199#discussion_r488067419



##########
File path: ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupMemberId.java
##########
@@ -33,6 +37,12 @@ public static RaftGroupMemberId valueOf(RaftPeerId peerId, RaftGroupId groupId)
   private final RaftGroupId groupId;
   private final String name;
 
+  private RaftGroupMemberId(RaftPeerId peerId, RaftGroupId groupId, int numMembers) {
+    this.peerId = Objects.requireNonNull(peerId, "peerId == null");
+    this.groupId = Objects.requireNonNull(groupId, "groupId == null");
+    this.name = "p:" + peerId + "@g:" + groupId + "@s:" + numMembers;

Review comment:
       Nit: should `@n` be used for `numMembers`? `n` means number. What `s` stands for?




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

[GitHub] [incubator-ratis] mukul1987 closed pull request #199: RATIS-769. Ratis RaftGroupId should also include number of nodes in the log output.

Posted by GitBox <gi...@apache.org>.
mukul1987 closed pull request #199:
URL: https://github.com/apache/incubator-ratis/pull/199


   


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