[kudu-CR] [WIP] Add BlockDeletionTransaction to Block Manager

[Dan Burkert (Code Review)]

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 (Code Review)]

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

[Hao Hao (Code Review)]

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 (Code Review)]

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