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 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/8162/5/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/8162/5/src/kudu/fs/log_block_manager.cc@1435 PS5, Line 1435: DCHECK(state_ == CLEAN || state_ == DIRTY) > I see, I remember the reasons we leave DoCloseBlocks() and DeleteBlock() in Okay, point #1 is the important one. Anyway, using a one-block deletion transaction here seems fine for now. http://gerrit.cloudera.org:8080/#/c/8162/6/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/8162/6/src/kudu/fs/log_block_manager.cc@2112 PS6, Line 2112: BlockId block_id = lb->block_id(); > I think the main reason is I want to hold the LBM's lock as short as possib This logic can be in RemoveLogBlocks but called outside the spinlock. If RemoveLogBlocks is too large and unwieldy, at least this can be a private helper called by RemoveLogBlocks (that is, no reason why anyone but RemoveLogBlocks should be aware of it). -- 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: 7 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-Comment-Date: Tue, 17 Oct 2017 23:58:05 +0000 Gerrit-HasComments: Yes
