Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/22482 )
Change subject: IMPALA-13751: Fix runtime-profile-test failure since IMPALA-13304 ...................................................................... Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/22482/8/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/22482/8/be/src/util/runtime-profile-test.cc@2034 PS8, Line 2034: // Retry until maximum setup retries : if (!EVENT_COMPLETENESS && missing_event_instances_expected.size() <= 0 : && setup_retries < MAX_SETUP_RETRIES) { As I said, there is very less to no reason to believe the test will not generate missing events. This seems to be the standard procedure for retrying a test fixture's setup, by having a maximum retry Even before implementing this, and in the initial commit IMPALA-13304, I had already thought about this. > This actually runs for each instance and for each event. Also, we had mutiple > SetUp and TearDowns to ensure this. > events_count x num of profiles x 2 setups and teardowns For example, Before implementing this change, even then it was a very discrete possibility. events count = 6 and expecting num of profiles = 1, the possibility of such an event is (0.5)^6 / 30 or 0.00052 or a maximum of 1 in 10000 cases. Even then the test would not fail, there just won't be missing events. And with this change, that becomes, (0.5)^18 / (30)^3 or 0.0000000001 or 1 in 10 Billion cases. In both cases, this is even without considering the 2nd tear down with 0.8 present. Also, we run the tests many times across different types of builds. There is no way a test can go through without missing events. It would be unreasonable to cover a case with probability equivalent to CPU's probability of skipping an instruction. http://gerrit.cloudera.org:8080/#/c/22482/8/be/src/util/runtime-profile-test.cc@2100 PS8, Line 2100: void Validate() { > Sorry if I'm not clear, but I simply ask for something this: Please see the changes on L2009, L2018 and L2133. I have already added the same, similar to BUCKET_SIZE or NUM_PROFILES. -- To view, visit http://gerrit.cloudera.org:8080/22482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If04744e215cac79f255b3d73c3e91e873c13749a Gerrit-Change-Number: 22482 Gerrit-PatchSet: 8 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Wed, 19 Feb 2025 23:08:53 +0000 Gerrit-HasComments: Yes
