[kudu-CR] tool: new action for adding to the set of data directories
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8352 ) Change subject: tool: new action for adding to the set of data directories .. tool: new action for adding to the set of data directories This patch includes support for adding to the set of data directories in an existing Kudu filesystem. The only user-facing bit is a new FsManager option that, when set, augments Open() to also look for missing fs roots. If any are found, they will be created, and the existing data directory instance files updated to recognize these new roots. Also included is a new tool action that opens an FsManager with this option set. Updating the set of data directories is a complex, multi-step operation, and a single error could leave the filesystem in a difficult-to-repair state. As such, there is some fairly gnarly rollback code that attempts to undo the changes made in the event of an error. This logic can be extended to remove data directories. This patch only addresses adding. Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643 Reviewed-on: http://gerrit.cloudera.org:8080/8352 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/fs/block_manager_util-test.cc M src/kudu/fs/block_manager_util.cc M src/kudu/fs/block_manager_util.h M src/kudu/fs/data_dirs-test.cc M src/kudu/fs/data_dirs.cc M src/kudu/fs/data_dirs.h M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_fs.cc 11 files changed, 730 insertions(+), 169 deletions(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- 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: merged Gerrit-Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643 Gerrit-Change-Number: 8352 Gerrit-PatchSet: 10 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: new action for adding to the set of data directories
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8352 ) Change subject: tool: new action for adding to the set of data directories .. Patch Set 9: Code-Review+2 -- 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: 9 Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 14 Nov 2017 07:40:32 + Gerrit-HasComments: No
[kudu-CR] tool: new action for adding to the set of data directories
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8352 ) Change subject: tool: new action for adding to the set of data directories .. Patch Set 9: (8 comments) Did a light refactor in DataDirManager::Open() to take out some recursion. Also updated FsManager::Open() to log the created files/directories. http://gerrit.cloudera.org:8080/#/c/8352/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8352/8//COMMIT_MSG@21 PS8, Line 21: : This logic can be extended to remove data directories. This patch only : addresses adding. > 2202 is now addressed. I think it's still fine to split this up to a follow Done http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/fs/block_manager_util.h File src/kudu/fs/block_manager_util.h: http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/fs/block_manager_util.h@92 PS8, Line 92: std::string dir() const { return DirName(filename_); } : const std::string& path() const { return filename_; } > these are somewhat confusingly ambiguous. Maybe we should rename 'path' to Done http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/fs/data_dirs.h File src/kudu/fs/data_dirs.h: http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/fs/data_dirs.h@225 PS8, Line 225: // Defaults to false. If true, 'read_only' must be false. > include the default in these docs? (or use inline default) Done http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/fs/fs_manager.cc@372 PS8, Line 372: canonicalized_data_fs_roots_, std::move(dm_opts), _manager_)); > put up a patch here to fix this false positive: http://gerrit.cloudera.org: Ack http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/fs/fs_manager.cc@433 PS8, Line 433: // Delete directories in reverse order since parent directories will have nit: This is confusing because they get synced at L466. maybe "...will not be synchronized to disk until the directory manager is created" or something? http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/fs/fs_manager.cc@469 PS8, Line 469: nit: this is a bit confusing because they're not being deleted. Rename to 'new_dirs' and 'new_files'? http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/fs/fs_manager.cc@508 PS8, Line 508: return Status::AlreadyPresent( : Substitute("FSManager root is not empty. See $0", KuduDocsTroubleshootingUrl()), : Not your fault, but this shouldn't be possible. http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/8352/8/src/kudu/tools/kudu-tool-test.cc@409 PS8, Line 409: update > let's be more specific here like update_dirs? Done -- 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: 9 Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 10 Nov 2017 01:02:00 + Gerrit-HasComments: Yes
[kudu-CR] tool: new action for adding to the set of data directories
Andrew Wong has uploaded a new patch set (#9) to the change originally created by Adar Dembo. ( http://gerrit.cloudera.org:8080/8352 ) Change subject: tool: new action for adding to the set of data directories .. tool: new action for adding to the set of data directories This patch includes support for adding to the set of data directories in an existing Kudu filesystem. The only user-facing bit is a new FsManager option that, when set, augments Open() to also look for missing fs roots. If any are found, they will be created, and the existing data directory instance files updated to recognize these new roots. Also included is a new tool action that opens an FsManager with this option set. Updating the set of data directories is a complex, multi-step operation, and a single error could leave the filesystem in a difficult-to-repair state. As such, there is some fairly gnarly rollback code that attempts to undo the changes made in the event of an error. This logic can be extended to remove data directories. This patch only addresses adding. Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643 --- M src/kudu/fs/block_manager_util-test.cc M src/kudu/fs/block_manager_util.cc M src/kudu/fs/block_manager_util.h M src/kudu/fs/data_dirs-test.cc M src/kudu/fs/data_dirs.cc M src/kudu/fs/data_dirs.h M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_fs.cc 11 files changed, 730 insertions(+), 169 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/8352/9 -- 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: newpatchset Gerrit-Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643 Gerrit-Change-Number: 8352 Gerrit-PatchSet: 9 Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon