Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20787 )
Change subject: [compaction] Add memory estimation unit test ...................................................................... Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/20787/6/src/kudu/tablet/compaction_policy-test.cc File src/kudu/tablet/compaction_policy-test.cc: http://gerrit.cloudera.org:8080/#/c/20787/6/src/kudu/tablet/compaction_policy-test.cc@651 PS6, Line 651: *1024*1024 > style nit for here and below: separate operation symbol from the operands b One more point that Ashwani pointed to. With trying to set FLAGS_memory_limit_hard_bytes, there are two issues: * Once it's set, the hard memory limit is modified for all the tests in this binary, and the setting isn't changing back even if rolling back the flag's setting. That's might lead to very unexpected outcomes, especially if randomizing the set of scenarios to run (i.e. when using `--gtest_shuffle`). * From the other side, is this setting at line 651 actually effective (i.e. does it set the hard memory limit as expected)? Did you check that InitLimits() hasn't been called earlier when running this test scenario? -- 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: 6 Gerrit-Owner: Ádám Bakai <aba...@cloudera.com> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Ashwani Raina <ara...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Ádám Bakai <aba...@cloudera.com> Gerrit-Comment-Date: Tue, 30 Jan 2024 06:16:15 +0000 Gerrit-HasComments: Yes
