Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8162 )
Change subject: KUDU-2055 [part 5]: Coalesce hole punch for LBM ...................................................................... Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/8162/9/src/kudu/fs/log_block_manager.h File src/kudu/fs/log_block_manager.h: http://gerrit.cloudera.org:8080/#/c/8162/9/src/kudu/fs/log_block_manager.h@296 PS9, Line 296: // blocks were already deleted, e.g encountered 'NotFound' error during removal. > encountered Done http://gerrit.cloudera.org:8080/#/c/8162/9/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/8162/9/src/kudu/fs/log_block_manager.cc@2064 PS9, Line 2064: } > Can this be const auto& ? I used std::move() at L2089, so it seems not good to use const auto& here as Tidy pointed out. http://gerrit.cloudera.org:8080/#/c/8162/9/src/kudu/fs/log_block_manager.cc@2075 PS9, Line 2075: : lb->container()->BlockDeleted(lb); > Only do CloneAndPrepend() if !s.ok(). Done http://gerrit.cloudera.org:8080/#/c/8162/9/src/kudu/fs/log_block_manager.cc@2126 PS9, Line 2126: if (ContainsKey(failed_dirs, uuid_idx)) { : LOG_EVERY_N(INFO, 10) << Substitute("Block $0 is in a failed directory; not deleting", : block_id.ToString()); : return Status::IOError("Block is in a failed directory"); : } : } > Now that we're passing 'lb' back to RemoveLogBlocks, we could consolidate a Done -- To view, visit http://gerrit.cloudera.org:8080/8162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ccc9538b8184f8f7ba8f6118713b82fef433275 Gerrit-Change-Number: 8162 Gerrit-PatchSet: 10 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Thu, 19 Oct 2017 21:02:11 +0000 Gerrit-HasComments: Yes
