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/05/03 15:27:57 UTC
[Impala-ASF-CR](2.x) IMPALA-6587: free buffers before ScanRange::Cancel() returns
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10271
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 to move more logic into ScanRange and
to avoid holding RequestContext::lock_ for longer than necessary.
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
Reviewed-on: http://gerrit.cloudera.org:8080/9680
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Tim Armstrong <ta...@cloudera.com>
---
M be/src/common/global-flags.cc
M be/src/runtime/io/disk-io-mgr-internal.h
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/disk-io-mgr.h
M be/src/runtime/io/request-context.cc
M be/src/runtime/io/request-context.h
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
11 files changed, 288 insertions(+), 174 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/10271/2
--
To view, visit http://gerrit.cloudera.org:8080/10271
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I87182b6bd51b5fb0b923e7e4c8d08a44e7617db2
Gerrit-Change-Number: 10271
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR](2.x) IMPALA-6587: free buffers before ScanRange::Cancel() returns
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10271 )
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 to move more logic into ScanRange and
to avoid holding RequestContext::lock_ for longer than necessary.
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
Reviewed-on: http://gerrit.cloudera.org:8080/9680
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Tim Armstrong <ta...@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/10271
---
M be/src/common/global-flags.cc
M be/src/runtime/io/disk-io-mgr-internal.h
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/disk-io-mgr.h
M be/src/runtime/io/request-context.cc
M be/src/runtime/io/request-context.h
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
11 files changed, 288 insertions(+), 174 deletions(-)
Approvals:
Tim Armstrong: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/10271
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I87182b6bd51b5fb0b923e7e4c8d08a44e7617db2
Gerrit-Change-Number: 10271
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>