Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24412 )
Change subject: KUDU-3429 Add extensive test for delta generation (3/n) ...................................................................... Patch Set 4: Code-Review+1 (2 comments) Overall LGTM; but please address the question on using STL-based random generators. If it's preferred to use the custom code, please add a comment to explain the reason. http://gerrit.cloudera.org:8080/#/c/24412/2/src/kudu/tablet/compaction-highmem-test.cc File src/kudu/tablet/compaction-highmem-test.cc: http://gerrit.cloudera.org:8080/#/c/24412/2/src/kudu/tablet/compaction-highmem-test.cc@546 PS2, Line 546: ASSERT_OK(tablet()->Compact(Tablet::COMPACT_NO_FLAGS)); : } : sw.stop(); > There is no threshold expectation here. The assert is to just record the fa Thank you for the clarification. I agree that assertion is required here for the memory amount overshoot. http://gerrit.cloudera.org:8080/#/c/24412/4/src/kudu/tablet/compaction-highmem-test.cc File src/kudu/tablet/compaction-highmem-test.cc: http://gerrit.cloudera.org:8080/#/c/24412/4/src/kudu/tablet/compaction-highmem-test.cc@326 PS4, Line 326: kLargePayload[i] = static_cast<char>((i * 193U + 37U) % 251U); > What is your problem with the usual std::mt19937? +1 for using STL-based random generators if it works here as expected -- To view, visit http://gerrit.cloudera.org:8080/24412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c337dba87a9d1cf549f9b53813da8d50e86b6b4 Gerrit-Change-Number: 24412 Gerrit-PatchSet: 4 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: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Mon, 03 Aug 2026 16:31:53 +0000 Gerrit-HasComments: Yes
