You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2019/02/20 20:59:07 UTC

[kudu-CR] KUDU-2704: Rowsets that are much bigger than the target size discourage compactions

Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12538

Change subject: KUDU-2704: Rowsets that are much bigger than the target size discourage compactions
......................................................................

KUDU-2704: Rowsets that are much bigger than the target size discourage compactions

If rowsets are flushed that are much bigger than the target rowset size,
then they may get a negative contribution to their score from the
size-based portion of their valuation in the compaction knapsack
problem. This is a problem for two reasons:

1. It can cause fruitful height-based compactions not to run even though
the compaction is under budget.
2. In an extreme case, the value of the rowset can become negative,
which breaks an invariant of the knapsack problem that item weights
be nonnegative.

This fixes the issue by flooring the size-based contribution at 0. A
regression test is included that is based on the real-world example that
I saw. I also tested that the real-life case I observed was fixed by
this patch.

Why do rowsets get flushed "too big"? It could be because the target
size was changed after they were flushed, but I also see almost all
rowsets flushed with a size that is much too big when the number of
columns becomes large. For example, on the cluster where I discovered
this problem, a table with 279 columns was flushing 85MB rowsets even
though the target size is 32MB. That issue ought to be investigated, but
in the meantime this is a workable fix. It has existed for a long time-
the KUDU-2701 fix just made it apparent because it increased how much
the rowsets exceed the target size in many cases.

Change-Id: I1771cd3dbbb17c87160a4bc38b48b3fbc7307676
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/rowset_info.cc
2 files changed, 37 insertions(+), 1 deletion(-)

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/12538/1
--
To view, visit http://gerrit.cloudera.org:8080/12538
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1771cd3dbbb17c87160a4bc38b48b3fbc7307676
Gerrit-Change-Number: 12538
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2704: Rowsets that are much bigger than the target size discourage compactions

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.

Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12538 )

Change subject: KUDU-2704: Rowsets that are much bigger than the target size discourage compactions
......................................................................

KUDU-2704: Rowsets that are much bigger than the target size discourage compactions

Change-Id: I1771cd3dbbb17c87160a4bc38b48b3fbc7307676
Reviewed-on: http://gerrit.cloudera.org:8080/12538
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/rowset_info.cc
2 files changed, 37 insertions(+), 1 deletion(-)

Approvals:
Andrew Wong: Looks good to me, approved
Kudu Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/12538
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1771cd3dbbb17c87160a4bc38b48b3fbc7307676
Gerrit-Change-Number: 12538
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-2704: Rowsets that are much bigger than the target size discourage compactions

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.

Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12538 )

Change subject: KUDU-2704: Rowsets that are much bigger than the target size discourage compactions
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12538/1/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:

http://gerrit.cloudera.org:8080/#/c/12538/1/src/kudu/tablet/compaction_policy-test.cc@630
PS1, Line 630:   ASSERT_EQ(2, picked.size());
             :   ASSERT_GT(quality, 0.0);
For my own understanding, without the patch, would the test case pick none, since the negative size score pushed them below the threshold?



-- 
To view, visit http://gerrit.cloudera.org:8080/12538
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1771cd3dbbb17c87160a4bc38b48b3fbc7307676
Gerrit-Change-Number: 12538
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 20 Feb 2019 21:19:04 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2704: Rowsets that are much bigger than the target size discourage compactions

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12538 )

Change subject: KUDU-2704: Rowsets that are much bigger than the target size discourage compactions
......................................................................


Patch Set 2:

(1 comment)

It seems it's too late, but those were just nits anyways.

http://gerrit.cloudera.org:8080/#/c/12538/1/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:

http://gerrit.cloudera.org:8080/#/c/12538/1/src/kudu/tablet/compaction_policy-test.cc@608
PS1, Line 608:   double quality = 0.0;
paranoid nit: maybe, set it to -1 because there is an  assertion below that compares the value of this variable to 0, and the code below assumes the variable is set by RunTestCase().

Also, consider moving both 'quality' and 'picked' to the site of their usage.



-- 
To view, visit http://gerrit.cloudera.org:8080/12538
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1771cd3dbbb17c87160a4bc38b48b3fbc7307676
Gerrit-Change-Number: 12538
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Feb 2019 22:32:40 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2704: Rowsets that are much bigger than the target size discourage compactions

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.

Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12538 )

Change subject: KUDU-2704: Rowsets that are much bigger than the target size discourage compactions
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12538/1/src/kudu/tablet/compaction_policy-test.cc
File src/kudu/tablet/compaction_policy-test.cc:

http://gerrit.cloudera.org:8080/#/c/12538/1/src/kudu/tablet/compaction_policy-test.cc@630
PS1, Line 630:   ASSERT_EQ(2, picked.size());
             :   ASSERT_GT(quality, 0.0);
> For my own understanding, without the patch, would the test case pick none,
Aye. Picking one rowset is disallowed.



-- 
To view, visit http://gerrit.cloudera.org:8080/12538
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1771cd3dbbb17c87160a4bc38b48b3fbc7307676
Gerrit-Change-Number: 12538
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Feb 2019 22:08:02 +0000
Gerrit-HasComments: Yes