David Ribeiro Alves has posted comments on this change. Change subject: open FS layout in presence of disk failure ......................................................................
Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/7784/4/src/kudu/fs/block_manager_util.cc File src/kudu/fs/block_manager_util.cc: PS4, Line 189: them nit s/them/it PS4, Line 189: it's nit its http://gerrit.cloudera.org:8080/#/c/7784/4/src/kudu/fs/block_manager_util.h File src/kudu/fs/block_manager_util.h: PS4, Line 56: // On success, either 'metadata_' is overwritten with the contents of the : // file, or, in the case of disk failure, returns OK and sets : // 'health_status_' to a non-OK value. : Status LoadFromDisk(); this is weird. whats the reason for this not to return a non-ok status right away? PS4, Line 83: // Whether or not the instance is healthy. If the instance file lives on a : // disk that has failed, this should return false. : bool healthy() const { : return health_status_.ok(); : } we usually don't have this extra getter (i.e. consider just doing pimf.healt_status().ok()) http://gerrit.cloudera.org:8080/#/c/7784/4/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: PS4, Line 794: if (FindUuidByRoot(root, &uuid)) { : return FindUuidIndexByUuid(uuid, uuid_idx); : } : return false; there are no chances of races here, right? http://gerrit.cloudera.org:8080/#/c/7784/4/src/kudu/fs/data_dirs.h File src/kudu/fs/data_dirs.h: PS4, Line 366: Finds the UUID for the given canonicalized root directory name. nit, for consistency append the same suffix as the previous new method (returning false...) -- To view, visit http://gerrit.cloudera.org:8080/7784 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib2a1c824526ed52a6b90ddfbc735cecc4c491118 Gerrit-PatchSet: 4 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: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes