Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19024 )
Change subject: KUDU-3403 Enable kudu cli to accept specific leader to step down ...................................................................... Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/19024/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19024/5//COMMIT_MSG@10 PS5, Line 10: deduct deduce? http://gerrit.cloudera.org:8080/#/c/19024/5//COMMIT_MSG@13 PS5, Line 13: leader in the master config. nit: ... leader in the tablet configuration known to master. http://gerrit.cloudera.org:8080/#/c/19024/5/src/kudu/tools/tool_action_tablet.cc File src/kudu/tools/tool_action_tablet.cc: http://gerrit.cloudera.org:8080/#/c/19024/5/src/kudu/tools/tool_action_tablet.cc@160 PS5, Line 160: nit: I guess for line continuations there should be 4 space indent, similar to function calls (see https://google.github.io/styleguide/cppguide.html#Function_Calls) http://gerrit.cloudera.org:8080/#/c/19024/5/src/kudu/tools/tool_replica_util.h File src/kudu/tools/tool_replica_util.h: http://gerrit.cloudera.org:8080/#/c/19024/5/src/kudu/tools/tool_replica_util.h@98 PS5, Line 98: for given leader_uuid What if 'leader_uuid' is set to 'nullopt'? http://gerrit.cloudera.org:8080/#/c/19024/5/src/kudu/tools/tool_replica_util.cc File src/kudu/tools/tool_replica_util.cc: http://gerrit.cloudera.org:8080/#/c/19024/5/src/kudu/tools/tool_replica_util.cc@194 PS5, Line 194: const string& tablet_id, : const std::optional<std::string>& leader_uuid, : HostPort* leader_hp) { nit: the indent is off for these 3 lines http://gerrit.cloudera.org:8080/#/c/19024/5/src/kudu/tools/tool_replica_util.cc@202 PS5, Line 202: if (leader_uuid && *leader_uuid == r->ts().uuid()) This logic looks a bit strange to me (so called 'bad smell'): with leader_uuid set to 'nullopt', this method always returns Status::NotFound(). From the other side, the semantics of the 'leader_uuid' parameter hints that specifying 'leader_uuid' isn't necessary. -- To view, visit http://gerrit.cloudera.org:8080/19024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I40569faa40a8173c51504c7567aa84a6ae1fb64a Gerrit-Change-Number: 19024 Gerrit-PatchSet: 5 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Tue, 11 Oct 2022 02:49:12 +0000 Gerrit-HasComments: Yes
