Alexey Serbin has posted comments on this change. Change subject: [tools] Implement a manual leader_step_down for a tablet ......................................................................
Patch Set 12: (1 comment) > > > Patch Set 11: Patch Set 10 was rebased > > > > Could you also add coverage for the new sub-command into > > ToolTest.TestModeHelp in kudu-tool-test.cc? > > Good idea, added. Please note the "*leader*step*" since I had to > workaround a small issue in how we validate the usage output when > lines are split by '\n' , because it doesn't go well with all the > output. For eg, > $ ./bin/kudu tablet > Usage: ./bin/kudu tablet <command> [<args>] > change_config Change a tablet's Raft configuration > leader_step_down Force the tablet's leader replica (if present) > to step > down > We really want to split the output such that one sub-command > becomes one vector element, instead of splitting them by '\n'. I > still don't know how to fix this, but will try to tackle this > problem in a different change. For now, we could just add another > regex with just "*down", but that doesn't look good, hence doing > away with 'down' in the kTabletModeRegexes at the moment. As an easier workaround, I would consider updating the help string for the new command. I added corresponding comment regarding that. Also, consider addressing Tidy Bot's warning about unused 'using' directive. Otherwise, looks good. http://gerrit.cloudera.org:8080/#/c/4533/12/src/kudu/tools/tool_action_tablet.cc File src/kudu/tools/tool_action_tablet.cc: PS12, Line 245: (if present) What if that '(if present)' is dropped? If it do not provide essential information you want to emphasize here, it would help with the test code command help coverage and would look better in 80-line terminal since no wrapping of lines. -- 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: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
