Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17010 )
Change subject: [tool] KUDU-2181 CLI to remove master ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/17010/3/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/17010/3/src/kudu/master/dynamic_multi_master-test.cc@403 PS3, Line 403: RemoveMasterFromClusterUsingCLITool > I wonder if it's worth using this in all the positive run-on-leader cases, Actually the RemoveMasterFromCluster() already uses the RunLeaderMaster() utility function that retries in case of leadership change. But I'll convert all callers to use this function as CLI will be the mechanism used by our users. http://gerrit.cloudera.org:8080/#/c/17010/3/src/kudu/master/dynamic_multi_master-test.cc@875 PS3, Line 875: master_to_remove_uuid > I might've missed it, but maybe add a negative test to exercise the case wh Done http://gerrit.cloudera.org:8080/#/c/17010/3/src/kudu/tools/tool_action_master.cc File src/kudu/tools/tool_action_master.cc: http://gerrit.cloudera.org:8080/#/c/17010/3/src/kudu/tools/tool_action_master.cc@62 PS3, Line 62: Permanent UUID of the master > nit: maybe mention that this is only used to disambiguate if there are mult Done http://gerrit.cloudera.org:8080/#/c/17010/3/src/kudu/tools/tool_action_master.cc@567 PS3, Line 567: remove_master > nit: maybe just 'remove', for parity with 'add'? Done -- To view, visit http://gerrit.cloudera.org:8080/17010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5c97b03475b0ffc7b387d7dfc17acc4b13858fb7 Gerrit-Change-Number: 17010 Gerrit-PatchSet: 3 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 05 Feb 2021 22:55:00 +0000 Gerrit-HasComments: Yes
