You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2017/10/13 18:56:52 UTC

[Impala-ASF-CR] IMPALA-2550: Switch to per-query exec rpc

Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/6535 )

Change subject: IMPALA-2550: Switch to per-query exec rpc
......................................................................


Patch Set 15:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/6535/15/be/src/runtime/coordinator-backend-state.cc@363
PS15, Line 363:   local_params.__set_bloom_filter(rpc_params->bloom_filter);
> Cleanup is definitely welcome.
Came across this while looking at Tianyi's patch. It looks like this patch actually removed the parallelisation. Before this patch the filter distribution was offloaded to exec_env_->rpc_pool(). After this change the filter distribution is done serially on the RPC thread that processes the UpdateFilter() message. I'm not sure what the consequences of this are, but we should keep an eye on this to see if it caused any regressions in Impala 2.9.

Unfortunately it looks like the change was snuck into the larger changes in this patch - it looks like it wasn't mentioned in the commit message or noticed by reviewers. Before the code was:

    exec_env_->rpc_pool()->Offer(bind<void>(DistributeFilters, rpc_params,
        fragment_inst->impalad_address(), fragment_inst->fragment_instance_id()));



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20769e420711737b6b385c744cef4851cee3facd
Gerrit-Change-Number: 6535
Gerrit-PatchSet: 15
Gerrit-Owner: Marcel Kornacker <ma...@gmail.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker <ma...@gmail.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Oct 2017 18:56:52 +0000
Gerrit-HasComments: Yes