You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yingchun Lai (Code Review)" <ge...@cloudera.org> on 2021/09/09 06:06:25 UTC
[kudu-CR] [fs] Add limit for log block container metadata
Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17837
Change subject: [fs] Add limit for log block container metadata
......................................................................
[fs] Add limit for log block container metadata
In some cases, log block container's data file is very small
after punching hole, while metadata file is very large cause it
contains many CREATE-DELETE pairs. This situation will cause
disk space wasting because it's hard to reach a container size
limit.
This patch add a limit ro metadata, and some other small refactors.
Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
---
M src/kudu/fs/block_id.cc
M src/kudu/fs/dir_manager.cc
M src/kudu/fs/dir_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/fs/fs_report.cc
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/server/server_base.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
13 files changed, 134 insertions(+), 93 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/17837/1
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [fs] Add limit for log block container metadata
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: [fs] Add limit for log block container metadata
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17837/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17837/3//COMMIT_MSG@11
PS3, Line 11: will cause
: disk space wasting
It would be nice if you extend the explanation of how starting a new block container helps to reduce the waste of the disk space. If looking at this from a simplistic perspective, starting a new block container doesn't seem to help in reducing the total size of the disk space used, but instead adds more because a block container has some metadata in the header.
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 09 Sep 2021 23:48:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 07:24:25 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 8:
LGTM, but I guess it's worth asking Andrew to take a quick look just in case :)
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 07:25:50 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] Add limit for log block container metadata
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: [fs] Add limit for log block container metadata
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17837/3/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/17837/3/src/kudu/fs/log_block_manager-test.cc@1186
PS3, Line 1186: 400 * 42
nit: it would be nice to add comment explaining the logic behind this size -- what do 400 and 42 correspond to given create_some_blocks' implementation and the format of the writable block?
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 09 Sep 2021 21:57:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 7: Code-Review+1
(6 comments)
Thank you for addressing the feedback!
We are almost there: just a few nits left in the commit's description.
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@11
PS7, Line 11: cause
because
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@11
PS7, Line 11: LBM will reclaim
: container files (.data and .metadata) when the container is
: full (reach .data file offset limitation, or reach block number
: limitation) and no live blocks left. Before reaching the reclaim
: condition the metadata file will continuous consume disk space.
How about:
LBM reclaims both .data and corresponding .metadata container files when the .data container file becomes full (i.e. reaches its size or block count threshold). So, the .metadata container file might grow without any limit before the .data container becomes full.
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@16
PS7, Line 16: add
adds
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@16
PS7, Line 16: to
for
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@17
PS7, Line 17: make it more easy to reach container's 'full' condition,
taking it into account while determining the container's 'full' condition.
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@17
PS7, Line 17: and
: then more opportunity to be reclaimed, to avoid wasting too much
: disk space.
That gives the LBM an opportunity to reclaim the disk space once the container becomes full.
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 05:12:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 5:
> Patch Set 3:
>
> (1 comment)
Done
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 03:54:05 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
KUDU-3318 [fs] Add size limit for log block container metadata
In some cases, log block container's data file is very small
after punching hole, while metadata file is relatively large
because it contains many CREATE-DELETE pairs. LBM reclaims
both .data and corresponding .metadata container files when
the .data container file becomes full (i.e. reaches its size
or block count threshold).
So, the .metadata container file might grow without any limit
before the .data container becomes full.
This patch adds a size limit for log block container's metadata,
taking it into account while determining the container's 'full'
condition. That gives the LBM an opportunity to reclaim the disk
space once the container becomes full.
Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Reviewed-on: http://gerrit.cloudera.org:8080/17837
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
4 files changed, 57 insertions(+), 3 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 9
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 8: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17837/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17837/8//COMMIT_MSG@17
PS8, Line 17: This patch adds a size limit for log block container's metadata,
: taking it into account while determining the container's 'full'
: condition. That gives the LBM an opportunity to reclaim the disk
: space once the container becomes full.
So the hope in setting this limit is that it's more likely that a container will become full and fully deleted if we apply some limits to the size of metadata, compared to if we let a container grow its metadata unbounded. In those cases, the container can be completely removed. If the containers don't become fully deleted, we may just end up with more containers. It seems like a tricky balance to hit, but I see why this may still be useful.
Longer term, I think the right approach here is to implement container metadata compaction/cleanup, either in the background or when a container becomes (or is near becoming) full.
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 13 Sep 2021 17:00:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17837
to look at the new patch set (#5).
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
KUDU-3318 [fs] Add size limit for log block container metadata
In some cases, log block container's data file is very small
after punching hole, while metadata file is relatively large
cause it contains many CREATE-DELETE pairs. LBM will reclaim
container files (.data and .metadata) when the container is
full (reach .data file offset limitation, or reach block number
limitation) and no live blocks left. Before reaching the reclaim
condition the metadata file will continuous consume disk space.
This patch add a size limit to log block container's metadata,
make it more easy to reach container's 'full' condition, and
then more opportunity to be reclaimed, to avoid wasting too much
disk space.
Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
4 files changed, 55 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/17837/5
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [fs] Add limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17837
to look at the new patch set (#2).
Change subject: [fs] Add limit for log block container metadata
......................................................................
[fs] Add limit for log block container metadata
In some cases, log block container's data file is very small
after punching hole, while metadata file is very large cause it
contains many CREATE-DELETE pairs. This situation will cause
disk space wasting because it's hard to reach a container size
limit.
This patch add a limit ro metadata, and some other small refactors.
Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
---
M src/kudu/fs/block_id.cc
M src/kudu/fs/dir_manager.cc
M src/kudu/fs/dir_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/fs/fs_report.cc
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/server/server_base.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
13 files changed, 146 insertions(+), 102 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/17837/2
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17837
to look at the new patch set (#8).
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
KUDU-3318 [fs] Add size limit for log block container metadata
In some cases, log block container's data file is very small
after punching hole, while metadata file is relatively large
because it contains many CREATE-DELETE pairs. LBM reclaims
both .data and corresponding .metadata container files when
the .data container file becomes full (i.e. reaches its size
or block count threshold).
So, the .metadata container file might grow without any limit
before the .data container becomes full.
This patch adds a size limit for log block container's metadata,
taking it into account while determining the container's 'full'
condition. That gives the LBM an opportunity to reclaim the disk
space once the container becomes full.
Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
4 files changed, 57 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/17837/8
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17837
to look at the new patch set (#6).
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
KUDU-3318 [fs] Add size limit for log block container metadata
In some cases, log block container's data file is very small
after punching hole, while metadata file is relatively large
cause it contains many CREATE-DELETE pairs. LBM will reclaim
container files (.data and .metadata) when the container is
full (reach .data file offset limitation, or reach block number
limitation) and no live blocks left. Before reaching the reclaim
condition the metadata file will continuous consume disk space.
This patch add a size limit to log block container's metadata,
make it more easy to reach container's 'full' condition, and
then more opportunity to be reclaimed, to avoid wasting too much
disk space.
Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
4 files changed, 56 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/17837/6
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17837/3/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/17837/3/src/kudu/fs/log_block_manager-test.cc@1186
PS3, Line 1186: otobuf f
> nit: it would be nice to add comment explaining the logic behind this size
Done
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 04:22:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@11
PS7, Line 11: becau
> because
Done
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@11
PS7, Line 11: . LBM reclaims
: both .data and corresponding .metadata container files when
: the .data container file becomes full (i.e. reaches its size
: or block count threshold).
: So, the .metadata container file might grow without any limit
> How about:
Done
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@16
PS7, Line 16: .da
> adds
Done
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@16
PS7, Line 16: ec
> for
Done
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@17
PS7, Line 17: This patch adds a size limit for log block container's m
> taking it into account while determining the container's 'full' condition.
Done
http://gerrit.cloudera.org:8080/#/c/17837/7//COMMIT_MSG@17
PS7, Line 17: tadata,
: taking it into account while determining the container's 'full'
: condition.
> That gives the LBM an opportunity to reclaim the disk space once the contai
Done
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 05:50:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17837
to look at the new patch set (#4).
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
KUDU-3318 [fs] Add size limit for log block container metadata
In some cases, log block container's data file is very small
after punching hole, while metadata file is relatively large
cause it contains many CREATE-DELETE pairs. LBM will reclaim
container files (.data and .metadata) when the container is
full (reach .data file offset limitation, or reach block number
limitation) and no live blocks left. Before reaching the reclaim
condition the metadata file will continuous consume disk space.
This patch add a size limit to log block container's metadata to
avoid wasting too much disk space.
Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
4 files changed, 55 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/17837/4
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [fs] Add limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: [fs] Add limit for log block container metadata
......................................................................
Patch Set 3:
> Patch Set 2:
>
> (1 comment)
OK, I've removed re-factoring code away.
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 09 Sep 2021 17:31:55 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] Add limit for log block container metadata
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: [fs] Add limit for log block container metadata
......................................................................
Patch Set 3:
> > Patch Set 2:
> >
> > (1 comment)
>
> OK, I've removed re-factoring code away.
Thank you! Feel free to post those as a separate patch -- I guess there is some value in the refactoring you did, and it would be nice to get those into the codebase eventually.
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 09 Sep 2021 20:37:16 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 8:
> Patch Set 8: Code-Review+2
>
> (1 comment)
I will plan to implement compact low live block metadata at runtime.
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 14 Sep 2021 07:09:23 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] Add limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17837
to look at the new patch set (#3).
Change subject: [fs] Add limit for log block container metadata
......................................................................
[fs] Add limit for log block container metadata
In some cases, log block container's data file is very small
after punching hole, while metadata file is very large cause it
contains many CREATE-DELETE pairs. This situation will cause
disk space wasting because it's hard to reach a container size
limit.
This patch add a limit to log block container's metadata to
avoid wasting too much disk space.
Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
4 files changed, 55 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/17837/3
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 5:
> Patch Set 2:
>
> (1 comment)
Done
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 04:01:56 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] Add limit for log block container metadata
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: [fs] Add limit for log block container metadata
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17837/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17837/2//COMMIT_MSG@14
PS2, Line 14: and some other small refactors.
Could you please put the re-factoring into a separate changelist? It would help to track the changes and focus on the essence of the change.
Thank you!
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 09 Sep 2021 16:50:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17837
to look at the new patch set (#7).
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
KUDU-3318 [fs] Add size limit for log block container metadata
In some cases, log block container's data file is very small
after punching hole, while metadata file is relatively large
cause it contains many CREATE-DELETE pairs. LBM will reclaim
container files (.data and .metadata) when the container is
full (reach .data file offset limitation, or reach block number
limitation) and no live blocks left. Before reaching the reclaim
condition the metadata file will continuous consume disk space.
This patch add a size limit to log block container's metadata,
make it more easy to reach container's 'full' condition, and
then more opportunity to be reclaimed, to avoid wasting too much
disk space.
Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
4 files changed, 57 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/17837/7
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17837 )
Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
......................................................................
Patch Set 8:
> Patch Set 8:
>
> LGTM, but I guess it's worth asking Andrew to take a quick look just in case :)
OK, thanks
--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 08:30:10 +0000
Gerrit-HasComments: No