Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17819 )
Change subject: IMPALA-10883: Do not override existing counters with empty profile ...................................................................... Patch Set 1: (2 comments) Sorry to be late here due to my vacation. I'll try to unblock this in the following days. The fix makes sense to me! http://gerrit.cloudera.org:8080/#/c/17819/1/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/17819/1/be/src/util/runtime-profile-test.cc@1489 PS1, Line 1489: (i - offset + 1) * 11 This should be 10 when i == offset due to line 1566. Or should we set the counter value to 11 at line 1566? http://gerrit.cloudera.org:8080/#/c/17819/1/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/17819/1/be/src/util/runtime-profile.cc@627 PS1, Line 627: if (LIKELY(!tcounter.values[i].empty())) { I traced the codes generating 'values' but still not sure whether a valid 'values' can be empty, e.g. at the begining or in some corner cases of ChunkedTimeSeriesCounter. Indeed, what we want to avoid is invalid/empty values overwriting valid values. Can we adjust this to the following? if (UNLIKELY(tcounter.values[i].empty() && !dst.values[idx].empty())) continue; -- To view, visit http://gerrit.cloudera.org:8080/17819 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9bb179bf739ffaa4e5ec8dc911480ac835ae387f Gerrit-Change-Number: 17819 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Tue, 07 Sep 2021 13:52:30 +0000 Gerrit-HasComments: Yes
