Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4014: Introduce query-wide execution state.
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4418/5/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:

PS5, Line 72: qs->refcnt_.Add(1);
> Another example: an ExecPlanFragment RPC can arrive late after some instanc
what you're describing is a complete corner case. i don't see a need to 
optimize for that.


Line 141: 
> With the current state of things, after all fragment instances finish execu
i do not want to introduce another counter to keep track of fragment instances, 
the whole point of this is to keep the qs around until all references to it are 
gone (and not just those needed for instance execution - i'd prefer to have the 
final status report rpc be sent when the qs gets gc'ed).

those late-arriving rpcs you're describing sound like another corner case, and 
i don't see how they would end up consuming a lot of resources. i would be 
worried about them if they increased the map lock contention by a sizable 
amount, say 30 or 50 or 100%, but i just don't see how that's possible.

aside from that, it's very easy to reduce the lock contention by an arbitrary 
amount (by partitioning the key space and having a separate map plus lock per 
partition). i just don't think we need that quite yet.


http://gerrit.cloudera.org:8080/#/c/4418/5/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

PS5, Line 351: TUniqueId no_instance_id_;
> Sorry I meant that it doesn't seem to be initialized. Where is it initializ
the default c'tor initializes it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I962ae6b7cb7dc0d07fbb8f70317aeb01d88d400b
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker <mar...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to