Andrew Wong has posted comments on this change. Change subject: separate DataDirManager from BlockManagers ......................................................................
Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/7602/8/src/kudu/fs/block_manager-stress-test.cc File src/kudu/fs/block_manager-stress-test.cc: PS8, Line 144: : : : : : : : > Don't we need to preserve this in some capacity? Done http://gerrit.cloudera.org:8080/#/c/7602/8/src/kudu/fs/data_dirs.h File src/kudu/fs/data_dirs.h: PS8, Line 231: qcquired > acquired Done http://gerrit.cloudera.org:8080/#/c/7602/8/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: Line 164 > Maybe restore this comment? Done Line 71: return value == "log" || value == "file"; > On macOS can you set this up so that "file" is the only valid option? Done Line 270: RETURN_NOT_OK(DataDirManager::OpenExisting(env_, > Hmm, but in the case of FsManager::CreateNewFileSystemLayout followed by Fs Yeah, it actually seems that this is mandatory. I was banging my head trying to figure out the test failures, and it seems like the double-initting was causing an early release of file locks. Anyway, the simple fix is to not run this block if dd_manager_ has already been constructed. A slightly more complicated fix might require changing the create/open of FsManager in a similar way, but I think that would belong in a separate patch. -- To view, visit http://gerrit.cloudera.org:8080/7602 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I387e4e88bd65298970a1e4201879de08fac07ace Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
