[kudu-CR] KUDU-2055: Coalesce hole punching when deleting groups of blocks

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change.

Change subject: KUDU-2055: Coalesce hole punching when deleting groups of blocks
..


Patch Set 3:

> I'd like to avoid the mega-review that we had for the other patch
 > by carving this one up. Here are some ideas for subpatches:
 > 1. Patch to add BlockDeletionTransaction and plumb it through the
 > LBM. At this stage it's just a new abstraction and doesn't yet
 > change semantics or improve performance.
 > 2. Patch to introduce (and test) coalescing of intervals, in fairly
 > generic fashion.
 > 3. Patch to add coalescing support to BlockDeletionTransaction. If
 > this is tiny, maybe combine it with patch #2.
 > 4. Patch to start using BlockDeletionTransaction outside of the
 > 'fs' module.

Sounds good, will do!

-- 
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] KUDU-2055: Coalesce hole punching when deleting groups of blocks

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-2055: Coalesce hole punching when deleting groups of blocks
..


Patch Set 3:

I'd like to avoid the mega-review that we had for the other patch by carving 
this one up. Here are some ideas for subpatches:
1. Patch to add BlockDeletionTransaction and plumb it through the LBM. At this 
stage it's just a new abstraction and doesn't yet change semantics or improve 
performance.
2. Patch to introduce (and test) coalescing of intervals, in fairly generic 
fashion.
3. Patch to add coalescing support to BlockDeletionTransaction. If this is 
tiny, maybe combine it with patch #2.
4. Patch to start using BlockDeletionTransaction outside of the 'fs' module.

-- 
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] KUDU-2055: Coalesce hole punching when deleting groups of blocks

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change.

Change subject: KUDU-2055: Coalesce hole punching when deleting groups of blocks
..


Patch Set 3:

(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: 
> I think the docs on the 'deleted' parameter could use some work here - I'm 
Done


Line 273:   // Returned block IDs are not guaranteed to be in any particular 
order,
> Since this is infallible (doesn't return Status), could it return the scope
Done


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: std::sort(log_blocks.begin(), log_blocks.end(),
> Not continue?
Done


Line 1217: // If we cannot find the block, then the block could be actually
> can drop the std:: prefix
Done


Line 1225: deleted.emplace_back(block);
> lowercase first letter
Done


Line 1983: 
> consider using CreateDeletionTransaction
That would introduce a downcast?


PS2, Line 2173: d
> lowercase first letter
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: 3
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] KUDU-2055: Coalesce hole punching when deleting groups of blocks

[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 (#3).

Change subject: KUDU-2055: Coalesce hole punching when deleting groups of blocks
..

KUDU-2055: Coalesce hole punching when deleting groups of blocks

Similar to 'BlockCreationTransaction', this patch adds a new layer
of abstraction at Block Manager to coalesce blocks deletions in a
logical operation, e.g. compaction.

By coalescing blocks deletions, the number of hole punching in LBM
will be reduced from one per block to one per log block container.
This should overall optimize the performance of operation that
involves batch deletions, such as compaction and hole repunching.

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
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
22 files changed, 329 insertions(+), 150 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7656/3
-- 
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: 3
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