Dan Burkert has posted comments on this change. Change subject: [WIP] Add BlockDeletionTransaction to Block Manager ......................................................................
Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/7656/2/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: Line 182: virtual void MarkDeletions(std::vector<BlockId> blocks, I think the docs on the 'deleted' parameter could use some work here - I'm not sure at a glance exactly what the out parameter should be used for. Line 273: virtual void CreateDeletionTransaction( Since this is infallible (doesn't return Status), could it return the scoped_refptr<..> instead of taking it as an out param? http://gerrit.cloudera.org:8080/#/c/7656/2/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 1184: break; Not continue? Line 1217: std::vector<BlockId> blocks, can drop the std:: prefix Line 1225: WARN_NOT_OK(s, Substitute("Could not delete block $0", lowercase first letter Line 1983: make_scoped_refptr(new LogBlockDeletionTransaction(this))); consider using CreateDeletionTransaction PS2, Line 2173: C lowercase first letter -- To view, visit http://gerrit.cloudera.org:8080/7656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao <hao....@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
