You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/06/18 18:51:02 UTC
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Hello Tianyi Wang, Vuk Ercegovac,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/10749
to review the following change.
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
IMPALA-7140 (part 5): support fetching file info for FS tables
This adds support for fetching file information and creating file
descriptors.
With this patch, I'm able to connect and run queries. Most planner tests
still fail because of missing column stats resulting in different join
orders compared to the existing implementation.
Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
---
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
8 files changed, 185 insertions(+), 49 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/10749/1
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@132
PS2, Line 132: FileStatus fileStatus) {
> nit: pull up to prev. line. thx for cleaning that signature.
Done
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@167
PS2, Line 167: must path
> Path must be
oops, I typed with a lisp there :)
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@172
PS2, Line 172: b.add(it.next());
> nit: one line
Done
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java:
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@81
PS2, Line 81: hostIndex_
> pls add a comment and todo to maintain it (afaict, it remains empty).
I think this gets modified as a side effect of loadFileDescriptors, which calls HdfsTable.createFileDescriptors, which updates the host index to include all referenced hosts. I'll add some comment.
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java@73
PS2, Line 73: Load files and locations
> This is more like inode info (file metadata), which is clearer than "Load f
Done
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Jun 2018 17:10:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Sat, 23 Jun 2018 03:21:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
Patch Set 2: Code-Review+2
(6 comments)
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@132
PS2, Line 132: FileStatus fileStatus) {
nit: pull up to prev. line. thx for cleaning that signature.
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@167
PS2, Line 167: must path
Path must be
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@172
PS2, Line 172: b.add(it.next());
nit: one line
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java:
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java@242
PS2, Line 242: new FakeRemoteIterator<>(stats)
nice.
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java:
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@81
PS2, Line 81: hostIndex_
pls add a comment and todo to maintain it (afaict, it remains empty).
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/10749/2/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java@73
PS2, Line 73: Load files and locations
This is more like inode info (file metadata), which is clearer than "Load files".
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Jun 2018 16:53:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
IMPALA-7140 (part 5): support fetching file info for FS tables
This adds support for fetching file information and creating file
descriptors.
With this patch, I'm able to connect and run queries. Most planner tests
still fail because of missing column stats resulting in different join
orders compared to the existing implementation.
Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Reviewed-on: http://gerrit.cloudera.org:8080/10749
Reviewed-by: Vuk Ercegovac <ve...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
8 files changed, 188 insertions(+), 50 deletions(-)
Approvals:
Vuk Ercegovac: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Jun 2018 02:03:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Tianyi Wang, Vuk Ercegovac,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10749
to look at the new patch set (#3).
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
IMPALA-7140 (part 5): support fetching file info for FS tables
This adds support for fetching file information and creating file
descriptors.
With this patch, I'm able to connect and run queries. Most planner tests
still fail because of missing column stats resulting in different join
orders compared to the existing implementation.
Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
---
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
8 files changed, 188 insertions(+), 50 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/10749/3
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2734/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Sat, 23 Jun 2018 00:07:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Sat, 23 Jun 2018 00:03:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2739/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Jun 2018 22:38:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Jun 2018 01:40:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 5): support fetching file info for FS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10749 )
Change subject: IMPALA-7140 (part 5): support fetching file info for FS tables
......................................................................
Patch Set 5:
This change did not cherrypick successfully into branch 2.x. To resolve this, please do the cherry-pick manually and submit it to Gerrit at refs/for/2.x or add an exception to the branch 2.x copy of bin/ignored_commits.json. Thanks, your friendly bot at https://jenkins.impala.io/job/cherrypick-2.x-and-test/651/ .
--
To view, visit http://gerrit.cloudera.org:8080/10749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d67ab754872fad094c7dacdd2e1182de1bf3e8
Gerrit-Change-Number: 10749
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Sat, 23 Feb 2019 20:18:10 +0000
Gerrit-HasComments: No