Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8376 )
Change subject: fs: defer failure from metadata load to bootstrap when data dir is missing ...................................................................... Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/fs/data_dirs.cc@327 PS8, Line 327: FindOrDie(uuid_by_uuid_idx, uuid_idx) is this not just 'uuid' above? http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/fs/data_dirs.cc@330 PS8, Line 330: pb->Swap(&group); I think now that we are on newish protobuf we could do '*pb = std::move(group);' which reads a bit better http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc@557 PS8, Line 557: // Ensure the tablet's data dirs are present and healthy before it is opened. not 100% sure I follow why you defer it all the way until bootstrap time vs marking it FAILED in TSTabletManager::Init immediately after it opens the TabletMeta (the old behavior)? Seems that way we'd have the correct failed status for missing tablets as early as possible vs the possibility that those missing tablets end up waiting on the bootstrap threadpool queue for many minutes behind non-failed tablets doing their log replay http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc@561 PS8, Line 561: error retrieving tablet data dir group will this be easy enough for an operator to understand? perhaps we should say something explanatory like "(data directory likely removed)" or somesuch? http://gerrit.cloudera.org:8080/#/c/8376/8/src/kudu/tablet/tablet_bootstrap.cc@563 PS8, Line 563: return Status::IOError("tablet data is in a failed directory"); perhaps more accurate to say that _some_ tablet data is in a failed directory? Do you think we could include the uuid in the message for shits and giggles? -- To view, visit http://gerrit.cloudera.org:8080/8376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e8d5697c2bb08287cce11fbdab6fb8d6e37d1ad Gerrit-Change-Number: 8376 Gerrit-PatchSet: 8 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 18 Jan 2018 18:51:48 +0000 Gerrit-HasComments: Yes
