You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2018/12/19 19:29:47 UTC
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12111
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
IMPALA-7265: Add parameter to cache remote HDFS file handles
Currently, the file handle cache does not apply to remote HDFS
files. This adds a parameter 'cache_remote_file_handles' that
enables the file handle cache for remote HDFS files. It is
currently being tested, so it is set to false by default.
This does not change the behavior for S3, ADLS, or ABFS.
Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
---
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/scan-range.cc
4 files changed, 30 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/12111/1
--
To view, visit http://gerrit.cloudera.org:8080/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12111 )
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Dec 2018 03:10:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12111 )
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
Patch Set 1:
(1 comment)
Thanks!
Do we want to have a custom cluster test that exercises this code path?
http://gerrit.cloudera.org:8080/#/c/12111/1/be/src/runtime/io/scan-range.cc
File be/src/runtime/io/scan-range.cc:
http://gerrit.cloudera.org:8080/#/c/12111/1/be/src/runtime/io/scan-range.cc@202
PS1, Line 202: // 2. The file must not be on S3, ADLS, or ABFS.
Perhaps isntead of checking that it's not S3, ADLS, or ABFS, we check explicitly that it's either Local Hdfs or that it's remote HDFS (if the given option is enabled).
I think this translates to:
is_file_handle_caching_enabled() &&
(expected_local_ || (FLAGS_cache_remote_file_handles && disk_id_ == io_mgr->RemoteDfsDiskId())
--
To view, visit http://gerrit.cloudera.org:8080/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Dec 2018 22:52:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12111 )
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
IMPALA-7265: Add parameter to cache remote HDFS file handles
Currently, the file handle cache does not apply to remote HDFS
files. This adds a parameter 'cache_remote_file_handles' that
enables the file handle cache for remote HDFS files. It is
currently being tested, so it is set to false by default.
This does not change the behavior for S3, ADLS, or ABFS.
Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Reviewed-on: http://gerrit.cloudera.org:8080/12111
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Philip Zeyliger <ph...@cloudera.com>
---
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/scan-range.cc
4 files changed, 28 insertions(+), 10 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Philip Zeyliger: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12111 )
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
Patch Set 1:
(1 comment)
> (1 comment)
>
> Thanks!
>
> Do we want to have a custom cluster test that exercises this code
> path?
The difficulty is that the minicluster does not have any remote reads. HDFS realizes that it is reading from localhost and ends up doing local reads.
http://gerrit.cloudera.org:8080/#/c/12111/1/be/src/runtime/io/scan-range.cc
File be/src/runtime/io/scan-range.cc:
http://gerrit.cloudera.org:8080/#/c/12111/1/be/src/runtime/io/scan-range.cc@202
PS1, Line 202: // 2. The file must not be on S3, ADLS, or ABFS.
> Perhaps isntead of checking that it's not S3, ADLS, or ABFS, we check expli
Makes sense, done.
--
To view, visit http://gerrit.cloudera.org:8080/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Dec 2018 23:03:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12111 )
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1648/ : 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/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Dec 2018 20:06:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12111 )
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1668/ : 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/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Dec 2018 23:40:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12111
to look at the new patch set (#2).
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
IMPALA-7265: Add parameter to cache remote HDFS file handles
Currently, the file handle cache does not apply to remote HDFS
files. This adds a parameter 'cache_remote_file_handles' that
enables the file handle cache for remote HDFS files. It is
currently being tested, so it is set to false by default.
This does not change the behavior for S3, ADLS, or ABFS.
Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
---
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/scan-range.cc
4 files changed, 28 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/12111/2
--
To view, visit http://gerrit.cloudera.org:8080/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12111 )
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
Patch Set 2: Code-Review+2
Thanks.
--
To view, visit http://gerrit.cloudera.org:8080/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Dec 2018 21:18:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12111 )
Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3599/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/12111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9
Gerrit-Change-Number: 12111
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Dec 2018 23:05:41 +0000
Gerrit-HasComments: No