Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/18979 )
Change subject: KUDU-2948 Enable kudu cli to accept specific leader to step down Takes three arguments to identify the leader: 1. Leader uuid 2. Leader Hostname 3. Leader Host port This can be useful in scenario when master is out of sync and user wants to choose a speci ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/18979/1/src/kudu/tools/tool_action_tablet.cc File src/kudu/tools/tool_action_tablet.cc: http://gerrit.cloudera.org:8080/#/c/18979/1/src/kudu/tools/tool_action_tablet.cc@63 PS1, Line 63: > nit for here and below: why to add this trailing space? Done http://gerrit.cloudera.org:8080/#/c/18979/1/src/kudu/tools/tool_action_tablet.cc@66 PS1, Line 66: DEFINE_string(current_leader_hp_port, "", : "Port of the current leader. "); > Why string, not integer for port? Also, please add a validator for the val I have removed host and port arguments in latest change. Now, only current leader's uuid is accepted and host info is figured out internally using uuid. http://gerrit.cloudera.org:8080/#/c/18979/1/src/kudu/tools/tool_action_tablet.cc@165 PS1, Line 165: > nit for here and below: wrong indent, should be four spaces from the beginn Do you mean two spaces (instead of four spaces) as mentioned in the style guide? I see two spaces in this file. e.g. line 158, etc. http://gerrit.cloudera.org:8080/#/c/18979/1/src/kudu/tools/tool_action_tablet.cc@167 PS1, Line 167: const optional<string> current_leader_hp_host = : FLAGS_current_leader_hp_host.empty() ? nullopt : : make_optional(FLAGS_current_leader_hp_host); : const optional<string> current_leader_hp_port = : FLAGS_current_leader_hp_port.empty() ? nullopt : : make_optional(FLAGS_current_leader_hp_port); > If tablet leader's RPC endpoint isn't specified, is it possible to deduce i As such, there doesn't seem to be any existing method for this but I have added another method similar to GetTabletLeader with some tweaks to accommodate our requirement. http://gerrit.cloudera.org:8080/#/c/18979/1/src/kudu/tools/tool_action_tablet.cc@174 PS1, Line 174: > nit: extra space in extra line Done http://gerrit.cloudera.org:8080/#/c/18979/1/src/kudu/tools/tool_action_tablet.cc@175 PS1, Line 175: current_leader_hp_host && current_leader_hp_port > What if only one is specified? With only UUID being accepted as argument, no need to have this code block. -- To view, visit http://gerrit.cloudera.org:8080/18979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f9631ec2001a7c030846cb835d540254b840e5c Gerrit-Change-Number: 18979 Gerrit-PatchSet: 2 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 20 Sep 2022 22:49:03 +0000 Gerrit-HasComments: Yes
