Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/7656 )
Change subject: KUDU-2055 [part 1]: Add fs::BlockDeletionTransaction API ...................................................................... Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/7656/9/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: http://gerrit.cloudera.org:8080/#/c/7656/9/src/kudu/fs/block_manager.h@325 PS9, Line 325: Status CommitDeletedBlocks(std::vector<BlockId>* deleted) { > Yes, this is used in follow-up commit. Especially at TabletMetadata::Delete OK, could you document this behavior. Right now it says 'The 'deleted' out parameter will be set with the list of block IDs that were successfully deleted', should probably add something to the end like 'in case of failure'. Or perhaps just change it so it adds the deleted block IDs regardless of success/failure. http://gerrit.cloudera.org:8080/#/c/7656/9/src/kudu/fs/block_manager.h@333 PS9, Line 333: deleted->emplace_back(block); > If the status is NotFound, we can consider it to be already deleted? And no I suppose it depends on whatever TabletMetadata::DeleteOrphanedBlocks expects, so it's probably fine as-is. -- To view, visit http://gerrit.cloudera.org:8080/7656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 Gerrit-Change-Number: 7656 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: Fri, 22 Sep 2017 18:16:35 +0000 Gerrit-HasComments: Yes
