You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2018/03/15 22:31:01 UTC
[Impala-ASF-CR](impala-4835) IMPALA-6587: free buffers before ScanRange::Cancel() returns
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9674
Change subject: IMPALA-6587: free buffers before ScanRange::Cancel() returns
......................................................................
IMPALA-6587: free buffers before ScanRange::Cancel() returns
ScanRange::Cancel() now waits until an in-flight read finishes so
that the disk I/O buffer being processed by the disk thread is
freed when Cancel() returns.
The fix is to set a 'read_in_flight_' flag on the scan range
while the disk thread is doing the read. Cancel() blocks until
read_in_flight_ == false.
The code is refactored a little bit by moving some logic into
ScanRange, in order to do the change cleanly.
Testing:
Added query test that reproduces the issue.
Added a unit test and a stress option that reproduces the problem in a
targeted way.
Ran disk-io-mgr-stress test for a few hours. Ran it under TSAN and
inspected output to make sure there were no non-benign data races.
Change-Id: I87182b6bd51b5fb0b923e7e4c8d08a44e7617db2
---
M be/src/common/global-flags.cc
M be/src/exec/parquet-column-readers.cc
M be/src/runtime/io/disk-io-mgr-stress.cc
M be/src/runtime/io/disk-io-mgr-test.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/request-context.cc
M be/src/runtime/io/request-ranges.h
M be/src/runtime/io/scan-range.cc
M testdata/workloads/functional-query/queries/QueryTest/scanners.test
9 files changed, 183 insertions(+), 61 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/9674/1
--
To view, visit http://gerrit.cloudera.org:8080/9674
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: impala-4835
Gerrit-MessageType: newchange
Gerrit-Change-Id: I87182b6bd51b5fb0b923e7e4c8d08a44e7617db2
Gerrit-Change-Number: 9674
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR](impala-4835) IMPALA-6587: free buffers before ScanRange::Cancel() returns
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9674 )
Change subject: IMPALA-6587: free buffers before ScanRange::Cancel() returns
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9674/1/be/src/runtime/io/disk-io-mgr-test.cc
File be/src/runtime/io/disk-io-mgr-test.cc:
http://gerrit.cloudera.org:8080/#/c/9674/1/be/src/runtime/io/disk-io-mgr-test.cc@1197
PS1, Line 1197: ASSERT_OK(io_mgr.AllocateBuffersForRange(reader.get(), &read_client, range, MAX_BUFFER_SIZE));
Need to fix long line.
--
To view, visit http://gerrit.cloudera.org:8080/9674
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: impala-4835
Gerrit-MessageType: comment
Gerrit-Change-Id: I87182b6bd51b5fb0b923e7e4c8d08a44e7617db2
Gerrit-Change-Number: 9674
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Mar 2018 22:31:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR](impala-4835) IMPALA-6587: free buffers before ScanRange::Cancel() returns
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has abandoned this change. ( http://gerrit.cloudera.org:8080/9674 )
Change subject: IMPALA-6587: free buffers before ScanRange::Cancel() returns
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/9674
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: impala-4835
Gerrit-MessageType: abandon
Gerrit-Change-Id: I87182b6bd51b5fb0b923e7e4c8d08a44e7617db2
Gerrit-Change-Number: 9674
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>