impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-2550: Switch to per-query exec rpc
Date Mon, 25 Sep 2017 22:43:20 GMT
Dan Hecht 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);
> Sorry for commenting on an old CR. I'm reading this part of code and have s
Note that this change only moved the code around (into coordinator.cc), it didn't introduce
this code. You'll have to go further back in time to see the change that introduced that code,
and maybe there is insight there.

That said, I agree it doesn't seem like shared_ptr makes sense here (and usually, we should
avoid use of shared_ptr in impala because it makes reasoning about lifetimes difficult). 

>From the comment in Coordinator::UpdateFilter() it sounds like there was some idea to
parallelize the RPCs, which maybe is where the thought of shared_ptr came in. I'm not sure
if the code ever did that, though. Going back further in git history may answer it.

// Make a 'master' copy that will be shared by all concurrent delivery RPC attempts.



-- 
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 <marcelk@gmail.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker <marcelk@gmail.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <twang@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Sep 2017 22:43:20 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message