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