Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17195
Change subject: IMPALA-10584: Allow UnpinStream to fail if reservation is not enough ...................................................................... IMPALA-10584: Allow UnpinStream to fail if reservation is not enough TestScratchLimit::test_with_unlimited_scratch_limit has been intermittently crashing in ubuntu-16.04-dockerised-tests environment after result spooling is enabled by default in IMPALA-9856. DCHECK violation occurs in ReservationTracker::CheckConsistency() due to BufferedTupleStream wrongly tries to reclaim memory reservation while unpinning the stream. For this bug to surface, all of the following needs to happen: - Stream is in pinned mode. - There are only 2 pages in the stream: 1 read and 1 write. - Stream can not increase reservation anymore either due to memory pressure or low buffer/memory limit. - The stream read page has been fully read and is attached to output RowBatch. But the output RowBatch has not cleaned up yet. - BufferedTupleStream::UnpinStream is invoked. The bug happens because UnpinStream proceeds to NextReadPage where the read page buffer was mistakenly assumed as released. default_page_len_ bytes were added into write_page_reservation_ and subsequently violates the total memory reservation. This patch fixes the bug by disallowing UnpinStream to advance the read iterator if the read page is attached to output RowBatch and there is no more unused reservation. If UnpinStream in turns unable to unpin any page to reclaim memory reservation, TErrorCode::NO_PAGE_UNPINNED will be returned and the stream will stay in pinned mode. In the context of BufferedPlanRootSink, if this error occurs in Send(), the writer will release the lock to give a chance for the reader to read some more rows and clean up the output RowBatch (current_batch_) before retying to add more rows again. Testing: - Add BE test UnpinReadPageMinReservation. - Reenable result spooling in TestScratchLimit that was disabled in IMPALA-10559. - Pass core tests. Change-Id: I16137b6e423f190f60c3115a06ccd0f77e9f585a --- M be/src/exec/buffered-plan-root-sink.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/spillable-row-batch-queue.cc M be/src/runtime/spillable-row-batch-queue.h M common/thrift/generate_error_codes.py M tests/query_test/test_scratch_limit.py 8 files changed, 200 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/17195/1 -- To view, visit http://gerrit.cloudera.org:8080/17195 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I16137b6e423f190f60c3115a06ccd0f77e9f585a Gerrit-Change-Number: 17195 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com>
