Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8978 )
Change subject: KUDU-2202: support for removing data directories (take two) ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc@805 PS1, Line 805: // This uuid's directory is missing outright, which can happen when : // opts_.read_only and opts_.update_on_disk are both true. > So this can only happen when we open our "staging" FsManager and we're miss After another look, I think this is ok. Since we're only opening up the speculative FsManager, we shouldn't need to worry about potential mismatched UUIDs (e.g. if there's a missing directory AND a failed directory, we might end up assigning the failed directory the wrong UUID). When we open the FsManager up for real, this shouldn't be the case because at that point, we'll update the path_sets and shouldn't reach this code block. Can you add a comment explaining why this missing directory doesn't matter? -- To view, visit http://gerrit.cloudera.org:8080/8978 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b Gerrit-Change-Number: 8978 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Wed, 10 Jan 2018 19:57:28 +0000 Gerrit-HasComments: Yes
