[kudu-CR] [WIP] Add BlockDeletionTransaction to Block Manager
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 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 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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] [WIP] Add BlockDeletionTransaction to Block Manager
Hao Hao has posted comments on this change. Change subject: [WIP] Add BlockDeletionTransaction to Block Manager .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/7656/1/src/kudu/fs/file_block_manager.cc File src/kudu/fs/file_block_manager.cc: Line 537: explicit FileBlockDeletionTransaction( > warning: single-argument constructors must be marked explicit to avoid unin Done http://gerrit.cloudera.org:8080/#/c/7656/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 1137: explicit LogBlockDeletionTransaction(LogBlockManager* block_manager); > warning: single-argument constructors must be marked explicit to avoid unin Done Line 1160: BlockDataByContainerMap deleted_block_map_; > warning: invalid case style for private member 'deleted_block_map' [readabi Done Line 1271: void LogBlock::RegisterDeletion( > warning: the parameter 'transaction' is copied for each invocation but only Done -- 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 Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] [WIP] Add BlockDeletionTransaction to Block Manager
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7656 to look at the new patch set (#2). Change subject: [WIP] Add BlockDeletionTransaction to Block Manager .. [WIP] Add BlockDeletionTransaction to Block Manager Similar to 'BlockCreationTransaction', this patch adds a new layer of abstraction at Block Manager to coalesce blocks deletions in a logical operation. Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 --- M src/kudu/cfile/bloomfile.cc M src/kudu/cfile/bloomfile.h M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/fs/block_manager-test.cc M src/kudu/fs/block_manager.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/file_block_manager.h M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/fs/log_block_manager.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/multi_column_writer.cc M src/kudu/tablet/multi_column_writer.h M src/kudu/tablet/tablet_metadata.cc 19 files changed, 307 insertions(+), 126 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7656/2 -- To view, visit http://gerrit.cloudera.org:8080/7656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [WIP] Add BlockDeletionTransaction to Block Manager
Hao Hao has uploaded a new change for review. http://gerrit.cloudera.org:8080/7656 Change subject: [WIP] Add BlockDeletionTransaction to Block Manager .. [WIP] Add BlockDeletionTransaction to Block Manager Similar to 'BlockCreationTransaction', this patch adds a new layer of abstraction at Block Manager to coalesce blocks deletions in a logical operation. Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 --- M src/kudu/cfile/bloomfile.cc M src/kudu/cfile/bloomfile.h M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/fs/block_manager-test.cc M src/kudu/fs/block_manager.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/file_block_manager.h M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/fs/log_block_manager.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/multi_column_writer.cc M src/kudu/tablet/multi_column_writer.h M src/kudu/tablet/tablet_metadata.cc 19 files changed, 306 insertions(+), 126 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7656/1 -- To view, visit http://gerrit.cloudera.org:8080/7656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao