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: (2 comments) Overall looks good to me. Just one small comment. 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@647 PS1, Line 647: TestMemoryEstimation You could also add an ASSERT_STR_CONTAINS/ASSERT_STR_NOT_CONTAINS at the end of both conditions to verify that this rowset was/was not picked because of memory budget constraints by matching with this pattern -> "removed from compaction input due to memory constraints" In case you do that, keep in mind that this error log is throttled. So you might see this error just for the first rowset. 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 > The idea was that what if it gets enabled by default later? Then the test w Never mind! That should be ok. -- 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-Reviewer: Ádám Bakai <[email protected]> Gerrit-Comment-Date: Tue, 19 Dec 2023 09:32:24 +0000 Gerrit-HasComments: Yes
