impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4890/5143: Coordinator race involving TearDown()
Date Wed, 24 May 2017 00:11:20 GMT
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.
File be/src/runtime/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I457a6424a0255c137336c4bc01a6e7ed830d18c7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-HasComments: Yes

View raw message