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/13 23:13:28 UTC
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Hello Vuk Ercegovac,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/10712
to review the following change.
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
IMPALA-7140 (part 2). Create skeleton for LocalFsTable
This adds a skeleton implementation of FeFsTable for the LocalCatalog.
Most of the implementation in this patch is stubbed out -- only the bare
bones necessary to instantiate the subclass when a table is loaded and
the simplest table-level methods.
Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
---
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.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/LocalCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalogException.java
A fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
10 files changed, 263 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/10712/1
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10712 )
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2703/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
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, 19 Jun 2018 03:20:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10712 )
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java:
http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@82
PS2, Line 82: Hdfs
> sorry about not catching this before, but pls add a todo (assign it to me)
Added a top-level comment on this class with a TODO for you - there are several methods needing rename so figured one TODO was better than one per method.
http://gerrit.cloudera.org:8080/#/c/10712/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/10712/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@164
PS2, Line 164: public Map<Long, List<FileDescriptor>> getFilesSample(Collection<? extends FeFsPartition> inputParts,
> nit: too long line
Done
http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
File fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java:
http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@95
PS2, Line 95: "__HIVE_DEFAULT_PARTITION__"
> make this a public member so the test can use it as well.
Done
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Jun 2018 17:19:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10712 )
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
Patch Set 5: Code-Review+2
Forwarding +2
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
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: Wed, 20 Jun 2018 05:07:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10712 )
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
Patch Set 2: Code-Review+2
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java:
http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@82
PS2, Line 82: Hdfs
sorry about not catching this before, but pls add a todo (assign it to me) to change the "Hdfs" to "Fs" for consistent naming with this interface. looking at all the places where this is used, the misalignment stands out.
http://gerrit.cloudera.org:8080/#/c/10712/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/10712/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@164
PS2, Line 164: public Map<Long, List<FileDescriptor>> getFilesSample(Collection<? extends FeFsPartition> inputParts,
nit: too long line
http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
File fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java:
http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@95
PS2, Line 95: "__HIVE_DEFAULT_PARTITION__"
make this a public member so the test can use it as well.
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Jun 2018 21:21:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10712 )
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2713/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
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: Wed, 20 Jun 2018 00:48:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10712 )
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
Patch Set 4: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2703/
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
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, 19 Jun 2018 06:47:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Vuk Ercegovac,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10712
to look at the new patch set (#3).
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
IMPALA-7140 (part 2). Create skeleton for LocalFsTable
This adds a skeleton implementation of FeFsTable for the LocalCatalog.
Most of the implementation in this patch is stubbed out -- only the bare
bones necessary to instantiate the subclass when a table is loaded and
the simplest table-level methods.
Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
---
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.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/LocalCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalogException.java
A fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
10 files changed, 278 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/10712/3
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10712 )
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
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: Wed, 20 Jun 2018 04:10:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7140 (part 2). Create skeleton for LocalFsTable
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10712 )
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable
......................................................................
IMPALA-7140 (part 2). Create skeleton for LocalFsTable
This adds a skeleton implementation of FeFsTable for the LocalCatalog.
Most of the implementation in this patch is stubbed out -- only the bare
bones necessary to instantiate the subclass when a table is loaded and
the simplest table-level methods.
Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Reviewed-on: http://gerrit.cloudera.org:8080/10712
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.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/LocalCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalogException.java
A fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
10 files changed, 278 insertions(+), 24 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Todd Lipcon: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/10712
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8
Gerrit-Change-Number: 10712
Gerrit-PatchSet: 6
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>