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 <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 06 Jan 2026 23:04:50 +0000 Gerrit-HasComments: Yes
