Henry Robinson has posted comments on this change. Change subject: IMPALA-4890/5143: Coordinator race involving TearDown() ......................................................................
Patch Set 1: (1 comment) Patch looks pretty reasonable, will finish shortly. http://gerrit.cloudera.org:8080/#/c/6897/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: Line 895 > i think the waitforbackendcompletion call should be removed altogether, but I believe that PlanRootSink() may set eos in GetNext() at the same time as it returns the final rows, if the sender calls Send() immediately followed by FlushFinal() before the consumer gets woken up in GetNext(). It looks to me like the callers expect that this might be possible. But since the PRS puts those results into a QueryResultSet owned by the ClientRequestState, I think it's safe to tear down the PRS once it sets eos. As regards post-query finalization - what about computing the final profiles based on the last report sent by each fragment? Do we need to wait before calling Coordinator::ComputeQuerySummary()? I think we should aim to be rid of WaitForBackendCompletion() here, but maybe it still needs to exist in some form on a tear-down path which is not on the user's critical path. -- To view, visit http://gerrit.cloudera.org:8080/6897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I457a6424a0255c137336c4bc01a6e7ed830d18c7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
