Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18077 )
Change subject: [tools] Add role filed when list remote replicas ...................................................................... Patch Set 6: (4 comments) Thank you for the contribution! Looks good, just a few nits. http://gerrit.cloudera.org:8080/#/c/18077/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18077/6//COMMIT_MSG@7 PS6, Line 7: Add role filed when list remote replicas How about: add the role field for 'kudu remote_replica list' http://gerrit.cloudera.org:8080/#/c/18077/6//COMMIT_MSG@9 PS6, Line 9: Add a raft role filed for output of 'kudu remote_replica list', How about: Added the Raft role field into the output of the 'kudu remote_replica list' CLI tool. http://gerrit.cloudera.org:8080/#/c/18077/6//COMMIT_MSG@10 PS6, Line 10: it's useful when you want to, for example, filter only LEADER replicas. How about moving this into a separate sentence for clarity: It's useful when filtering replicas by their Raft role from the output of the CLI tool (e.g. when it's necessary to find LEADER replicas on a tablet server). http://gerrit.cloudera.org:8080/#/c/18077/6/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/18077/6/src/kudu/tools/kudu-tool-test.cc@3402 PS6, Line 3402: LEARNER Just curious: is it possible to see 'FOLLOWER' here at all even if kNumTservers is set to 1? From the other side, if including all possible roles here regardless of the reality due to the specifics of this scenario, then maybe include 'NON_PARTICIPANT' and 'UNKNOWN_ROLE' as well and add a comment to clarify on this approach? Also, since the line above accepts both 'INITIALIZED' and 'BOOTSTRAPPING' states, could it happen that the Raft role is 'UNKNOWN_ROLE' and this test scenario fails? From the code in tablet_service.cc the role is set to 'UNKNOWN_ROLE' if the tablet isn't in 'RUNNING' state. -- To view, visit http://gerrit.cloudera.org:8080/18077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie38d20d129b18f3a170ec5b0db5a2caf7c0d80ef Gerrit-Change-Number: 18077 Gerrit-PatchSet: 6 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 22 Dec 2021 19:33:25 +0000 Gerrit-HasComments: Yes
