Hao Hao has posted comments on this change. Change subject: [WIP]log block manager:mark container as 'read_only' after syncing error ......................................................................
Patch Set 8: (12 comments) http://gerrit.cloudera.org:8080/#/c/7374/7/src/kudu/fs/block_manager-test.cc File src/kudu/fs/block_manager-test.cc: Line 968: } > How about: Done http://gerrit.cloudera.org:8080/#/c/7374/7/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: PS7, Line 470: ; it has the same lifespan : // as the block manager. > Correct, at least not until the next restart. The tablet's superblock conta Done Line 472: const LogBlockManagerMetrics* metrics_; > Maybe "no_longer_writable_"? Or "read_only_"? Done Line 766: auto cleanup = MakeScopedCleanup([&]() { > Make sense, will update. Done Line 767: // Handle errors that have not been handled by marking the > Could just Store(true) here. Done PS7, Line 778: : if (!(*s).ok()) { : // Early return; 'cleanup' makes the container as 'read_only'. : return; : } : : // A failure when syncing the container means the container (and its new : // blocks) may be missing the next time the on-disk state is reloaded. As : // such, it's not c > This entire comment should be rewritten in light of the new behavior. If Sy Done Line 787: // synchronization succeeds. > A failure here won't actually lead to non_writable_ being set because we ar Done Line 827: > Why are these new assertions CHECK and not DCHECK? Done Line 858: DCHECK_GE(offset, 0); > We shouldn't allow metadata appends either. Done Line 968: if (full()) { > Shouldn't allow this either. Done Line 1198: block_length_ += data_size; > We should mark the container as unusable if this fails too, or if FlushMeta Done Line 1614: } while (!TryUseBlockId(new_block_id)); > This will have to change to take non-writable containers into consideration Done -- 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: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
