You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2021/11/19 14:12:05 UTC
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18041
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
When local catalog mode is used, Impala retrieves the Iceberg
snapshot from CatalogD. The response contains a map of the file
descriptors. The file descriptors contain block location information,
but the hosts are only referred by indexes. In the Coordinator's local
catalog the host indexes might refer to different hosts than in
CatalogD. This might lead to unnecessary remote reads as scan ranges
are scheduled to random hosts.
This patch translates the host index to the coordinators host list, so
block locations remain consisten.
Testing:
* tested manually on a 6-node cluster, and verified that the file
locations are consistent with HDFS
Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
---
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartitionLocationCompressor.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/util/ListMap.java
13 files changed, 74 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/18041/1
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7663/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Nov 2021 21:53:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
When local catalog mode is used, Impala retrieves the Iceberg
snapshot from CatalogD. The response contains a map of the file
descriptors. The file descriptors contain block location information,
but the hosts are only referred to by indexes. In the Coordinator's
local catalog the host indexes might refer to different hosts than in
CatalogD. This might lead to unnecessary remote reads as scan ranges
are scheduled to random hosts.
This patch properly translates the host index to the coordinators host
list, so block locations remain consistent.
Testing:
* tested manually on a 6-node cluster, and verified that the file
locations are consistent with HDFS
* added unit test to LocalCatalogTest
Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Reviewed-on: http://gerrit.cloudera.org:8080/18041
Reviewed-by: Qifan Chen <qc...@cloudera.com>
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.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/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
8 files changed, 123 insertions(+), 14 deletions(-)
Approvals:
Qifan Chen: Looks good to me, but someone else must approve
Csaba Ringhofer: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Nov 2021 13:27:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7657/
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Nov 2021 21:11:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9813/ : 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/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 14:33:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7657/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Nov 2021 14:52:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/18041/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18041/1//COMMIT_MSG@18
PS1, Line 18: block locations remain consisten.
t
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@455
PS1, Line 455: } else {
precondition about hostIndex != null?
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1014
PS1, Line 1014: //return resp.table_info.getIceberg_snapshot();
code in comment
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/util/ListMap.java
File fe/src/main/java/org/apache/impala/util/ListMap.java:
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/util/ListMap.java@56
PS1, Line 56: public int getOrAddIndex(T t) {
can you add a comment about this rename in the comment, or possibly move it to a different patch? I agree with the rename as the old name was misleading, but it adds some extra noise to the commit
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 16:14:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18041
to look at the new patch set (#2).
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
When local catalog mode is used, Impala retrieves the Iceberg
snapshot from CatalogD. The response contains a map of the file
descriptors. The file descriptors contain block location information,
but the hosts are only referred by indexes. In the Coordinator's local
catalog the host indexes might refer to different hosts than in
CatalogD. This might lead to unnecessary remote reads as scan ranges
are scheduled to random hosts.
This patch translates the host index to the coordinators host list, so
block locations remain consistent.
Testing:
* tested manually on a 6-node cluster, and verified that the file
locations are consistent with HDFS
Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
---
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
6 files changed, 64 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/18041/2
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Qifan Chen, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18041
to look at the new patch set (#3).
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
When local catalog mode is used, Impala retrieves the Iceberg
snapshot from CatalogD. The response contains a map of the file
descriptors. The file descriptors contain block location information,
but the hosts are only referred to by indexes. In the Coordinator's
local catalog the host indexes might refer to different hosts than in
CatalogD. This might lead to unnecessary remote reads as scan ranges
are scheduled to random hosts.
This patch properly translates the host index to the coordinators host
list, so block locations remain consistent.
Testing:
* tested manually on a 6-node cluster, and verified that the file
locations are consistent with HDFS
* added unit test to LocalCatalogTest
Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
---
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.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/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
8 files changed, 123 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/18041/3
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3:
(5 comments)
Thanks for the comments!
http://gerrit.cloudera.org:8080/#/c/18041/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18041/2//COMMIT_MSG@12
PS2, Line 12: but the hosts are only referred to by indexes. In the Coordinator's
> nit: referred to
Done
http://gerrit.cloudera.org:8080/#/c/18041/2//COMMIT_MSG@17
PS2, Line 17: This patch properly translates the host index to the coordinators host
> nit properly
Done
http://gerrit.cloudera.org:8080/#/c/18041/2//COMMIT_MSG@21
PS2, Line 21: manually
> nit. I wonder if it is feasible to have a query test to protect this piece
I was able to add a unit test.
http://gerrit.cloudera.org:8080/#/c/18041/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/18041/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@458
PS2, Line 458: Des
> may need to check fd is not null.
Done
http://gerrit.cloudera.org:8080/#/c/18041/2/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/18041/2/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java@111
PS2, Line 111: snapshot
> Should we check snapshot is null?
loadIcebergSnapshot() shouldn't return null, but added Preconditions check anyway.
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Nov 2021 14:51:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 2:
(5 comments)
Thanks for the comments!
http://gerrit.cloudera.org:8080/#/c/18041/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18041/1//COMMIT_MSG@18
PS1, Line 18: block locations remain consistent.
> t
Done
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@455
PS1, Line 455: } else {
> precondition about hostIndex != null?
Done
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1014
PS1, Line 1014: Map<String, FileDescriptor> pathToFds =
> code in comment
Done
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
File fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java:
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java@500
PS1, Line 500: new TScanRangeLocation(analyzer.getHostIndex().getIndex(networkAddress)));
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/util/ListMap.java
File fe/src/main/java/org/apache/impala/util/ListMap.java:
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/util/ListMap.java@56
PS1, Line 56: public int getIndex(T t) {
> can you add a comment about this rename in the comment, or possibly move it
Opened IMPALA-11031
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 16:31:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
File fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java:
http://gerrit.cloudera.org:8080/#/c/18041/1/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java@500
PS1, Line 500: new TScanRangeLocation(analyzer.getHostIndex().getOrAddIndex(networkAddress)));
line too long (93 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 14:12:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9814/ : 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/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 16:51:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 2:
(5 comments)
Nice work!
http://gerrit.cloudera.org:8080/#/c/18041/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18041/2//COMMIT_MSG@12
PS2, Line 12: but the hosts are only referred by indexes. In the Coordinator's local
nit: referred to
http://gerrit.cloudera.org:8080/#/c/18041/2//COMMIT_MSG@17
PS2, Line 17: This patch translates the host index to the coordinators host list, so
nit properly
http://gerrit.cloudera.org:8080/#/c/18041/2//COMMIT_MSG@21
PS2, Line 21: manually
nit. I wonder if it is feasible to have a query test to protect this piece of work.
http://gerrit.cloudera.org:8080/#/c/18041/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/18041/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@458
PS2, Line 458: fd.
may need to check fd is not null.
http://gerrit.cloudera.org:8080/#/c/18041/2/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/18041/2/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java@111
PS2, Line 111: snapshot
Should we check snapshot is null?
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 20:48:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3:
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7663/
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Nov 2021 04:08:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Nov 2021 11:34:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3: Code-Review+1
Looks great and thanks a lot for the work!
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Nov 2021 16:26:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7666/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Nov 2021 07:14:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18041 )
Change subject: IMPALA-11022: Impala uses wrong file descriptors for Iceberg tables in local catalog mode
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9825/ : 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/18041
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253b505846e1cf4d1be445c0d06b2552dc4ba1f8
Gerrit-Change-Number: 18041
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Nov 2021 15:13:09 +0000
Gerrit-HasComments: No