Hao Hao has posted comments on this change. Change subject: KUDU-1943: Add BlockTransaction to Block Manager ......................................................................
Patch Set 22: (10 comments) > (1 comment) > > Looks good! I'm very pleased how this came together, net-net I > think the code is easier to understand with this patch. Thank you! Really appreciate yours and Adar's thorough code reviews! http://gerrit.cloudera.org:8080/#/c/7207/22//COMMIT_MSG Commit Message: PS22, Line 15: for > an Done http://gerrit.cloudera.org:8080/#/c/7207/22/src/kudu/fs/block_manager.cc File src/kudu/fs/block_manager.cc: PS22, Line 34: blocks > block Done Line 45: DEFINE_string(block_manager_flush_control, "finalize", > It sounds scarier than it really is. We should at least clarify that this f Done PS22, Line 46: when to flush > More like when to pre-flush? Done http://gerrit.cloudera.org:8080/#/c/7207/22/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: PS22, Line 70: is > is set to Done PS22, Line 127: is > is set to Done PS22, Line 128: asynchronous flush of dirty block data to disk > Sounds good for performance. What is the crash consistency contract? Can th Yes, in the worst case we could have 'gaps' in the data file, but this should be handled by hole repunching. Also, we only flush data in this case, and always append metadata after data is durable. This ensures metadata never points to garbage data. http://gerrit.cloudera.org:8080/#/c/7207/22/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 276: // This is called after synchronization of dirty data and metadata > doesn't finalization have to happen before the data is synced to disk? DoClose() is called by DoCloseBlocks(). If closing a group of blocks, the blocks should be finalized. But for a single block it can be called without being finalized. There is a DCHECK at L904. PS22, Line 458: rounds up > "rounds up"? Done PS22, Line 459: it > clarify: is "it" the block or the container? Done -- To view, visit http://gerrit.cloudera.org:8080/7207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I01f3ed830cc1f86d7bb8a66b2f780914a88884bd Gerrit-PatchSet: 22 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
