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 <[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: Wed, 25 Feb 2026 07:29:10 +0000 Gerrit-HasComments: Yes
