Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9749 )
Change subject: IMPALA-6694: fix "buffer pool" child profile order ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/9749/3/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/9749/3/be/src/util/runtime-profile-test.cc@250 PS3, Line 250: child 2 > child 1? Done http://gerrit.cloudera.org:8080/#/c/9749/3/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/9749/3/be/src/util/runtime-profile.cc@207 PS3, Line 207: Update > Could they also come from UpdateAverage()? Good point http://gerrit.cloudera.org:8080/#/c/9749/3/be/src/util/runtime-profile.cc@345 PS3, Line 345: // if children are added after the first Update() call (IMPALA-6694). > I think a very simple example would help to understand this, something like Thanks, I stole that text. http://gerrit.cloudera.org:8080/#/c/9749/3/be/src/util/runtime-profile.cc@358 PS3, Line 358: insert_pos != children_.end() > I can't think of a case where this happens without found_child == true. If It's possible that the children got reordered in the original profile between updates, then the child we're search for could be before the previous insert position. I don't think it necessarily happens in practice, but the SortChildren() method does allow you to arbitrarily reorder children at any point. I extended the test case to exercise this. Without this check it it runs off the end of the vector. -- To view, visit http://gerrit.cloudera.org:8080/9749 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I230f0673edf20a846fdb13191b7a292d329c1bb8 Gerrit-Change-Number: 9749 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 28 Mar 2018 04:31:48 +0000 Gerrit-HasComments: Yes
