Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16088 )
Change subject: IMPALA-9842: Fix hang when cancelling BufferedPlanRootSink ...................................................................... IMPALA-9842: Fix hang when cancelling BufferedPlanRootSink In BufferedPlanRootSink::FlushFinal(), if Cancel() runs before FlushFinal() waits on the consumer_eos_ condition variable, the thread in FlushFinal() will wait forever. This is because it is not checking for cancellation or synchronizing with the Cancel() thread. Specifically: Thread A: Calls BufferedPlanRootSink::Cancel(), signalling any thread currently waiting on the consumer_eos_ condition variable. Thread B: Enters FlushFinal(). Never tests RuntimeState::is_cancelled() and calls Wait() on the consumer_eos_ condition variable. This waits forever. This changes BufferedPlanRootSink::Cancel() to get the lock_ before signalling the consumer_eos_ condition variable. It also changes FlushFinal() to call Wait() in a loop. It breaks out of the loop if it is cancelled or the batch_queue_ is empty. There are two cases: 1. FlushFinal() gets the lock_ first and only releases it when waiting on the consumer_eos_ condition variable. It will get signalled by Cancel(). 2. Cancel() gets the lock_ first and FlushFinal() will not wait, because is_cancelled() is true. Testing: - Run core tests Change-Id: Id6f3fbc05420ca95313fa79ea106547feb92b16b Reviewed-on: http://gerrit.cloudera.org:8080/16088 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- M be/src/exec/buffered-plan-root-sink.cc 1 file changed, 11 insertions(+), 2 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/16088 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id6f3fbc05420ca95313fa79ea106547feb92b16b Gerrit-Change-Number: 16088 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]>
