Dinesh Bhat has posted comments on this change.

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


Patch Set 9:

(6 comments)

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

PS6, Line 222: ctor<TServerDetails*> tservers;
> Note that BuildAndStart() wait for all tablets to reach the state of RUNNIN
Hmmm, AFAICT the only way we could whitelist that string is by comparing with 
stderr output. Let me know if I misunderstood your comment. Done that change.


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:                                             tablet_id_,
> Nit: since the variable 's' is not used anywhere else down the code, consid
Done


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: 
> Is capitalization of 'i' intentional?
Yeah, wanted to be consistent with rest of the places.


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 kTableNameArg = "table_
> It seems this is used across different set of tools.  Is it possible to def
Done


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 saw this string is defined in different file as well.  Does it make sense
Done


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


-- 
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: 9
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