[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. IMPALA-5200: Count child time for parent's total time One problem with the total time counter on runtime profiles is that a parent's time may not be updated if execution is stuck in a child node. The child can accumulate time while the parent is stuck at zero. This leads to incorrect or misleading calculations of total time or non-child time for the parent node during execution. This makes a modest change in calculation for total time for parent nodes. It takes advantage of the fact that the parent should count all of the time from all of its children as total time for itself. Specifically, if a parent has accumulated X in its total timer and its children have accumulated Y summed across all of their timers, then a parent's total time should be at least max(X, Y). There is no way to know the appropriate overlap between X and Y, so this uses a conservative calculation assuming complete overlap. This prevents a parent node from reporting itself as 100% non-child time when it is actually stuck executing child code. However, it does not help if a child node is stuck and is not reporting its own time. Testing: - Added test case to runtime-profile-test - Core tests pass Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Reviewed-on: http://gerrit.cloudera.org:8080/11791 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h 3 files changed, 125 insertions(+), 17 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Dec 2018 02:11:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3583/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 18 Dec 2018 21:46:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 4: Code-Review+2 Can't reproduce the UBSAN failure. Rebased, carrying +2 -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 18 Dec 2018 18:31:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3551/ -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 12 Dec 2018 00:10:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1573/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 11 Dec 2018 20:50:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3551/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 11 Dec 2018 20:15:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 3: Code-Review+2 Carry +2 -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 11 Dec 2018 20:15:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Hello Philip Zeyliger, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11791 to look at the new patch set (#3). Change subject: IMPALA-5200: Count child time for parent's total time .. IMPALA-5200: Count child time for parent's total time One problem with the total time counter on runtime profiles is that a parent's time may not be updated if execution is stuck in a child node. The child can accumulate time while the parent is stuck at zero. This leads to incorrect or misleading calculations of total time or non-child time for the parent node during execution. This makes a modest change in calculation for total time for parent nodes. It takes advantage of the fact that the parent should count all of the time from all of its children as total time for itself. Specifically, if a parent has accumulated X in its total timer and its children have accumulated Y summed across all of their timers, then a parent's total time should be at least max(X, Y). There is no way to know the appropriate overlap between X and Y, so this uses a conservative calculation assuming complete overlap. This prevents a parent node from reporting itself as 100% non-child time when it is actually stuck executing child code. However, it does not help if a child node is stuck and is not reporting its own time. Testing: - Added test case to runtime-profile-test - Core tests pass Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e --- M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h 3 files changed, 125 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/11791/3 -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11791/2/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/11791/2/be/src/util/runtime-profile.cc@427 PS2, Line 427: total_time_ns_ = max(children_total_time, total_time_counter()->value()); > Might be worth mentioning this JIRA in a comment here to justify taking the Added a comment with the JIRA and a brief explanation. -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 11 Dec 2018 20:14:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 10 Dec 2018 23:03:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 2: Code-Review+1 (1 comment) LGTM aside from one minor comment. Not sure if Phil plans to take a look. http://gerrit.cloudera.org:8080/#/c/11791/2/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/11791/2/be/src/util/runtime-profile.cc@427 PS2, Line 427: total_time_ns_ = max(children_total_time, total_time_counter()->value()); Might be worth mentioning this JIRA in a comment here to justify taking the max() -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 06 Dec 2018 18:21:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1463/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 29 Nov 2018 01:31:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Hello Philip Zeyliger, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11791 to look at the new patch set (#2). Change subject: IMPALA-5200: Count child time for parent's total time .. IMPALA-5200: Count child time for parent's total time One problem with the total time counter on runtime profiles is that a parent's time may not be updated if execution is stuck in a child node. The child can accumulate time while the parent is stuck at zero. This leads to incorrect or misleading calculations of total time or non-child time for the parent node during execution. This makes a modest change in calculation for total time for parent nodes. It takes advantage of the fact that the parent should count all of the time from all of its children as total time for itself. Specifically, if a parent has accumulated X in its total timer and its children have accumulated Y summed across all of their timers, then a parent's total time should be at least max(X, Y). There is no way to know the appropriate overlap between X and Y, so this uses a conservative calculation assuming complete overlap. This prevents a parent node from reporting itself as 100% non-child time when it is actually stuck executing child code. However, it does not help if a child node is stuck and is not reporting its own time. Testing: - Added test case to runtime-profile-test - Core tests pass Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e --- M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h 3 files changed, 122 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/11791/2 -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger
