You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Marshall (Code Review)" <ge...@cloudera.org> on 2019/01/29 22:45:56 UTC
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12297
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
IMPALA-8138: Reintroduce rpc debugging options
In the past, we had fault injection options for backend rpcs
implemented in ImpalaBackendClient. With the move the krpc, we lost
some of those options.
This patch reintroduces equivalent options, except that they now
utilize the existing DebugAction framework, which is more flexible and
powerful than the FAULT_INJECTION framework used previously.
Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
A be/src/runtime/krpc-backend-client.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/service/control-service.cc
M be/src/service/control-service.h
M tests/query_test/test_cancellation.py
8 files changed, 72 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/12297/1
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Andrew Sherman, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12297
to look at the new patch set (#4).
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
IMPALA-8138: Reintroduce rpc debugging options
In the past, we had fault injection options for backend rpcs
implemented in ImpalaBackendClient. With the move the krpc, we lost
some of those options.
This patch reintroduces equivalent options, except that they now
utilize the existing DebugAction framework, which is more flexible and
powerful than the FAULT_INJECTION framework used previously.
Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
---
A be/src/rpc/impala-control-service-proxy.h
A be/src/rpc/impala-data-stream-service-proxy.h
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/rpc-mgr.inline.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/service/client-request-state.cc
M be/src/service/control-service.cc
M be/src/service/control-service.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M be/src/util/CMakeLists.txt
A be/src/util/krpc-debug-util.cc
A be/src/util/krpc-debug-util.h
M tests/custom_cluster/test_restart_services.py
M tests/query_test/test_cancellation.py
20 files changed, 487 insertions(+), 133 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/12297/4
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Andrew Sherman, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12297
to look at the new patch set (#3).
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
IMPALA-8138: Reintroduce rpc debugging options
In the past, we had fault injection options for backend rpcs
implemented in ImpalaBackendClient. With the move the krpc, we lost
some of those options.
This patch reintroduces equivalent options, except that they now
utilize the existing DebugAction framework, which is more flexible and
powerful than the FAULT_INJECTION framework used previously.
Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
---
M be/src/common/global-flags.cc
A be/src/rpc/impala-control-service-proxy.h
A be/src/rpc/impala-data-stream-service-proxy.h
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/rpc-mgr.inline.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/service/client-request-state.cc
M be/src/service/control-service.cc
M be/src/service/control-service.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M be/src/util/CMakeLists.txt
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
A be/src/util/krpc-debug-util.cc
A be/src/util/krpc-debug-util.h
M tests/custom_cluster/test_restart_services.py
M tests/query_test/test_cancellation.py
23 files changed, 475 insertions(+), 136 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/12297/3
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Marshall has abandoned this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Abandoned
we've decided to go in a different direction
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 1:
(6 comments)
Thanks for working on this patch. My general comment is to consider moving away from the existing "fake" fault injection framework in Thrift and use debug actions to simulate scenarios in which we may actually fail to exercise the entire RPC stack better.
http://gerrit.cloudera.org:8080/#/c/12297/5/be/src/rpc/impala-control-service-proxy.h
File be/src/rpc/impala-control-service-proxy.h:
http://gerrit.cloudera.org:8080/#/c/12297/5/be/src/rpc/impala-control-service-proxy.h@44
PS5, Line 44:
As mentioned elsewhere, this kind of artificial fault injection doesn't seem to be too useful.
http://gerrit.cloudera.org:8080/#/c/12297/3/be/src/rpc/impala-control-service-proxy.h
File be/src/rpc/impala-control-service-proxy.h:
http://gerrit.cloudera.org:8080/#/c/12297/3/be/src/rpc/impala-control-service-proxy.h@43
PS3, Line 43:
Not needed. Same below.
http://gerrit.cloudera.org:8080/#/c/12297/5/be/src/rpc/rpc-mgr.inline.h
File be/src/rpc/rpc-mgr.inline.h:
http://gerrit.cloudera.org:8080/#/c/12297/5/be/src/rpc/rpc-mgr.inline.h@65
PS5, Line 65:
:
:
:
:
:
:
:
:
:
:
:
:
It appears that this send vs recv debug actions were carried over from Thrift implementation.
Retrospectively, the "fault injection" we did with Thrift was quite hacky (I am the culprit here) and it stemmed from the total lack of fault injection testing back then for exercising the error paths in Thrift RPC.
As part of the KRPC development, we invested in proper fault injection testing by truly pausing the Impala and artificially creates various failure scenario. This allows a more extensive exercise across the entire RPC stack instead of just exercising the RPC handlers at the client and server sides.
With the fault injection framework, it seems to be not too useful to continue with this path of artificial fault injection via debug action we used to do with Thrift.
Instead, we may want to rethink the fault injection testing with KRPC. In particular, it may exercise the code better by doing some of the followings:
- use debug actions to inject random delays in the RPC handlers. This is particularly useful for RPCs with timeout
- use debug actions to randomly reject some of the incoming RPCs in ImpalaServicePool
- use debug actions to respond with error status in the RPC handlers. The errors will be specific to each RPC handler (e.g. deserialization error of Thrift profiles, deserialization errors of RowBatch)
- debug action to force some incoming RPCs to use deferred queue in KrpcDataStreamRecvr
- (experimental / dangerous) "randomly" corrupt the incoming RPC payloads in ImpalaServicePool.
- inject delay in RPC callback in the client side to simulate an overloaded client
The above are some examples I can think of right now.
For other failures, we may need to rely on the fault injection framework:
- use iptables to drop all incoming packets to the RPC port from a particular host. This simulates a host which was powered off or network partitions
- Restart remote Impalad will trigger the behavior of broken connections (by sending a RST packet)
- Send SIGSTOP to remote Impalad (which we already do in the fault injection framework) to simulate non-responsive Impalad
- other ideas...
In general, my suggestion is to use debug actions to simulate failure which can actually happen instead of using this artificial fault injection which seems a bit meaningless at this point.
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/query-state.h@314
PS1, Line 314: proxy_
> I assume that if I change the class name back to something ending in "Proxy
Yup.
http://gerrit.cloudera.org:8080/#/c/12297/5/be/src/service/control-service.cc
File be/src/service/control-service.cc:
http://gerrit.cloudera.org:8080/#/c/12297/5/be/src/service/control-service.cc@171
PS5, Line 171: if (qs.get() == nullptr) {
: Status status(ErrorMsg(TErrorCode::INTERNAL_ERROR,
Should this be converted to debug action ?
http://gerrit.cloudera.org:8080/#/c/12297/5/be/src/service/control-service.cc@186
PS5, Line 186:
:
Should this be converted to debug action ?
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Apr 2019 17:47:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 1:
(1 comment)
This all looks good but I have questions...
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h
File be/src/runtime/krpc-backend-client.h:
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h@28
PS1, Line 28: class KrpcBackendClient : public ControlServiceProxy
> Hmm...what about DataStreamService ? This seems to be a client for ControlS
Should this class show that it is a decorated form of ControlServiceProxy, perhaps by having ControlServiceProxy as part of it's name? ControlServiceProxyInjected? ControlServiceProxyDecorated?
I think this change introduces a convention, that we expect ControlServiceProxy methods to be overridden in KrpcBackendClient? If that's so then a comment somewhere should say that. Is it in fact now a mistake to use ControlServiceProxy?
It seems like KrpcBackendClient is mostly boilerplate. Did you consider extending protoc-gen-krpc.cc to either generate the decorated methods in ControlServiceProxy, or to generate KrpcBackendClient?
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Jan 2019 20:24:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2758/ : 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/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Apr 2019 20:14:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 3:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/2582/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Mar 2019 00:14:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 2:
(3 comments)
This is not really ready for full review until https://gerrit.cloudera.org/#/c/12672/ is submitted and it is rebased on top, I just updated it so that reviewers of that patch can see what I've done for the async case.
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h
File be/src/runtime/krpc-backend-client.h:
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h@27
PS1, Line 27:
> Please add a class level comment.
Done
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h@28
PS1, Line 28:
> Should this class show that it is a decorated form of
> ControlServiceProxy, perhaps by having ControlServiceProxy as part
> of it's name? ControlServiceProxyInjected? ControlServiceProxyDecorated?
Good idea. "Injected" and "Decorated" seem awkward to me, is it clear enough if I go with ImpalaControlServiceProxy?
>
> I think this change introduces a convention, that we expect
> ControlServiceProxy methods to be overridden in KrpcBackendClient?
> If that's so then a comment somewhere should say that. Is it in
> fact now a mistake to use ControlServiceProxy?
Addressed in the class comment.
>
> It seems like KrpcBackendClient is mostly boilerplate. Did you
> consider extending protoc-gen-krpc.cc to either generate the
> decorated methods in ControlServiceProxy, or to generate
> KrpcBackendClient?
As discussed, while this is an interesting idea, the Kudu people don't have much interest in directly adding such a feature to krpc, and for now at least the benefit of automating this boiler-plate is probably not worth the work.
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/query-state.h@314
PS1, Line 314: eProxy
> Will client_ be a more appropriate name ? Same for other places.
I assume that if I change the class name back to something ending in "Proxy" that you're fine leaving this as is?
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Mar 2019 19:33:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1922/ : 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/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Jan 2019 01:23:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h
File be/src/runtime/krpc-backend-client.h:
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h@27
PS1, Line 27:
Please add a class level comment.
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h@28
PS1, Line 28: class KrpcBackendClient : public ControlServiceProxy
Hmm...what about DataStreamService ? This seems to be a client for ControlService's RPCs only ?
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/query-state.h@314
PS1, Line 314: proxy_
Will client_ be a more appropriate name ? Same for other places.
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Jan 2019 00:55:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 4:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/2743/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 23:39:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12297/4/tests/custom_cluster/test_restart_services.py
File tests/custom_cluster/test_restart_services.py:
http://gerrit.cloudera.org:8080/#/c/12297/4/tests/custom_cluster/test_restart_services.py@337
PS4, Line 337:
flake8: E203 whitespace before ':'
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 23:13:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 1:
Any update ?
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Feb 2019 23:27:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 3:
Rebased on top of the latest version of https://gerrit.cloudera.org/#/c/12672/, still not really ready for full review until that patch goes in
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Mar 2019 23:34:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 1:
(1 comment)
I will try to get https://gerrit.cloudera.org/#/c/12672/ into submittable shape,
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h
File be/src/runtime/krpc-backend-client.h:
http://gerrit.cloudera.org:8080/#/c/12297/1/be/src/runtime/krpc-backend-client.h@28
PS1, Line 28: class KrpcBackendClient : public ControlServiceProxy
> > Should this class show that it is a decorated form of
Thanks this all seems fine.
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Mar 2019 23:51:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Andrew Sherman, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12297
to look at the new patch set (#5).
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
IMPALA-8138: Reintroduce rpc debugging options
In the past, we had fault injection options for backend rpcs
implemented in ImpalaBackendClient. With the move the krpc, we lost
some of those options.
This patch reintroduces equivalent options, except that they now
utilize the existing DebugAction framework, which is more flexible and
powerful than the FAULT_INJECTION framework used previously.
Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
---
A be/src/rpc/impala-control-service-proxy.h
A be/src/rpc/impala-data-stream-service-proxy.h
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/rpc-mgr.inline.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/service/client-request-state.cc
M be/src/service/control-service.cc
M be/src/service/control-service.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M be/src/util/CMakeLists.txt
A be/src/util/krpc-debug-util.cc
A be/src/util/krpc-debug-util.h
M tests/custom_cluster/test_restart_services.py
M tests/query_test/test_cancellation.py
20 files changed, 489 insertions(+), 133 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/12297/5
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Andrew Sherman, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12297
to look at the new patch set (#2).
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
IMPALA-8138: Reintroduce rpc debugging options
In the past, we had fault injection options for backend rpcs
implemented in ImpalaBackendClient. With the move the krpc, we lost
some of those options.
This patch reintroduces equivalent options, except that they now
utilize the existing DebugAction framework, which is more flexible and
powerful than the FAULT_INJECTION framework used previously.
Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
---
A be/src/rpc/impala-control-service-proxy.h
A be/src/rpc/impala-data-stream-service-proxy.h
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/service/client-request-state.cc
M be/src/service/control-service.cc
M be/src/service/control-service.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M be/src/util/CMakeLists.txt
A be/src/util/krpc-debug-util.cc
A be/src/util/krpc-debug-util.h
M tests/custom_cluster/test_restart_services.py
M tests/query_test/test_cancellation.py
19 files changed, 424 insertions(+), 108 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/12297/2
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12297 )
Change subject: IMPALA-8138: Reintroduce rpc debugging options
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/2388/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/12297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2046cb9dadf846ea90c04e95781b2bbde3325941
Gerrit-Change-Number: 12297
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Mar 2019 20:11:51 +0000
Gerrit-HasComments: No