Hao Hao has posted comments on this change. Change subject: [WIP]log block manager: mark container as 'read_only' after syncing error ......................................................................
Patch Set 9: (11 comments) http://gerrit.cloudera.org:8080/#/c/7374/8/src/kudu/fs/block_manager-test.cc File src/kudu/fs/block_manager-test.cc: Line 848: const int kNumTries = 3; > Would this test be more effective if the difference between short and long Done PS8, Line 851: a = s > I think it was more correct before as 'so little' Done PS8, Line 870: > s/around/in Done http://gerrit.cloudera.org:8080/#/c/7374/8/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 266: Status FinishBlock(Status& s, WritableBlock* block); > Does the signature have to change here? I think it's a little more ambiguo Done Line 379: void SetReadOnly(); > Perhaps it should be called 'SetReadOnly()' so there isn't any ambiguity ab Done Line 772: // or append to it. > s->ok() Done Line 773: if (!result_status.ok()) { > Use SetMode/SetReadOnly? Done Line 779: > This can use RETURN_NOT_OK Done Line 794: RETURN_NOT_OK(result_status); > Do we want to continue through to below in the case of failure? Right, we should not. Updated it. Line 833: Status LogBlockContainer::WriteVData(int64_t offset, const vector<Slice>& data) { > Here and below, consider using read_only() Done Line 1206: auto cleanup = MakeScopedCleanup([&]() { > Instead of creating a scoped cleanup object, and thus having to always reme One thing is that if FlushData() fail, we may not necessarily need to mark the container as read_only? As there hasn't been any metadata been appended yet. -- To view, visit http://gerrit.cloudera.org:8080/7374 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I15062b9d82c9cb563fb6bb2af7ec89da5f71e28f Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
