Hello Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16088
to look at the new patch set (#2).
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
---
M be/src/exec/buffered-plan-root-sink.cc
1 file changed, 11 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/16088/2
--
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: newpatchset
Gerrit-Change-Id: Id6f3fbc05420ca95313fa79ea106547feb92b16b
Gerrit-Change-Number: 16088
Gerrit-PatchSet: 2
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]>
