Adar Dembo has posted comments on this change. Change subject: log block manage: tighten up read only usage ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5383/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 410: if (!block_manager->read_only_ && > Shouldn't this still return Status::Aborted if we are in read_only_ mode? Not necessarily; we'll fail in one of the open calls below and Open() will still fail until the repair. But, if we return Aborted, we'll safely ignore this container and carry on. That's clearly the better experience, so I'll make the change. Line 499: if (read_status.IsIncomplete() && !block_manager()->read_only_) { > Same here - should we return OK but not attempt to repair it? See above: we could just fail outright. But, as long as we don't try to write to metadata_file_ (which we shouldn't, since this is a read-only instance), we can't corrupt anything. -- To view, visit http://gerrit.cloudera.org:8080/5383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I029527b78148d68a002a3c42af5a52f179089a95 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
