Andrew Wong 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, to avoid flakiness attributed to random leadership changes. 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 where this was not required but not supplied? 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 multiple masters with the same host port? 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'? -- 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 20:00:17 +0000 Gerrit-HasComments: Yes
