impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4014: Introduce query-wide execution state.
Date Wed, 30 Nov 2016 23:05:58 GMT
Marcel Kornacker has posted comments on this change.

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

Patch Set 5:

File be/src/runtime/

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.
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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I962ae6b7cb7dc0d07fbb8f70317aeb01d88d400b
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-HasComments: Yes

View raw message