You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sahil Takiar (Code Review)" <ge...@cloudera.org> on 2020/10/08 23:25:02 UTC
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Sahil Takiar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16567
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
IMPALA-9485: Enable file handle cache for EC files
This is essentially a revert of IMPALA-8178. HDFS-14308 added
CanUnbuffer support to the EC input stream APIs in the HDFS client lib.
This patch enables file handle caching for EC files.
Testing:
* Ran core tests against an EC build (ERASURE_CODING=true)
Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
---
M be/src/exec/base-sequence-scanner.cc
M be/src/exec/hdfs-orc-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-text-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/parquet-page-index.cc
M be/src/exec/parquet/parquet-page-reader.cc
M be/src/exec/scanner-context.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/request-ranges.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/scheduling/scheduler.cc
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M tests/custom_cluster/test_hdfs_fd_caching.py
19 files changed, 50 insertions(+), 82 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/16567/1
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16567/1/be/src/runtime/io/request-ranges.h
File be/src/runtime/io/request-ranges.h:
http://gerrit.cloudera.org:8080/#/c/16567/1/be/src/runtime/io/request-ranges.h@263
PS1, Line 263: /// 'file'; the mtime must change when the file changes.
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/16567/1/tests/custom_cluster/test_hdfs_fd_caching.py
File tests/custom_cluster/test_hdfs_fd_caching.py:
http://gerrit.cloudera.org:8080/#/c/16567/1/tests/custom_cluster/test_hdfs_fd_caching.py@21
PS1, Line 21:
flake8: W291 trailing whitespace
http://gerrit.cloudera.org:8080/#/c/16567/1/tests/custom_cluster/test_hdfs_fd_caching.py@21
PS1, Line 21: from tests.common.skip import SkipIfLocal
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Oct 2020 23:26:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16567/1/be/src/runtime/io/request-ranges.h
File be/src/runtime/io/request-ranges.h:
http://gerrit.cloudera.org:8080/#/c/16567/1/be/src/runtime/io/request-ranges.h@263
PS1, Line 263: /// 'file'; the mtime must change when the file changes.
> line has trailing whitespace
Done
http://gerrit.cloudera.org:8080/#/c/16567/1/tests/custom_cluster/test_hdfs_fd_caching.py
File tests/custom_cluster/test_hdfs_fd_caching.py:
http://gerrit.cloudera.org:8080/#/c/16567/1/tests/custom_cluster/test_hdfs_fd_caching.py@21
PS1, Line 21:
> flake8: W291 trailing whitespace
Done
http://gerrit.cloudera.org:8080/#/c/16567/1/tests/custom_cluster/test_hdfs_fd_caching.py@21
PS1, Line 21: from tests.common.skip import SkipIfLocal
> line has trailing whitespace
Done
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Oct 2020 23:30:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7391/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Oct 2020 23:52:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
Patch Set 2:
> This makes sense to me.
>
> It is nice to remove all the plumbing for the EC value. The only
> question is whether we will ever want that value for something else
> (stats, etc). I guess we can always revive this plumbing if we need
> it down the line.
Yeah. Had similar thoughts. We can always re-add it later. Removing it has the added benefit of simplifying the code.
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Oct 2020 01:26:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Oct 2020 06:47:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
IMPALA-9485: Enable file handle cache for EC files
This is essentially a revert of IMPALA-8178. HDFS-14308 added
CanUnbuffer support to the EC input stream APIs in the HDFS client lib.
This patch enables file handle caching for EC files.
Testing:
* Ran core tests against an EC build (ERASURE_CODING=true)
Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Reviewed-on: http://gerrit.cloudera.org:8080/16567
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/base-sequence-scanner.cc
M be/src/exec/hdfs-orc-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-text-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/parquet-page-index.cc
M be/src/exec/parquet/parquet-page-reader.cc
M be/src/exec/scanner-context.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/request-ranges.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/scheduling/scheduler.cc
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M tests/custom_cluster/test_hdfs_fd_caching.py
19 files changed, 50 insertions(+), 82 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7390/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Oct 2020 23:44:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6533/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Oct 2020 01:26:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
Patch Set 2: Code-Review+2
This makes sense to me.
It is nice to remove all the plumbing for the EC value. The only question is whether we will ever want that value for something else (stats, etc). I guess we can always revive this plumbing if we need it down the line.
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Oct 2020 01:21:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16567 )
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Oct 2020 01:26:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9485: Enable file handle cache for EC files
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16567
to look at the new patch set (#2).
Change subject: IMPALA-9485: Enable file handle cache for EC files
......................................................................
IMPALA-9485: Enable file handle cache for EC files
This is essentially a revert of IMPALA-8178. HDFS-14308 added
CanUnbuffer support to the EC input stream APIs in the HDFS client lib.
This patch enables file handle caching for EC files.
Testing:
* Ran core tests against an EC build (ERASURE_CODING=true)
Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
---
M be/src/exec/base-sequence-scanner.cc
M be/src/exec/hdfs-orc-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-text-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/parquet-page-index.cc
M be/src/exec/parquet/parquet-page-reader.cc
M be/src/exec/scanner-context.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/request-ranges.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/scheduling/scheduler.cc
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M tests/custom_cluster/test_hdfs_fd_caching.py
19 files changed, 50 insertions(+), 82 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/16567/2
--
To view, visit http://gerrit.cloudera.org:8080/16567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieb455eeed02a229a4559d3972dfdac7df32cdb99
Gerrit-Change-Number: 16567
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>