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 Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2019/11/21 23:44:11 UTC

[Impala-ASF-CR] IMPALA-9181: Serialize TQueryCtx once per query

Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14777


Change subject: IMPALA-9181: Serialize TQueryCtx once per query
......................................................................

IMPALA-9181: Serialize TQueryCtx once per query

When issuing Exec() rpcs to backends, we currently serialize the
TQueryCtx once per backend. This is inefficient as the TQueryCtx is
the same for all backends and really only needs to be serialized once.

This patch serializes the TQueryCtx in the coordinator and then passes
it to each BackendState when calling Exec().

Testing:
- Passed full run of existing tests.
- Single node perf run showed no significant change.

Change-Id: I6a4dd302fd5602ec2775492a041ddd51e7d7a6c6
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-exec-mgr.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/test-env.cc
M be/src/service/control-service.cc
M be/src/service/control-service.h
M common/protobuf/control_service.proto
M common/thrift/ImpalaInternalService.thrift
13 files changed, 133 insertions(+), 108 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/14777/1
-- 
To view, visit http://gerrit.cloudera.org:8080/14777
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6a4dd302fd5602ec2775492a041ddd51e7d7a6c6
Gerrit-Change-Number: 14777
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-9181: Serialize TQueryCtx once per query

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.

Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14777 )

Change subject: IMPALA-9181: Serialize TQueryCtx once per query
......................................................................


Patch Set 3: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14777/3/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/14777/3/be/src/runtime/coordinator-backend-state.cc@215
PS3, Line 215:   unique_ptr<kudu::faststring> sidecar_buf = make_unique<kudu::faststring>();
             :   sidecar_buf->assign_copy(serialized_buf, serialized_len);
             :   unique_ptr<RpcSidecar> rpc_sidecar = RpcSidecar::FromFaststring(move(sidecar_buf));
Not this change but this can in theory be converted to sidecar using Slice too. That said, we may better hold off from doing so now until we convert ExecQueryFInstance() RPC to asynchronous as the memory management is slightly different in that case.

May be a TODO for now is sufficient.


http://gerrit.cloudera.org:8080/#/c/14777/3/be/src/service/control-service.cc
File be/src/service/control-service.cc:

http://gerrit.cloudera.org:8080/#/c/14777/3/be/src/service/control-service.cc@120
PS3, Line 120: Status GetSidecar(int sidecar_idx, RpcContext* rpc_context, T* thrift_obj) {
static



-- 
To view, visit http://gerrit.cloudera.org:8080/14777
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a4dd302fd5602ec2775492a041ddd51e7d7a6c6
Gerrit-Change-Number: 14777
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 00:09:52 +0000
Gerrit-HasComments: Yes