Alexey Serbin has posted comments on this change.

Change subject: [tools] Implement a manual leader_step_down for a tablet
......................................................................


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4533/8/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

Line 227:   Status s = GetTermFromConsensus(tservers, tablet_id_,
Nit: since the variable 's' is not used anywhere else down the code, consider

ASSERT_TRUE(GetTermFromConsensus(...).IsNotFound());


http://gerrit.cloudera.org:8080/#/c/4533/8/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:

PS8, Line 121: I
Is capitalization of 'i' intentional?


http://gerrit.cloudera.org:8080/#/c/4533/8/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:

PS8, Line 53: const char* const kMasterAddressesArgDesc
It seems this is used across different set of tools.  Is it possible to define 
it only once, not repeating it over and over again across all those files which 
require this parameter?


http://gerrit.cloudera.org:8080/#/c/4533/8/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

PS8, Line 67: I
I saw this string is defined in different file as well.  Does it make sense to 
have it defined only in a single place?  E.g., putting it into 
kudu::tools::util namespace or alike.


PS8, Line 177: string
const string&  here and further when using FindOrDie() ?

The idea is that FindOrDie() returns const reference, and the result is not 
modified in the code below.


-- 
To view, visit http://gerrit.cloudera.org:8080/4533
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to