[GitHub] [incubator-ratis] juaby edited a comment on pull request #93: RATIS-931. Fixed Add a raft group to all peers currently registered

[GitBox <gi...@apache.org>]

juaby edited a comment on pull request #93:
URL: https://github.com/apache/incubator-ratis/pull/93#issuecomment-641847686


   > Can we rename the member `peers` to be `allKnownPeers` or similar? Something to try to prevent such an error in the future.
   > 
   > What about a test? Did you try to write a unit test to catch this? I think having 4 available workers, create a log, and then look at the raft groups and see that it has 4 members instead of 3 would work.
   
   Sorry, it took so long to reply to you. Regarding the log service, it is okay to verify according to the document (because the number of worker nodes is equal to the number of new raft groups), but when more than three worker nodes are started, a log is created At the time, the metadata service will broadcast the addition of Raft groups to unnecessary nodes outside the group. 
   Personal suggestion: Ratis server adds verification. When an additional raft group is received, verify that the new raft group members must contain the current working node


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