Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/20787 )
Change subject: [compaction] Add memory estimation unit test ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/20787/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20787/1//COMMIT_MSG@7 PS1, Line 7: Add memory estimation unit test Maybe add more details about how changing hard limit helps cover test scenario for memory budgeting logic. http://gerrit.cloudera.org:8080/#/c/20787/1//COMMIT_MSG@9 PS1, Line 9: FLAGS_rowset_compaction_estimate_min_deltas_size_mb Did you mean rowset_compaction_memory_estimate_enabled? rowset_compaction_estimate_min_deltas_size_mb is for minimum deltas size and initialised to 64MB default. http://gerrit.cloudera.org:8080/#/c/20787/1/src/kudu/tablet/compaction_policy-test.cc File src/kudu/tablet/compaction_policy-test.cc: http://gerrit.cloudera.org:8080/#/c/20787/1/src/kudu/tablet/compaction_policy-test.cc@649 PS1, Line 649: FLAGS_rowset_compaction_memory_estimate_enabled Isn't this disabled by default? No need to set it to false here unless it could be true here for some other reason. http://gerrit.cloudera.org:8080/#/c/20787/1/src/kudu/tablet/compaction_policy.cc File src/kudu/tablet/compaction_policy.cc: http://gerrit.cloudera.org:8080/#/c/20787/1/src/kudu/tablet/compaction_policy.cc@182 PS1, Line 182: { Move above to align with other instances. http://gerrit.cloudera.org:8080/#/c/20787/1/src/kudu/tablet/rowset.cc File src/kudu/tablet/rowset.cc: http://gerrit.cloudera.org:8080/#/c/20787/1/src/kudu/tablet/rowset.cc@266 PS1, Line 266: for (const shared_ptr<RowSet> &rs : new_rowsets_) OnDiskDeltaSize used in compaction is per rowset. Here it seems to be computing total size for all rowsets inside new_rowsets_. Where is it getting used? -- To view, visit http://gerrit.cloudera.org:8080/20787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5397c7e7bf29942a610b0b8203b6918811c49bbb Gerrit-Change-Number: 20787 Gerrit-PatchSet: 1 Gerrit-Owner: Ádám Bakai <[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: Wed, 13 Dec 2023 14:52:50 +0000 Gerrit-HasComments: Yes
