Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8352 )
Change subject: tool: new actions for adding and removing data directories ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/8352/4/src/kudu/tools/tool_action_fs.cc File src/kudu/tools/tool_action_fs.cc: http://gerrit.cloudera.org:8080/#/c/8352/4/src/kudu/tools/tool_action_fs.cc@377 PS4, Line 377: dirs_to_delete; : vector<string> files_to_delete; nit: looking at this again, it seems a bit strange that on the happy path, these won't be deleted. 'new_dirs' and 'new_files' or something? http://gerrit.cloudera.org:8080/#/c/8352/4/src/kudu/tools/tool_action_fs.cc@496 PS4, Line 496: _root.path == fs.dd_manager()->GetDataRoots()[0]) { : return Status::InvalidArgument( : Substitute("cannot remove metadata directory $0", root_to_remove)); : } We don't do a great job today verifying the fs_flags are in the expected order wrt the metadata directory, but maybe it'd be worth the extra check here (e.g look for /consensus-meta or /tablet-meta)? -- To view, visit http://gerrit.cloudera.org:8080/8352 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643 Gerrit-Change-Number: 8352 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Thu, 26 Oct 2017 16:26:39 +0000 Gerrit-HasComments: Yes