Adar Dembo 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 9: (5 comments) Looks much better! 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 encounter 'NotFound' error during removal. encountered 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@1247 PS9, Line 1247: CHECK_OK_PREPEND(CoalesceIntervals<int64_t>(&entry.second), Didn't even know we had CHECK_OK_PREPEND. Neat! http://gerrit.cloudera.org:8080/#/c/8162/9/src/kudu/fs/log_block_manager.cc@2064 PS9, Line 2064: for (auto& lb : lbs) { Can this be const auto& ? http://gerrit.cloudera.org:8080/#/c/8162/9/src/kudu/fs/log_block_manager.cc@2075 PS9, Line 2075: .CloneAndPrepend( : "Unable to append deletion record to block metadata"); Only do CloneAndPrepend() if !s.ok(). http://gerrit.cloudera.org:8080/#/c/8162/9/src/kudu/fs/log_block_manager.cc@2126 PS9, Line 2126: mem_tracker_->Release(kudu_malloc_usable_size(lb->get())); : : if (metrics()) { : metrics()->blocks_under_management->Decrement(); : metrics()->bytes_under_management->DecrementBy((*lb)->length()); : } Now that we're passing 'lb' back to RemoveLogBlocks, we could consolidate all of these across LogBlocks into just three calls. -- 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: 9 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: Wed, 18 Oct 2017 21:47:44 +0000 Gerrit-HasComments: Yes
