You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Ashwani Raina (Code Review)" <ge...@cloudera.org> on 2025/08/26 13:35:55 UTC
[kudu-CR] WIP [compaction] include undo deltas while picking rowsets
Ashwani Raina has uploaded this change for review. ( http://gerrit.cloudera.org:8080/23348
Change subject: WIP [compaction] include undo deltas while picking rowsets
......................................................................
WIP [compaction] include undo deltas while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The idea is to get the initial feedback on this approach. And run all
the basic sanity tests via dist-test framework.
From unit testing side, I am planning to run some targeted tests on
huge dataset with considerable size in undo deltas. Additionally, add
a few unit tests once there is some consensus on the approach.
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 24 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/23348/1
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 10:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2302/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 10
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 26 Feb 2026 11:15:13 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 7:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2294/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Feb 2026 12:02:49 +0000
Gerrit-HasComments: No
[kudu-CR] WIP [compaction] include undo deltas while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: WIP [compaction] include undo deltas while picking rowsets
......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/1714/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 26 Aug 2025 15:11:24 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 9:
(1 comment)
It seems I missed pointing to one obvious point in earlier review rounds, but it's better late than never.
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc
File src/kudu/tablet/rowset_info.cc:
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc@475
PS9, Line 475: if (FLAGS_rowset_merge_compaction_weight_includes_undo_deltas) {
: extra_->base_and_deltas_size_bytes = rs->OnDiskBaseDataSizeWithDeltas();
: } else {
: extra_->base_and_deltas_size_bytes = rs->OnDiskBaseDataSizeWithRedos();
: }
Why to have this logic around FLAGS_rowset_merge_compaction_weight_includes_undo_deltas spread everywhere?
Why not to encapsulate the details into the implementation of OnDiskBaseDataSizeWithDeltas() method?
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 9
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Feb 2026 20:17:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 7:
Build Aborted
http://jenkins.kudu.apache.org/job/pre_commit/2294/ : ABORTED
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Feb 2026 12:27:06 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 10: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2302/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 10
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 26 Feb 2026 12:17:42 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG@7
PS2, Line 7: [compaction]
> Please reference an appropriate upstream ticket for this bug.
This patch is independent of the functionality introduced with KUDU-3406.
The calculation is always going to take into consideration the size of UNDO deltas along with existing REDO and base data sizes.
This new calculated size is used at the time of deciding whether the rowset candidate is to be picked for compaction.
With this change, we don't need to get into complex steps of enabling the budgeted compaction introduced in KUDU-3406.
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG@22
PS2, Line 22: With this patch, rowset compaction never hit OOM and the resident
: memory kept well within limits (~1GB).
> I guess the tests were run with the compaction memory budgeting turned on?
No memory budgeting configuration needs to be turned on for this.
This change simply includes size of UNDO deltas(which is not being done currently) into the usual budgeting (not to be confused with budgeting code added in KUDU-3406) calculation while picking the rowsets for compaction.
This essentially fixes the day-one issue of incorrect way of adding rowset candidates. Currently, only base data and redo delta sizes are taken into consideration while creating min-heap. With that, we end up adding rowsets with huge UNDO deltas as well that are eventually going to push the compaction op beyond budget limits.
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG@37
PS2, Line 37: [<redacted>,<redacted>]
> nit for here and below: if this isn't adding any useful information, maybe
Done
http://gerrit.cloudera.org:8080/#/c/23348/2/src/kudu/tablet/rowset.h
File src/kudu/tablet/rowset.h:
http://gerrit.cloudera.org:8080/#/c/23348/2/src/kudu/tablet/rowset.h@206
PS2, Line 206: or UNDO deltas
> here and elsewhere: please revise and update the comments where necessary
Done
http://gerrit.cloudera.org:8080/#/c/23348/2/src/kudu/tablet/rowset.h@207
PS2, Line 207: RedosAndUndos
> here and elsewhere: change 'RedosAndUndos' into 'Deltas' for brevity?
Done
http://gerrit.cloudera.org:8080/#/c/23348/2/src/kudu/tablet/rowset.h@449
PS2, Line 449: not including UNDOs
> ditto
Done
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jan 2026 13:27:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 5:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2223/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Jan 2026 09:16:22 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc
File src/kudu/tablet/rowset_info.cc:
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc@475
PS9, Line 475: if (FLAGS_rowset_merge_compaction_weight_includes_undo_deltas) {
: extra_->base_and_deltas_size_bytes = rs->OnDiskBaseDataSizeWithDeltas();
: } else {
: extra_->base_and_deltas_size_bytes = rs->OnDiskBaseDataSizeWithRedos();
: }
> The idea was to encapsulate the logic into the RowSet::OnDiskBaseDataSizeWi
Ah! yes, there is the third place in tablet.cc as well.
renaming 'rowset_merge_compaction_weight_includes_undo_deltas' to 'rowset_deltas_size_include_undo' kind of flips my argument in favor of encapsulating the logic of including the size of undo deltas into the OnDiskBaseDataSizeWithDeltas().
This way sounds good to me as the name of the flag name kept aligned with the purpose and also at the right place.
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 9
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 26 Feb 2026 11:14:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 2:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2191/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 17 Dec 2025 11:53:55 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 4:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2219/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 4
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Jan 2026 18:22:23 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 4: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2219/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 4
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Jan 2026 19:11:08 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 10: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc
File src/kudu/tablet/rowset_info.cc:
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc@475
PS9, Line 475: base_and_deltas_size_mb_ =
: std::max(implicit_cast<int>(extra_->base_and_deltas_size_bytes / 1024 / 1024),
: kMinSizeMb);
: }
:
> Ah! yes, there is the third place in tablet.cc as well.
Great, thank you for re-considering and updating the code.
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 10
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 26 Feb 2026 19:27:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
KUDU-3734 include undo size while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The test data used here has significant undo deltas that fits perfectly
into the OOM scenario. Total size of uncompacted data is 29GB.
With this patch, rowset compaction never hit OOM and the resident
memory kept well within limits (~1GB).
Without this patch, compaction hit OOM on a node with limited memory
where rowset compaction peak memory touched ~30GB.
Testing with different scenarios:
1. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 1024 MB
Result: Rowset 211 was skipped due to 1024 MB size constraint.
Budgeted compaction selection:
[ ] RowSet(211)( 1945M) [0.0000, 1.0000]
[ ] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
2. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 2048 MB
Result: Rowset 211 was NOT skipped with 2048 MB size limit.
Budgeted compaction selection:
[x] RowSet(211)( 1945M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
3. Compaction that takes into consideration:
- Base data
- Redo deltas
Compaction budget (tablet_compaction_budget_mb) is default 128 MB
Result: Rowset 211 with size 1M (ignoring UNDO deltas) included.
Budgeted compaction selection:
[x] RowSet(211)( 1M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0026, 0.2015]
[x] RowSet(216)( 1M) [0.5358, 0.5385]
[x] RowSet(218)( 1M) [0.5385, 0.5404]
[x] RowSet(219)( 1M) [0.5404, 0.5404]
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Reviewed-on: http://gerrit.cloudera.org:8080/23348
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 51 insertions(+), 39 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 11
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 9:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2297/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 9
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Feb 2026 06:30:08 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 8: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2295/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Feb 2026 13:41:42 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 6:
Do we need to consider the undo delta's older than ancient history mark? Could there be a race between them GC'd vs being compacted?
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 05 Feb 2026 07:00:44 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kurt Deschler, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23348
to look at the new patch set (#4).
Change subject: [compaction] include undo size while picking rowsets
......................................................................
[compaction] include undo size while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The test data used here has significant undo deltas that fits perfectly
into the OOM scenario. Total size of uncompacted data is 29GB.
With this patch, rowset compaction never hit OOM and the resident
memory kept well within limits (~1GB).
Without this patch, compaction hit OOM on a node with limited memory
where rowset compaction peak memory touched ~30GB.
Testing with different scenarios:
1. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 1024 MB
Result: Rowset 211 was skipped due to 1024 MB size constraint.
Budgeted compaction selection:
[ ] RowSet(211)( 1945M) [0.0000, 1.0000]
[ ] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
2. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 2048 MB
Result: Rowset 211 was NOT skipped with 2048 MB size limit.
Budgeted compaction selection:
[x] RowSet(211)( 1945M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
3. Compaction that takes into consideration:
- Base data
- Redo deltas
Compaction budget (tablet_compaction_budget_mb) is default 128 MB
Result: Rowset 211 with size 1M (ignoring UNDO deltas) included.
Budgeted compaction selection:
[x] RowSet(211)( 1M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0026, 0.2015]
[x] RowSet(216)( 1M) [0.5358, 0.5385]
[x] RowSet(218)( 1M) [0.5385, 0.5404]
[x] RowSet(219)( 1M) [0.5404, 0.5404]
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 35 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/23348/4
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 4
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 2: Verified+1
Build Successful
http://jenkins.kudu.apache.org/job/pre_commit/2191/ : SUCCESS
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 17 Dec 2025 13:05:42 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kurt Deschler, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23348
to look at the new patch set (#6).
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
KUDU-3734 include undo size while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The test data used here has significant undo deltas that fits perfectly
into the OOM scenario. Total size of uncompacted data is 29GB.
With this patch, rowset compaction never hit OOM and the resident
memory kept well within limits (~1GB).
Without this patch, compaction hit OOM on a node with limited memory
where rowset compaction peak memory touched ~30GB.
Testing with different scenarios:
1. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 1024 MB
Result: Rowset 211 was skipped due to 1024 MB size constraint.
Budgeted compaction selection:
[ ] RowSet(211)( 1945M) [0.0000, 1.0000]
[ ] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
2. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 2048 MB
Result: Rowset 211 was NOT skipped with 2048 MB size limit.
Budgeted compaction selection:
[x] RowSet(211)( 1945M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
3. Compaction that takes into consideration:
- Base data
- Redo deltas
Compaction budget (tablet_compaction_budget_mb) is default 128 MB
Result: Rowset 211 with size 1M (ignoring UNDO deltas) included.
Budgeted compaction selection:
[x] RowSet(211)( 1M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0026, 0.2015]
[x] RowSet(216)( 1M) [0.5358, 0.5385]
[x] RowSet(218)( 1M) [0.5385, 0.5404]
[x] RowSet(219)( 1M) [0.5404, 0.5404]
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 35 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/23348/6
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23348
to look at the new patch set (#9).
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
KUDU-3734 include undo size while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The test data used here has significant undo deltas that fits perfectly
into the OOM scenario. Total size of uncompacted data is 29GB.
With this patch, rowset compaction never hit OOM and the resident
memory kept well within limits (~1GB).
Without this patch, compaction hit OOM on a node with limited memory
where rowset compaction peak memory touched ~30GB.
Testing with different scenarios:
1. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 1024 MB
Result: Rowset 211 was skipped due to 1024 MB size constraint.
Budgeted compaction selection:
[ ] RowSet(211)( 1945M) [0.0000, 1.0000]
[ ] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
2. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 2048 MB
Result: Rowset 211 was NOT skipped with 2048 MB size limit.
Budgeted compaction selection:
[x] RowSet(211)( 1945M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
3. Compaction that takes into consideration:
- Base data
- Redo deltas
Compaction budget (tablet_compaction_budget_mb) is default 128 MB
Result: Rowset 211 with size 1M (ignoring UNDO deltas) included.
Budgeted compaction selection:
[x] RowSet(211)( 1M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0026, 0.2015]
[x] RowSet(216)( 1M) [0.5358, 0.5385]
[x] RowSet(218)( 1M) [0.5385, 0.5404]
[x] RowSet(219)( 1M) [0.5404, 0.5404]
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 102 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/23348/9
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 9
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23348
to look at the new patch set (#10).
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
KUDU-3734 include undo size while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The test data used here has significant undo deltas that fits perfectly
into the OOM scenario. Total size of uncompacted data is 29GB.
With this patch, rowset compaction never hit OOM and the resident
memory kept well within limits (~1GB).
Without this patch, compaction hit OOM on a node with limited memory
where rowset compaction peak memory touched ~30GB.
Testing with different scenarios:
1. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 1024 MB
Result: Rowset 211 was skipped due to 1024 MB size constraint.
Budgeted compaction selection:
[ ] RowSet(211)( 1945M) [0.0000, 1.0000]
[ ] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
2. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 2048 MB
Result: Rowset 211 was NOT skipped with 2048 MB size limit.
Budgeted compaction selection:
[x] RowSet(211)( 1945M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
3. Compaction that takes into consideration:
- Base data
- Redo deltas
Compaction budget (tablet_compaction_budget_mb) is default 128 MB
Result: Rowset 211 with size 1M (ignoring UNDO deltas) included.
Budgeted compaction selection:
[x] RowSet(211)( 1M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0026, 0.2015]
[x] RowSet(216)( 1M) [0.5358, 0.5385]
[x] RowSet(218)( 1M) [0.5385, 0.5404]
[x] RowSet(219)( 1M) [0.5404, 0.5404]
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 51 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/23348/10
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 10
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 9: Verified+1
Build Successful
http://jenkins.kudu.apache.org/job/pre_commit/2297/ : SUCCESS
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 9
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Feb 2026 07:33:35 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23348
to look at the new patch set (#7).
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
KUDU-3734 include undo size while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The test data used here has significant undo deltas that fits perfectly
into the OOM scenario. Total size of uncompacted data is 29GB.
With this patch, rowset compaction never hit OOM and the resident
memory kept well within limits (~1GB).
Without this patch, compaction hit OOM on a node with limited memory
where rowset compaction peak memory touched ~30GB.
Testing with different scenarios:
1. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 1024 MB
Result: Rowset 211 was skipped due to 1024 MB size constraint.
Budgeted compaction selection:
[ ] RowSet(211)( 1945M) [0.0000, 1.0000]
[ ] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
2. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 2048 MB
Result: Rowset 211 was NOT skipped with 2048 MB size limit.
Budgeted compaction selection:
[x] RowSet(211)( 1945M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
3. Compaction that takes into consideration:
- Base data
- Redo deltas
Compaction budget (tablet_compaction_budget_mb) is default 128 MB
Result: Rowset 211 with size 1M (ignoring UNDO deltas) included.
Budgeted compaction selection:
[x] RowSet(211)( 1M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0026, 0.2015]
[x] RowSet(216)( 1M) [0.5358, 0.5385]
[x] RowSet(218)( 1M) [0.5385, 0.5404]
[x] RowSet(219)( 1M) [0.5404, 0.5404]
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 97 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/23348/7
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23348
to look at the new patch set (#2).
Change subject: [compaction] include undo size while picking rowsets
......................................................................
[compaction] include undo size while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The test data used here has significant undo deltas that fits perfectly
into the OOM scenario. Total size of uncompacted data is 29GB.
With this patch, rowset compaction never hit OOM and the resident
memory kept well within limits (~1GB).
Without this patch, compaction hit OOM on a node with limited memory
where rowset compaction peak memory touched ~30GB.
Testing with different scenarios:
1. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 1024 MB
Result: Rowset 211 was skipped due to 1024 MB size constraint.
Budgeted compaction selection:
[ ] RowSet(211)( 1945M) [0.0000, 1.0000] [<redacted>,<redacted>]
[ ] RowSet(217)( 1M) [0.0027, 0.1953] [<redacted>,<redacted>]
[x] RowSet(216)( 1M) [0.5341, 0.5341] [<redacted>,<redacted>]
[x] RowSet(218)( 1M) [0.5341, 0.5341] [<redacted>,<redacted>]
[x] RowSet(219)( 1M) [0.5341, 0.5341] [<redacted>,<redacted>]
2. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 2048 MB
Result: Rowset 211 was NOT skipped with 2048 MB size limit.
Budgeted compaction selection:
[x] RowSet(211)( 1945M) [0.0000, 1.0000] [<redacted>,<redacted>]
[x] RowSet(217)( 1M) [0.0027, 0.1953] [<redacted>,<redacted>]
[x] RowSet(216)( 1M) [0.5341, 0.5341] [<redacted>,<redacted>]
[x] RowSet(218)( 1M) [0.5341, 0.5341] [<redacted>,<redacted>]
[x] RowSet(219)( 1M) [0.5341, 0.5341] [<redacted>,<redacted>]
3. Compaction that takes into consideration:
- Base data
- Redo deltas
Compaction budget (tablet_compaction_budget_mb) is default 128 MB
Result: Rowset 211 with size 1M (ignoring UNDO deltas) included.
Budgeted compaction selection:
[x] RowSet(211)( 1M) [0.0000, 1.0000] [<redacted>,<redacted>]
[x] RowSet(217)( 1M) [0.0026, 0.2015] [<redacted>,<redacted>]
[x] RowSet(216)( 1M) [0.5358, 0.5385] [<redacted>,<redacted>]
[x] RowSet(218)( 1M) [0.5385, 0.5404] [<redacted>,<redacted>]
[x] RowSet(219)( 1M) [0.5404, 0.5404] [<redacted>,<redacted>]
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 32 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/23348/2
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG@7
PS2, Line 7: [compaction]
> Does it make sense to add a particular JIRA item tag here, e.g., KUDU-3406,
Please reference an appropriate upstream ticket for this bug.
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jan 2026 14:18:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 3:
(5 comments)
Apologies for being late on this. I thought I had responded already. Either, I missed to hit send button or responses didn't get saved.
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG@10
PS3, Line 10: taking into consideration the size of undo deltas while picking rowsets
: during rowset compaction. I could not find any historical reason to why
: it was not done before. Maybe there was some analysis done that ended
: in a conclusion that considering undo deltas may not be right approach
: when estimating upper bound fractional solution in the knapsack.
> I don't think that was something to do with the nature of the fractional kn
This algorithm is only applicable to rowset merge compaction. I don't see this having any role in major or minor delta compaction ops.
Even if there was any partial application or use somewhere in major or minor delta compaction, I would expect it would be different for both, such as inclusion of just the redo deltas for minor delta compaction and inclusion of both base data as well redo deltas for major delta compaction.
For major delta compaction op, the scheduling decision based calculation makes use of redo_deltas_size and base_data_size directly without considering undo deltas at all.
Similarly, for minor delta compaction op, delta store count is taken into consideration instead of redo/undo delta size or base data size.
I hope this clarifies the confusion.
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG@38
PS3, Line 38: [ ] RowSet(217)( 1M) [0.0027, 0.1953]
> I think this is not exactly related to the essence of the patch, but do you
Basically, what is happening here is that rowset 217 is ranging from 0.0027 to 0.1953 which is increasing the overall width (by almost 100 times) of the resultant rowset with possibility of increased overlaps in future. From compaction policy goal, this is not desirable and highly inefficient. If rowset 217 is not part of selected rowsets, the resultant rowset would have high number of overlaps (i.e. 216, 218 and 219) for small width/range which is highly efficient.
We don't want to end up with unnecessarily wide rowsets with average height 1, instead a number of non-overlapping rowsets with average height 1 is better.
For below case (2048 MB) where budget is beyond the total size of deltas, rowset 211 can be picked even. That changes the equation a little bit. Now, the overlap between 211 and 217 makes it an attractive candidate. Even though resultant rowset will be wide, the average rowset height (consulted during future ops) would remain minimum. Not to mention, 211 overlaps with all the other rowsets as well.
http://gerrit.cloudera.org:8080/#/c/23348/3/src/kudu/tablet/rowset_info.h
File src/kudu/tablet/rowset_info.h:
http://gerrit.cloudera.org:8080/#/c/23348/3/src/kudu/tablet/rowset_info.h@92
PS3, Line 92: redos_and_undos
> nit for here and elsewhere: similar to method names in other interfaces, c
Done
http://gerrit.cloudera.org:8080/#/c/23348/3/src/kudu/tablet/rowset_info.h@142
PS3, Line 142: base_redos_and_undos_size_mb_
> nit: base_and_deltas_size_mb_
Done
http://gerrit.cloudera.org:8080/#/c/23348/3/src/kudu/tablet/rowset_info.h@166
PS3, Line 166: base_redos_and_undos_size_bytes
> nit: ditto
Done
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Jan 2026 17:52:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23348
to look at the new patch set (#8).
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
KUDU-3734 include undo size while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The test data used here has significant undo deltas that fits perfectly
into the OOM scenario. Total size of uncompacted data is 29GB.
With this patch, rowset compaction never hit OOM and the resident
memory kept well within limits (~1GB).
Without this patch, compaction hit OOM on a node with limited memory
where rowset compaction peak memory touched ~30GB.
Testing with different scenarios:
1. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 1024 MB
Result: Rowset 211 was skipped due to 1024 MB size constraint.
Budgeted compaction selection:
[ ] RowSet(211)( 1945M) [0.0000, 1.0000]
[ ] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
2. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 2048 MB
Result: Rowset 211 was NOT skipped with 2048 MB size limit.
Budgeted compaction selection:
[x] RowSet(211)( 1945M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
3. Compaction that takes into consideration:
- Base data
- Redo deltas
Compaction budget (tablet_compaction_budget_mb) is default 128 MB
Result: Rowset 211 with size 1M (ignoring UNDO deltas) included.
Budgeted compaction selection:
[x] RowSet(211)( 1M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0026, 0.2015]
[x] RowSet(216)( 1M) [0.5358, 0.5385]
[x] RowSet(218)( 1M) [0.5385, 0.5404]
[x] RowSet(219)( 1M) [0.5404, 0.5404]
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 98 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/23348/8
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23348/6//COMMIT_MSG
Commit Message:
PS6:
> Sure!
https://issues.apache.org/jira/browse/KUDU-3741
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Feb 2026 06:20:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 10: Verified+1
ASAN, TSAN: unrelated failures of known flaky tests (KUDU-3481, etc.)
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 10
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 26 Feb 2026 19:26:00 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/23348/8/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
http://gerrit.cloudera.org:8080/#/c/23348/8/src/kudu/tablet/tablet.cc@110
PS8, Line 110: DEFINE_int32(tablet_compaction_budget_mb, 256,
: "Budget for a single compaction. Consider setting this back to 128 if "
: "rowset_merge_compaction_weight_includes_undo_deltas is set to false.");
> Just curious: did you do any particular evaluation to come up with 128 -->
Yes. For more information, please refer to my response to your comment #1 in PS6.
http://gerrit.cloudera.org:8080/#/c/23348/8/src/kudu/tablet/tablet.cc@2245
PS8, Line 2245: // Check if the single rowset is effectively empty based on the weight configuration
: const bool is_empty_rowset = FLAGS_rowset_merge_compaction_weight_includes_undo_deltas ?
: input.rowsets()[0]->OnDiskBaseDataSizeWithDeltas() == 0 :
: input.rowsets()[0]->OnDiskBaseDataSizeWithRedos() == 0;
> What if there aren't any rowsets? Could this trigger an UB or SIGSEGV then
Good catch! You are right.
Fixed.
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Feb 2026 06:20:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 6:
> (3 comments)
>
> Looks good to me from the code-related perspective.
>
> However, there are a few very important follow-up action items
> here:
>
> 0. Should there be a flag to switch between the former and the new
> rowset merge compaction budgeting approach? In the items below
> there is more context on why it might be a good idea.
Yes, I agree. I have added a flag to switch between old and new approach. This should give customers enough flexibility to change the behavior if new approach turns out to be problematic in their workload.
>
> 1. It's necessary to re-evaluate the default setting for the
> --tablet_compaction_budget_mb flag. Since the accumulated delta
> history might be quite large (by current default the system keeps 7
> days of delta history) and the progress in minor/major delta
> compactions does not help with reducing the the rowset estimated
> 'size' once the UNDO deltas are taking into account, I suspect that
> the updated policy could seize up already existing systems in
> production, so no rowset merge compaction happening at all.
With a workload that has steady and frequent mix of updates and inserts, I haven’t noticed 128MB of budget causing any issues, mainly because of the fact that the workload was not putting enough ‘stress’ to generate UNDO deltas at the desired rate. Nevertheless, the flag needs to be adjusted for default setting.
So, I did run number of perf loadgen tests to generate frequent redo and undo deltas with additional debug logs that clearly print separate sizes for undo, redo deltas and base data.
Averaging out the proportionate use of —tablet_compaction_budget_mb (i.e. 128MB) for all the samples, it is observed that base data size can be accounted for 96.8% of 128MB, while redo delta size can be 3.2%. Also, undo size is roughly close to base data size in such workload types. With that logic, around 97% bump in default setting for —tablet_compaction_budget_mb (128MB) sounds reasonable i.e. 256GB (after rounding up to the next power of two). This can be a good starting point and we can certainly adjust the value in future if need be.
>
> 2. On the related note, it's necessary to test how the compaction
> behaves on real workloads with this updated policy. Is there a
> risk of the compaction activity falling behind and eventually not
> compacting a single rowset because of long tail of accumulated UNDO
> delta history?
I ran a few workload combinations (using perf loadgen) with and without the accounting of undo deltas.
For all except one([1]), there is no evidence of compaction avoidance due to this patch.
A few examples are as follows:
- Rows with frequent updates (2500 rows x 4 threads) and every background operation running along with it.
- Rows with frequent updates (2500 rows x 4 threads), with all three compactions disabled.
Then, restart the tablet servers with all three enabled compactions.
This is to see test the behaviour on large number of un-applied redo deltas.
- Rows with frequent updates (2500 rows x 4 threads), with rowset merge compaction disabled.
Then, restart the tablet servers with enabled rowset merge compactions.
This is to see test the behaviour on accumulated undo deltas.
- [1] Rows with frequent updates (1000 rows x 10 threads), with —flush_threshold_secs=1 and —flush_threshold_mb=1 to enforce frequent flushes
and more participating rowsets during compaction approximation for picking rowsets.
This generated undo deltas of total size worth ~2GB per tablet server for a table.
In essence, although this not an usual scenario because it requires so many conditions to be true at a time
i.e. frequent flushes, rowset compaction not running for long duration, high frequency of updates but it is close to the OOM case
and hence avoidance is expected here.
- Rows with frequent updates (start with 2500 at first then dynamically increase rows by factor of 10 in each iteration). This creates
inserts at high rates as compared updates, in terms of disk usage.
>
> 3. How does this affect small rowset compaction (see KUDU-1400)?
> Should we expect any surprises once this update deployed on systems
> running real world workloads?
I don’t think this has any significant impact on small rowset compaction. This change specifically targets rowset merge compaction where goal is to reduce height of overlapping rowsets. However, ‘small rowset compaction’ is applicable to large number of small rowsets with keys in increasing order. One will have continuous updates on base data, followed by frequent DMS flushes and hence accumulation undo deltas. While other (small rowset compaction) will have continuous insertion of rows that form more number of small rowsets with little or no overlap. Additionally, I ran multiple successful instances of all the small rowset tests from compaction_policy-test.
>
> As a realistic workload, consider playing with YCSB or similar.
> Just running 'kudu perf loadgen ... --num_rows_per_thread=-1 ...'
> for a day or two with UPSERT enabled might be another workload to
> validate this new policy. You might find useful information on
> running YCSB workloads against a Kudu cluster in changelist
> b915df7f335ec947cfc8339aa8f59be0188e4469. There are many other
> changelists in the git history mentioning YCSB and related
> experiments that were performed when making similar changes in the
> past.
YCSB workload ran fine for more than an hour for both the cases - with and without the taking into account UNDO deltas.
The command looked like this (arguments may not be accurate as I had to adjust it further to enforce definitive runtime):
./bin/ycsb.sh load kudu -P workloads/workloada -s -p kudu_master_addresses="127.0.0.1:8764" -p recordcount=1000000 -p table=workloada -p kudu_table_num_replicas=3
./bin/ycsb.sh run kudu -P workloads/workloada -s -p kudu_master_addresses="127.0.0.1:8764" -p operationcount=10000000 -p table=workloada -p readproportion=0.2 -p updateproportion=0.8
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Feb 2026 06:06:56 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 3: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2199/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jan 2026 14:41:53 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 3:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2199/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jan 2026 13:27:59 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kurt Deschler, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23348
to look at the new patch set (#3).
Change subject: [compaction] include undo size while picking rowsets
......................................................................
[compaction] include undo size while picking rowsets
As part of compaction improvement effort, this change is focussing on
taking into consideration the size of undo deltas while picking rowsets
during rowset compaction. I could not find any historical reason to why
it was not done before. Maybe there was some analysis done that ended
in a conclusion that considering undo deltas may not be right approach
when estimating upper bound fractional solution in the knapsack. The
size of undo deltas are taken into consideration while deciding whether
a new rowset (a potential compaction 'candidate') fits into the budget
and is at least denser than least dense candidate in knapsack.
The test data used here has significant undo deltas that fits perfectly
into the OOM scenario. Total size of uncompacted data is 29GB.
With this patch, rowset compaction never hit OOM and the resident
memory kept well within limits (~1GB).
Without this patch, compaction hit OOM on a node with limited memory
where rowset compaction peak memory touched ~30GB.
Testing with different scenarios:
1. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 1024 MB
Result: Rowset 211 was skipped due to 1024 MB size constraint.
Budgeted compaction selection:
[ ] RowSet(211)( 1945M) [0.0000, 1.0000]
[ ] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
2. Compaction that takes into consideration:
- Base data
- Redo deltas
- Undo delats
Budget for compaction (tablet_compaction_budget_mb) is 2048 MB
Result: Rowset 211 was NOT skipped with 2048 MB size limit.
Budgeted compaction selection:
[x] RowSet(211)( 1945M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0027, 0.1953]
[x] RowSet(216)( 1M) [0.5341, 0.5341]
[x] RowSet(218)( 1M) [0.5341, 0.5341]
[x] RowSet(219)( 1M) [0.5341, 0.5341]
3. Compaction that takes into consideration:
- Base data
- Redo deltas
Compaction budget (tablet_compaction_budget_mb) is default 128 MB
Result: Rowset 211 with size 1M (ignoring UNDO deltas) included.
Budgeted compaction selection:
[x] RowSet(211)( 1M) [0.0000, 1.0000]
[x] RowSet(217)( 1M) [0.0026, 0.2015]
[x] RowSet(216)( 1M) [0.5358, 0.5385]
[x] RowSet(218)( 1M) [0.5385, 0.5404]
[x] RowSet(219)( 1M) [0.5404, 0.5404]
Note: This is different from rowset compaction batching effort.
Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
---
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/compaction_policy.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_info.cc
M src/kudu/tablet/rowset_info.h
M src/kudu/tablet/tablet.cc
11 files changed, 35 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/23348/3
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 10
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23348/6//COMMIT_MSG
Commit Message:
PS6:
> https://issues.apache.org/jira/browse/KUDU-3741
KUDU-3741
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Feb 2026 06:22:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 6: Code-Review+1
Yep, that answers the question. Thanks!
I don't have any question here apart from the ones Alexey flagged.
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Fri, 06 Feb 2026 18:11:04 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 6: Code-Review+1
(3 comments)
Looks good to me from the code-related perspective.
However, there are a few very important follow-up action items here:
0. Should there be a flag to switch between the former and the new rowset merge compaction budgeting approach? In the items below there is more context on why it might be a good idea.
1. It's necessary to re-evaluate the default setting for the --tablet_compaction_budget_mb flag. Since the accumulated delta history might be quite large (by current default the system keeps 7 days of delta history) and the progress in minor/major delta compactions does not help with reducing the the rowset estimated 'size' once the UNDO deltas are taking into account, I suspect that the updated policy could seize up already existing systems in production, so no rowset merge compaction happening at all.
2. On the related note, it's necessary to test how the compaction behaves on real workloads with this updated policy. Is there a risk of the compaction activity falling behind and eventually not compacting a single rowset because of long tail of accumulated UNDO delta history?
3. How does this affect small rowset compaction (see KUDU-1400)? Should we expect any surprises once this update deployed on systems running real world workloads?
As a realistic workload, consider playing with YCSB or similar. Just running 'kudu perf loadgen ... --num_rows_per_thread=-1 ...' for a day or two with UPSERT enabled might be another workload to validate this new policy. You might find useful information on running YCSB workloads against a Kudu cluster in changelist b915df7f335ec947cfc8339aa8f59be0188e4469. There are many other changelists in the git history mentioning YCSB and related experiments that were performed when making similar changes in the past.
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG@10
PS3, Line 10: taking into consideration the size of undo deltas while picking rowsets
: during rowset compaction. I could not find any historical reason to why
: it was not done before. Maybe there was some analysis done that ended
: in a conclusion that considering undo deltas may not be right approach
: when estimating upper bound fractional solution in the knapsack.
> This algorithm is only applicable to rowset merge compaction. I don't see t
I took a closer look at the compaction code and it turned out you were right: the knapsack solver is used only for picking rowsets for merge compaction (essentially, it's usage is limited to BudgetedCompactionPolicy::PickRowSets()).
After some more spelunking, from the logs in the git repo I found that some time ago other people were also puzzled by only redo deltas were considered when assessing the size of a rowset: e.g. take a look at the comment of changelist 3e3bd1ccbc2b4b070c733b36b1971de63977428b
With that, it's not clear to me why undo deltas weren't originally accounted for, either.
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG@38
PS3, Line 38: [ ] RowSet(217)( 1M) [0.0027, 0.1953]
> Basically, what is happening here is that rowset 217 is ranging from 0.0027
Thanks a lot for the explanation; that makes sense, indeed.
http://gerrit.cloudera.org:8080/#/c/23348/6//COMMIT_MSG
Commit Message:
PS6:
Do you mind adding a test to track regressions if anything changes and the compaction starts picking up rowsets beyond the budget again?
It's OK with me if doing that in a separate changelist. The crucial point is making sure the fixed/updated behavior doesn't regress back to the original if changing the related code in future.
Thank you!
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Jan 2026 21:21:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 8:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/23348/6//COMMIT_MSG
Commit Message:
PS6:
> KUDU-3741
Thank you!
http://gerrit.cloudera.org:8080/#/c/23348/8/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
http://gerrit.cloudera.org:8080/#/c/23348/8/src/kudu/tablet/tablet.cc@110
PS8, Line 110: DEFINE_int32(tablet_compaction_budget_mb, 256,
: "Budget for a single compaction. Consider setting this back to 128 if "
: "rowset_merge_compaction_weight_includes_undo_deltas is set to false.");
Just curious: did you do any particular evaluation to come up with 128 --> 256 update?
Thanks!
http://gerrit.cloudera.org:8080/#/c/23348/8/src/kudu/tablet/tablet.cc@2245
PS8, Line 2245: // Check if the single rowset is effectively empty based on the weight configuration
: const bool is_empty_rowset = FLAGS_rowset_merge_compaction_weight_includes_undo_deltas ?
: input.rowsets()[0]->OnDiskBaseDataSizeWithDeltas() == 0 :
: input.rowsets()[0]->OnDiskBaseDataSizeWithRedos() == 0;
What if there aren't any rowsets? Could this trigger an UB or SIGSEGV then?
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Feb 2026 21:42:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc
File src/kudu/tablet/rowset_info.cc:
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc@475
PS9, Line 475: if (FLAGS_rowset_merge_compaction_weight_includes_undo_deltas) {
: extra_->base_and_deltas_size_bytes = rs->OnDiskBaseDataSizeWithDeltas();
: } else {
: extra_->base_and_deltas_size_bytes = rs->OnDiskBaseDataSizeWithRedos();
: }
> Why to have this logic around FLAGS_rowset_merge_compaction_weight_includes
Basically, there are couple of reasons:
1. The idea is to ensure clarity and uniformity in what the function name says and what it does.
2. The purpose of RowSetInfo class is to cache the stats for compaction policy while OnDiskBaseDataSizeWithDeltas is a method specific to RowSet class and has no knowledge whatsoever about how callers (compaction) are using the result from this function.
While encapsulating the details into the implementation of OnDiskBaseDataSizeWithDeltas would certainly make it simpler and readable, it would also introduce the ambiguity in case OnDiskBaseDataSizeWithDeltas is used for a different purpose in future that should not depend on whether FLAGS_rowset_merge_compaction_weight_includes_undo_deltas.
Additionally, this if/else logic based on the flag is present at only two places: here and in compaction_policy-test.c (where its purpose is to come up with a total size value that doesn't exceed budget limit).
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 9
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Feb 2026 07:29:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP [compaction] include undo deltas while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: WIP [compaction] include undo deltas while picking rowsets
......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/1850/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 24 Sep 2025 10:45:55 +0000
Gerrit-HasComments: No
[kudu-CR] WIP [compaction] include undo deltas while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: WIP [compaction] include undo deltas while picking rowsets
......................................................................
Patch Set 1: -Verified
Build Started http://jenkins.kudu.apache.org/job/pre_commit/1850/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 24 Sep 2025 08:44:58 +0000
Gerrit-HasComments: No
[kudu-CR] WIP [compaction] include undo deltas while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: WIP [compaction] include undo deltas while picking rowsets
......................................................................
Patch Set 1:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/1714/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 26 Aug 2025 13:36:00 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 8:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2295/
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Feb 2026 12:27:09 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 2:
(6 comments)
Thanks for taking care of this issue.
I took a quick first look at the patch; I'm planning to take another one soon. Overall, it looks OK at the first glance.
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG@7
PS2, Line 7: [compaction]
Does it make sense to add a particular JIRA item tag here, e.g., KUDU-3406, KUDU-3429 or alike? Alternatively, if you have an umbrella item for compaction performance improvement effort, maybe use the corresponding JIRA ticket here?
At least, I'd think to mention KUDU-3406 in the description below, so people understand that this fix is related to the functionality introduced with KUDU-3406, and they need to pick up this patch to allow the rowset compaction memory budgeting working as expected.
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG@22
PS2, Line 22: With this patch, rowset compaction never hit OOM and the resident
: memory kept well within limits (~1GB).
I guess the tests were run with the compaction memory budgeting turned on? Would be great to mention that, and also mention the setting for the memory budget since the memory budgeting for rowset compaction isn't enabled by default, IIRC.
http://gerrit.cloudera.org:8080/#/c/23348/2//COMMIT_MSG@37
PS2, Line 37: [<redacted>,<redacted>]
nit for here and below: if this isn't adding any useful information, maybe drop this part of the output?
http://gerrit.cloudera.org:8080/#/c/23348/2/src/kudu/tablet/rowset.h
File src/kudu/tablet/rowset.h:
http://gerrit.cloudera.org:8080/#/c/23348/2/src/kudu/tablet/rowset.h@206
PS2, Line 206: or UNDO deltas
here and elsewhere: please revise and update the comments where necessary
http://gerrit.cloudera.org:8080/#/c/23348/2/src/kudu/tablet/rowset.h@207
PS2, Line 207: RedosAndUndos
here and elsewhere: change 'RedosAndUndos' into 'Deltas' for brevity?
http://gerrit.cloudera.org:8080/#/c/23348/2/src/kudu/tablet/rowset.h@449
PS2, Line 449: not including UNDOs
ditto
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 06 Jan 2026 23:04:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 5: Verified+1
Build Successful
http://jenkins.kudu.apache.org/job/pre_commit/2223/ : SUCCESS
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Jan 2026 10:15:53 +0000
Gerrit-HasComments: No
[kudu-CR] [compaction] include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: [compaction] include undo size while picking rowsets
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG@10
PS3, Line 10: taking into consideration the size of undo deltas while picking rowsets
: during rowset compaction. I could not find any historical reason to why
: it was not done before. Maybe there was some analysis done that ended
: in a conclusion that considering undo deltas may not be right approach
: when estimating upper bound fractional solution in the knapsack.
I don't think that was something to do with the nature of the fractional knapsack algorithm and the boundaries for a solution.
IIUC, the same algorithm is used to get the most bang on the same budget among all possible maintenance ops on the existing rowsets. In other words, this is used not only for rowset merge compaction, but also for minor and major delta compaction.
In this context, it's quite clear why to include only redo deltas, and not undo deltas. This is related to heuristics on the amount of IO necessary to build the current value given the base data and the redo deltas to apply. The most frequement requests are for the very latest data, not some data in the past, and a part of that information (i.e. whether a row for given key is present) is also necessary when processing DML operations like INSERT/UPDATE/DELETE/UPSERT and their xxx_IGNORE flavors. IIUC, that's the reason why it makes sense to use only the size of redo deltas when trying to find the 'worst' rowsets to perform minor/major delta compaction on them.
Does this make sense?
If yes, then I'm curious what are your thoughts on the IO-related heuristics in the context of selecting the 'worst' rowset for minor/major delta compaction, and whether those heuristics are now skewed once introducing a long tail of redo deltas with the introduction of this update (IIRC, by default the system keeps 7 days of delta history).
Thanks!
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG@38
PS3, Line 38: [ ] RowSet(217)( 1M) [0.0027, 0.1953]
I think this is not exactly related to the essence of the patch, but do you have any reasonable explanation why RowSet 217 wasn't picked up with 1024MB budget, but was picked up when the budget was set to 2048MB?
http://gerrit.cloudera.org:8080/#/c/23348/3/src/kudu/tablet/rowset_info.h
File src/kudu/tablet/rowset_info.h:
http://gerrit.cloudera.org:8080/#/c/23348/3/src/kudu/tablet/rowset_info.h@92
PS3, Line 92: redos_and_undos
nit for here and elsewhere: similar to method names in other interfaces, consider changing this 'redos_and_undos' --> 'and_deltas'
http://gerrit.cloudera.org:8080/#/c/23348/3/src/kudu/tablet/rowset_info.h@142
PS3, Line 142: base_redos_and_undos_size_mb_
nit: base_and_deltas_size_mb_
http://gerrit.cloudera.org:8080/#/c/23348/3/src/kudu/tablet/rowset_info.h@166
PS3, Line 166: base_redos_and_undos_size_bytes
nit: ditto
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jan 2026 04:24:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 6:
(2 comments)
> Do we need to consider the undo delta's older than ancient history
> mark? Could there be a race between them GC'd vs being compacted?
Each rowset is protected by a lock(compact_flush_lock_) while an on-going modifying maintenance op such as GC, compaction or flush. Once a rowset is selected for compaction, the lock remains until compaction is done with both load and commit phase. Rowset compaction also gc's ancient undo deltas (if any). Does that answer your question?
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23348/3//COMMIT_MSG@10
PS3, Line 10: taking into consideration the size of undo deltas while picking rowsets
: during rowset compaction. I could not find any historical reason to why
: it was not done before. Maybe there was some analysis done that ended
: in a conclusion that considering undo deltas may not be right approach
: when estimating upper bound fractional solution in the knapsack.
> I took a closer look at the compaction code and it turned out you were righ
Ack
http://gerrit.cloudera.org:8080/#/c/23348/6//COMMIT_MSG
Commit Message:
PS6:
> Do you mind adding a test to track regressions if anything changes and the
Sure!
Let me take this up in a separate changelist. I have made a note.
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 05 Feb 2026 12:30:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3734 include undo size while picking rowsets
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23348 )
Change subject: KUDU-3734 include undo size while picking rowsets
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc
File src/kudu/tablet/rowset_info.cc:
http://gerrit.cloudera.org:8080/#/c/23348/9/src/kudu/tablet/rowset_info.cc@475
PS9, Line 475: if (FLAGS_rowset_merge_compaction_weight_includes_undo_deltas) {
: extra_->base_and_deltas_size_bytes = rs->OnDiskBaseDataSizeWithDeltas();
: } else {
: extra_->base_and_deltas_size_bytes = rs->OnDiskBaseDataSizeWithRedos();
: }
The idea was to encapsulate the logic into the RowSet::OnDiskBaseDataSizeWithDeltas() implementation. In earlier revisions (e.g., at least in PS6), there is already a version of RowSet that behaved unconditionally different by changing the logic and renaming OnDiskBaseDataSizeWithRedos() into OnDiskBaseDataSizeWithDeltas(). It might be a good venue to introduce a flag that would control the behavior, making it configurable. The RowSetInfo's logic is downstream from the one that RowSet has, and that's great to have no exposure to that logic in the RowSetInfo's implementation.
Aside from hypothetical considerations that OnDiskBaseDataSizeWithRedos() might be needed in future, it seems there isn't any other reason to keep OnDiskBaseDataSizeWithRedos() and OnDiskBaseDataSizeWithDeltas() as two separate methods in the current code, right?
If so, then according to the YAGNI principle https://martinfowler.com/bliki/Yagni.html it's better to keep just OnDiskBaseDataSizeWithDeltas().
Having this observations in mind, what do you think of renaming 'rowset_merge_compaction_weight_includes_undo_deltas' into 'rowset_deltas_size_include_undo' and encapsulating the logic of including the size of undo deltas into the OnDiskBaseDataSizeWithDeltas() implementation?
> Additionally, this if/else logic based on the flag is present at only two places: here and in compaction_policy-test.c (where its purpose is to come up with a total size value that doesn't exceed budget limit).
What about tablet.cc? IIUC, the UB/SIGSEGV condition which could have been introduced in PS8 (https://gerrit.cloudera.org/#/c/23348/8/src/kudu/tablet/tablet.cc@2248) appeared in an attempt to duplicate the logic outside of the RowSet class.
--
To view, visit http://gerrit.cloudera.org:8080/23348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I351c0ba96a02e6ded5153adf9d981083a8c40592
Gerrit-Change-Number: 23348
Gerrit-PatchSet: 9
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Feb 2026 21:36:08 +0000
Gerrit-HasComments: Yes