Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15909 )
Change subject: WIP [tools] multiple tablet ids in 'local_replica delete' ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/kudu-tool-test.cc@2782 PS1, Line 2782: ts->server()->tablet_manager()->GetTabletReplicas(&tablet_replicas); You probably need to clear this list before shutting down. http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/tool_action_local_replica.cc@411 PS1, Line 411: vector<string> tablet_ids = strings::Split(tablet_ids_str, ",", strings::SkipEmpty()); : if (tablet_ids.empty()) { : return Status::InvalidArgument("no tablet identifiers provided"); : } nit: Split() is templatized and can return a set. Why not use that? Is order preservation important here? http://gerrit.cloudera.org:8080/#/c/15909/1/src/kudu/tools/tool_action_local_replica.cc@421 PS1, Line 421: LOG(INFO) << "removed duplicate tablet identifiers"; nit: might it be worth displaying how many unique tablet IDs there are? Or maybe VLOGging a newline-separated list of tablet IDs? -- To view, visit http://gerrit.cloudera.org:8080/15909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If0e509d1775be2a728e4e3b10c724c1f15a96ec1 Gerrit-Change-Number: 15909 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 May 2020 22:30:05 +0000 Gerrit-HasComments: Yes
