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 <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[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-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Mon, 23 Feb 2026 21:42:49 +0000 Gerrit-HasComments: Yes
