Andrew Wong has posted comments on this change. Change subject: disk failure: add persistent disk states ......................................................................
Patch Set 9: (10 comments) Still working on updating this one. Note that there's now a dependency on a new patch to separate the dd manager from the block manager: https://gerrit.cloudera.org/#/c/7602/ http://gerrit.cloudera.org:8080/#/c/7270/7/src/kudu/fs/block_manager_util.h File src/kudu/fs/block_manager_util.h: Line 83: // > warning: parameter 's' is passed by value and only copied once; consider mo Done http://gerrit.cloudera.org:8080/#/c/7270/6/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: Line 212: pool_->Shutdown(); > Done As per a different comment, this was changed so all DataDirs, even those with failed instances, will have a pool. Line 359: initted_ = true; > Done Actually, I it occurred to me that it may not make sense to Create() the DataDirManager with failed dirs. If an operator tries to start Kudu for the first time with a failed disk, why be permissive? In fact, if there is a failed disk, the disk may fail to canonicalize, and we wouldn't even be able to get a good mapping from UUID. I've updated this to just return an error for now, but let me know what you think. http://gerrit.cloudera.org:8080/#/c/7270/7/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: Line 38: #include "kudu/fs/fs_manager.h" > warning: #includes are not sorted properly [llvm-include-order] Done Line 387: InsertOrDie(&uuid_by_root, root, all_uuids[idx]); > warning: passing result of std::move() as a const reference argument; no mo Removed to not support disk failures on Create() Line 398: if (flags & kFlagCreateTestHolePunch) { > warning: 'd' used after it was moved [misc-use-after-move] likewise http://gerrit.cloudera.org:8080/#/c/7270/7/src/kudu/fs/data_dirs.h File src/kudu/fs/data_dirs.h: Line 204: static const char* kDataDirName; > warning: invalid case style for global constant 'FLAG_CREATE_TEST_HOLE_PUNC Done Line 205: static const char* kInvalidPath; > warning: invalid case style for global constant 'FLAG_CREATE_FSYNC' [readab Done Line 350: FRIEND_TEST(DataDirGroupTest, TestLoadBalancingBias); > warning: parameter 'dir_to_update' is const-qualified in the function decla Done Line 350: FRIEND_TEST(DataDirGroupTest, TestLoadBalancingBias); > warning: function 'kudu::fs::DataDirManager::WritePathHealthStates' has a d Done -- To view, visit http://gerrit.cloudera.org:8080/7270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifddf0817fe1a82044077f5544c400c88de20769f Gerrit-PatchSet: 9 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-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
