You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2019/03/21 23:49:42 UTC
[Impala-ASF-CR] Add extra TRACE calls for cancellation code path
Hello Joe McDonnell,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/12832
to review the following change.
Change subject: Add extra TRACE calls for cancellation code path
......................................................................
Add extra TRACE calls for cancellation code path
In an attempt to help diagnose IMPALA-8322, this adds more tracing
information for cancellation RPCs. This should make it easier to see
what happened when slow cancellation RPCs dump their traces to the log.
Change-Id: Ifedd95098a011d7a3bb3ebb54bfb3cbb7e94fab4
---
M be/src/kudu/util/trace.h
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/service/control-service.cc
M be/src/testutil/fault-injection-util.cc
7 files changed, 34 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/12832/1
--
To view, visit http://gerrit.cloudera.org:8080/12832
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifedd95098a011d7a3bb3ebb54bfb3cbb7e94fab4
Gerrit-Change-Number: 12832
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] Add extra TRACE calls for cancellation code path
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12832 )
Change subject: Add extra TRACE calls for cancellation code path
......................................................................
Patch Set 1:
> Patch Set 1:
>
> Thanks for working on this. Taking a look. For my own reference, it looks like this trace gets dumped here: https://github.com/apache/impala/blob/master/be/src/kudu/rpc/rpcz_store.cc#L243
Yep. It also gets sampled in /rpcz on Kudu into a set of buckets. i'm not sure if the Impala /rpcz includes the sampled traces or not.
My guess is that the underlying issue is that something is holding onto one of the various locks for too long, and hopefully this will at least identify which lock is held, even if it doesn't identify the holder.
--
To view, visit http://gerrit.cloudera.org:8080/12832
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifedd95098a011d7a3bb3ebb54bfb3cbb7e94fab4
Gerrit-Change-Number: 12832
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 22 Mar 2019 00:20:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Add extra TRACE calls for cancellation code path
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12832 )
Change subject: Add extra TRACE calls for cancellation code path
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12832/1/be/src/runtime/krpc-data-stream-mgr.cc
File be/src/runtime/krpc-data-stream-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/12832/1/be/src/runtime/krpc-data-stream-mgr.cc@215
PS1, Line 215: TRACE_COUNTER_SCOPE_LATENCY_US("find_receiver_us");
Doing it here instead of FindRecvr() means you can also measure the time spent waiting for the lock. So, does it make sense to also sample similar code path in CloseSender() ?
http://gerrit.cloudera.org:8080/#/c/12832/1/be/src/runtime/krpc-data-stream-mgr.cc@378
PS1, Line 378: {
Do we care about sampling this scope too ?
http://gerrit.cloudera.org:8080/#/c/12832/1/be/src/runtime/krpc-data-stream-mgr.cc@406
PS1, Line 406: {
Same question here.
http://gerrit.cloudera.org:8080/#/c/12832/1/be/src/service/control-service.cc
File be/src/service/control-service.cc:
http://gerrit.cloudera.org:8080/#/c/12832/1/be/src/service/control-service.cc@24
PS1, Line 24: #include "kudu/util/trace.h"
nit: is this new #include needed ?
--
To view, visit http://gerrit.cloudera.org:8080/12832
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifedd95098a011d7a3bb3ebb54bfb3cbb7e94fab4
Gerrit-Change-Number: 12832
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 22 Mar 2019 01:47:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] Add extra TRACE calls for cancellation code path
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12832 )
Change subject: Add extra TRACE calls for cancellation code path
......................................................................
Patch Set 1:
Thanks for working on this. Taking a look. For my own reference, it looks like this trace gets dumped here: https://github.com/apache/impala/blob/master/be/src/kudu/rpc/rpcz_store.cc#L243
--
To view, visit http://gerrit.cloudera.org:8080/12832
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifedd95098a011d7a3bb3ebb54bfb3cbb7e94fab4
Gerrit-Change-Number: 12832
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2019 00:14:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Add extra TRACE calls for cancellation code path
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12832 )
Change subject: Add extra TRACE calls for cancellation code path
......................................................................
Patch Set 1:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/2516/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/12832
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifedd95098a011d7a3bb3ebb54bfb3cbb7e94fab4
Gerrit-Change-Number: 12832
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 22 Mar 2019 00:34:49 +0000
Gerrit-HasComments: No