Riza Suminto 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:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/22482/3/be/src/util/runtime-profile-test.cc
File be/src/util/runtime-profile-test.cc:

http://gerrit.cloudera.org:8080/#/c/22482/3/be/src/util/runtime-profile-test.cc@1961
PS3, Line 1961: id SetUp(float events_completeness, bo
> Fix to 50.
Resolved stale comment.


http://gerrit.cloudera.org:8080/#/c/22482/3/be/src/util/runtime-profile-test.cc@1968
PS3, Line 1968: PROFILES = uni_dis_instances(gen);
> Fix to 5.
Resolved stale comment.


http://gerrit.cloudera.org:8080/#/c/22482/3/be/src/util/runtime-profile-test.cc@1977
PS3, Line 1977: else {
> Fix to 50.
Resolved stale comment.


http://gerrit.cloudera.org:8080/#/c/22482/5/be/src/util/runtime-profile-test.cc
File be/src/util/runtime-profile-test.cc:

http://gerrit.cloudera.org:8080/#/c/22482/5/be/src/util/runtime-profile-test.cc@1960
PS5, Line 1960:
> This was the reason we had mutiple SetUp and TearDowns.
Resolved stale comment.


http://gerrit.cloudera.org:8080/#/c/22482/5/be/src/util/runtime-profile-test.cc@2023
PS5, Line 2023: nt i = 0; i < NUM_PROFILES - 1
> This actually runs for each instance and for each event. Also,  we had muti
Resolved stale comment.


http://gerrit.cloudera.org:8080/#/c/22482/5/be/src/util/runtime-profile-test.cc@2029
PS5, Line 2029:
> Please let me know if I should increase the number of SetUp and TearDown to
Resolved stale comment.


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) {
What happen if SetUp gives up here?

Does the MissingEvents test simply not a MissingTests anymore?


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:

  void Validate(bool expect_complete_event) {
    EXPECT_TRUE(expect_complete_event == 
missing_event_instances_expected.empty());



--
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 21:58:46 +0000
Gerrit-HasComments: Yes

Reply via email to