Adar Dembo 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, Done 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 or The FsManager itself doesn't do that (i.e. it always considers the first entry in fs_data_dirs to be the metadata dir), and I'm a little wary of checking for that here because it seems like it could render the tool unable to remove a data dir in a post-metadata striping world. -- 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 17:01:54 +0000 Gerrit-HasComments: Yes
