Adar Dembo has posted comments on this change. Change subject: KUDU-2135 (part 2): don't use previously failed disks ......................................................................
Patch Set 30: (2 comments) Not many new comments from me since I already reviewed this a while ago, and the logic you retained doesn't appear to have changed much. I still think Todd should review it, though. http://gerrit.cloudera.org:8080/#/c/7270/30/src/kudu/fs/block_manager_util.cc File src/kudu/fs/block_manager_util.cc: PS30, Line 199: // TODO(awong): in the case where we restart and the most up-to-date instance : // has failed, we fall back on the next-most up-to-date instance, which may : // not accurately describe the current state of the world in terms of what : // disks are safe to use. What's the TODO here though? Isn't this a risk we're willing to take? http://gerrit.cloudera.org:8080/#/c/7270/30/src/kudu/fs/fs.proto File src/kudu/fs/fs.proto: Line 70: repeated bytes DEPRECATED_all_uuids = 2; // Field used before Kudu 1.6.0. In a recent review Dan suggested we handle deprecation by just removing the field outright and reserving the field number via "reserved 2" at the beginning of the message declaration. You can see how I did that here: https://gerrit.cloudera.org/#/c/8045. Of course, that depends on whether we'll continue to write to this field or not. If we're continuing to write to it, then consider marking it as deprecated via protobuf itself (https://gerrit.cloudera.org/#/c/8027/3). -- 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: 30 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
