You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2020/05/08 03:25:51 UTC
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15887
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
In fact, if the hmsTable object of a table/view is loaded in local
cache, we can extract its exact table type and comment. This patch fixes
the problem by adding an additional lookup in the local cache for the
table meta (hmsTable) of each table when creating the
LocalIncompleteTables. When getting the table type and comment from a
LocalIncompleteTable, use the loaded hmsTable.
Tests
- Add tests in test_hs2.test_get_tables
- Run CORE tests
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
---
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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/hs2/test_hs2.py
7 files changed, 90 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/15887/1
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 7:
Added some comments and fixed the failed tests
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 May 2020 11:02:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15887/6/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/15887/6/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@721
PS6, Line 721: invalidateStaleTableList(dbName.toLowerCase(), ref);
> I see. This solution seems hacky to me. Does SYNC_DDL help in this case? It
Just a side note, if we're running with catalog_topic_mode=minimal, we could safely increase the frequency of statestore topic updates. We have it running at 100ms for cluster membership and admission control updates, and the catalog invalidations are probably not much larger than those.
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 May 2020 17:32:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 9
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 May 2020 17:27:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 May 2020 07:51:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15887/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15887/2//COMMIT_MSG@21
PS2, Line 21: his patch fixes
: the problem by adding an additional lookup in the local cache for the
: table meta (hmsTable) of each table when creating the
: LocalIncompleteTables
> Good point! Load-balancers will cause the inconsistent results. I planned t
Thanks. For now, this solution is better than what we have until we have the fix for IMPALA-9670.
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2020 17:02:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 9: Code-Review+1
The patch looks good to me. Thanks for addressing my comments. Giving a +1 since Tim is also reviewing. Will let him give a +1 in case he is still reviewing else I can upgrade my vote to a +2.
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 9
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 May 2020 16:49:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 6: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5844/
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2020 19:41:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 May 2020 23:42:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15887
to look at the new patch set (#7).
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
This is fixed by loading table types and comments along with table names
from catalogd. The cached list of table names of a DB is changed to be a
map containing the brief table metadata (name, type, comment). In case
of stale types or comments in the list, when loading the msTable of a
table, coordinator checks the type and comment and invalidates the table
list if any of them are stale.
Tests
- Add tests in test_hs2.test_get_tables
- Run CORE tests
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Db.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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
M tests/hs2/test_hs2.py
M tests/query_test/test_observability.py
12 files changed, 224 insertions(+), 78 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/15887/7
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5873/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 May 2020 23:42:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 6:
(1 comment)
The latest patch looks good to me except for the comment below. Also, curious to know if we still need IMPALA-9670 given now we will always fetch the type and comments.
http://gerrit.cloudera.org:8080/#/c/15887/6/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/15887/6/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@721
PS6, Line 721: invalidateStaleTableList(dbName.toLowerCase(), ref);
Why do we need this? If the table type or comment has changed since we loaded the tablelist shouldn't we have received a invalidation topic update for that table?
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2020 17:19:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15887
to look at the new patch set (#8).
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
This is fixed by loading table types and comments along with table names
from catalogd. The cached list of table names of a DB is changed to be a
map containing the brief table metadata (name, type, comment). In case
of stale types or comments in the list, when loading the msTable of a
table, coordinator checks the type and comment and invalidates the table
list if any of them are stale.
Tests
- Add tests in test_hs2.test_get_tables and manually test it in
LocalCatalog mode.
- Run CORE tests.
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Db.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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
M tests/hs2/test_hs2.py
M tests/query_test/test_observability.py
12 files changed, 248 insertions(+), 78 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/15887/8
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 10: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 May 2020 05:01:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@416
PS1, Line 416: Table msTbl = table.getMetaStoreTable();
> Is msTbl always guaranteed to be not null?
No, for unloaded tables (i.e. IncompleteTable in catalog-v1, FailedLoadLocalTable and LocalIncompleteTable created with only db, table names in catalog-v2) it's null.
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@419
PS1, Line 419: getOrDefault(tableTypeStr, TABLE_TYPE_TABLE)
> It is concerning that we are defaulting to TABLE type here when in fact we
Yeah, it's a bad default value but it's our current behavior... From the jdbc java doc [1], the typical types are "TABLE", "VIEW", "SYSTEM TABLE", "GLOBAL TEMPORARY", "LOCAL TEMPORARY", "ALIAS", "SYNONYM". It is not against returning a value like “UNKNOWN”. But I'm not sure whether clients will be broken and ignore tables in "UNKNOWN" type. Since this is a bug-fix patch, can we keep the current behavior? BTW, after IMPALA-9670, we should be able to show correct table types.
[1] https://docs.oracle.com/javase/8/docs/api/java/sql/DatabaseMetaData.html#getTables-java.lang.String-java.lang.String-java.lang.String-java.lang.String:A-
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 May 2020 03:17:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5853/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 May 2020 07:42:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15887
to look at the new patch set (#2).
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
In fact, if the hmsTable object of a table/view is loaded in local
cache, we can extract its exact table type and comment. This patch fixes
the problem by adding an additional lookup in the local cache for the
table meta (hmsTable) of each table when creating the
LocalIncompleteTables. When getting the table type and comment from a
LocalIncompleteTable, use the loaded hmsTable.
Tests
- Add tests in test_hs2.test_get_tables
- Run CORE tests
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
---
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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/hs2/test_hs2.py
7 files changed, 99 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/15887/2
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15887/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15887/7//COMMIT_MSG@20
PS7, Line 20: This is fixed by loading table types and comments along with table names
I guess we never had SYNC_DDL semantics for GET_TABLES, did we? I.e. a table load could trigger a change in the results of GET_TABLES, but we don't block waiting for that invalidation to be propagated to other impalads. I don't think we need to fix it, I just noticed this in the current code and wanted to confirm it was expected.
http://gerrit.cloudera.org:8080/#/c/15887/7/fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java:
http://gerrit.cloudera.org:8080/#/c/15887/7/fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java@31
PS7, Line 31: private final String msTableType;
These can be null if it's partially loaded, right?
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 May 2020 17:29:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5852/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Sun, 17 May 2020 08:18:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Sun, 17 May 2020 13:36:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5826/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 May 2020 02:18:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
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/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
This is fixed by loading table types and comments along with table names
from catalogd. The cached list of table names of a DB is changed to be a
map containing the brief table metadata (name, type, comment). In case
of stale types or comments in the list, when loading the msTable of a
table, coordinator checks the type and comment and invalidates the table
list if any of them are stale.
Tests
- Add tests in test_hs2.test_get_tables and manually test it in
LocalCatalog mode.
- Run CORE tests.
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Reviewed-on: http://gerrit.cloudera.org:8080/15887
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Db.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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
M tests/hs2/test_hs2.py
M tests/query_test/test_observability.py
12 files changed, 249 insertions(+), 78 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 11
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 May 2020 17:32:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6114/ : 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/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 May 2020 12:41:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15887
to look at the new patch set (#5).
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
This is fixed by loading table types and comments along with table names
from catalogd. The cached list of table names of a DB is changed to be a
map containing the brief table metadata (name, type, comment). In case
of stale types or comments in the list, when loading the msTable of a
table, coordinator checks the type and comment and invalidates the table
list if any of them are stale.
Tests
- Add tests in test_hs2.test_get_tables
- Run CORE tests
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Db.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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
M tests/hs2/test_hs2.py
11 files changed, 199 insertions(+), 68 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/15887/5
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 9:
(4 comments)
Thanks for the comments! Revised the patch accordingly.
Also make some changes to avoid unnecessarily invalidating the table list in usual case (cachedType is null and loadedType is table).
http://gerrit.cloudera.org:8080/#/c/15887/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15887/7//COMMIT_MSG@20
PS7, Line 20: This is fixed by loading table types and comments along with table names
> I guess we never had SYNC_DDL semantics for GET_TABLES, did we? I.e. a tabl
Yeah, SYNC_DDL only works for DDL/DMLs. To be specifit, only execDdlRequest RPCs with SYNC_DDL=true will wait for the topic update that covers the changes being sent:
https://github.com/apache/impala/blob/6fcc758c1aa0b6075738edb76371a0cc43d04388/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#L497
For catalog-v2, table loading is triggered by getPartialCatalogObject RPCs which is another code path.
http://gerrit.cloudera.org:8080/#/c/15887/6/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/15887/6/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@721
PS6, Line 721: });
> Just a side note, if we're running with catalog_topic_mode=minimal, we coul
@Vihang, table loading doesn't have SYNC_DDL option. only execDdlRequest RPCs will deal with SYNC_DDL. I plan to add a flag for IMPALA-9670 since it will increase the catalogd launch time. If the flag is turned off, we still need this...
@Tim, thanks for providing the workaround!
I'll add a TODO for removing this code path.
http://gerrit.cloudera.org:8080/#/c/15887/7/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/15887/7/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@723
PS7, Line 723: aded at the time in catalogd, we assume that the table type is "TABLE" and
: // comment is empty (See MetadataOp.getTableType and MetadataOp.getT
> Do you its clearer if we say it like this? "The table list is populated bas
Yeah, thanks for giving a clearer comment!
http://gerrit.cloudera.org:8080/#/c/15887/7/fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java:
http://gerrit.cloudera.org:8080/#/c/15887/7/fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java@31
PS7, Line 31: public class LocalIncompleteTable extends LocalTable implements FeIncompleteTable {
> These can be null if it's partially loaded, right?
Yes, added a Nullable annotation to emphasize this. We deal with nulls in MetadataOp.getTableType() and MetadataOp.getTableComment().
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 9
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 May 2020 11:59:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 6:
(1 comment)
> Patch Set 6:
>
> (1 comment)
>
> The latest patch looks good to me except for the comment below. Also, curious to know if we still need IMPALA-9670 given now we will always fetch the type and comments.
We still need IMPALA-9670. After this patch, if the table is loaded in catalogd, coordinators can show its correct type and comment regardless whether it is loaded in local catalog. But if the table is unloaded in catalogd, we are still using the default table type and comment.
http://gerrit.cloudera.org:8080/#/c/15887/6/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/15887/6/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@721
PS6, Line 721: invalidateStaleTableList(dbName.toLowerCase(), ref);
> Why do we need this? If the table type or comment has changed since we load
Sorry for not leaving a comment. This is the only point that makes this patch a little complex:
We will receive a topic update in 2s (default statestore update frequency). After the table is loaded here and before the invalidation arrives, the stale tablelist will still be used. So GetTable op right after a DESCRIBE query will still get the stale type and comment. This can be reflected by test failure of test_hs2.test_get_tables.
There are some test failures due to this. Will add the comment and fix the tests.
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 May 2020 02:00:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15887
to look at the new patch set (#6).
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
This is fixed by loading table types and comments along with table names
from catalogd. The cached list of table names of a DB is changed to be a
map containing the brief table metadata (name, type, comment). In case
of stale types or comments in the list, when loading the msTable of a
table, coordinator checks the type and comment and invalidates the table
list if any of them are stale.
Tests
- Add tests in test_hs2.test_get_tables
- Run CORE tests
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Db.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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
M tests/hs2/test_hs2.py
11 files changed, 199 insertions(+), 68 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/15887/6
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15887
to look at the new patch set (#4).
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
This is fixed by loading table types and comments along with table names
from catalogd. The cached list of table names of a DB is changed to be a
map containing the brief table metadata (name, type, comment). In case
of stale types or comments in the list, when loading the msTable of a
table, coordinator checks the type and comment and invalidates the table
list if any of them are stale.
Tests
- Add tests in test_hs2.test_get_tables
- Run CORE tests
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Db.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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
M tests/hs2/test_hs2.py
11 files changed, 199 insertions(+), 68 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/15887/4
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15887
to look at the new patch set (#3).
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
In fact, if the hmsTable object of a table/view is loaded in local
cache, we can extract its exact table type and comment. This patch fixes
the problem by adding an additional lookup in the local cache for the
table meta (hmsTable) of each table when creating the
LocalIncompleteTables. When getting the table type and comment from a
LocalIncompleteTable, use the loaded hmsTable.
Tests
- Add tests in test_hs2.test_get_tables
- Run CORE tests
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
---
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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M tests/hs2/test_hs2.py
7 files changed, 102 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/15887/3
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15887/6/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/15887/6/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@721
PS6, Line 721: invalidateStaleTableList(dbName.toLowerCase(), ref);
> Sorry for not leaving a comment. This is the only point that makes this pat
I see. This solution seems hacky to me. Does SYNC_DDL help in this case? It would be good to keep things simple by having just one path for cache invalidation. As I understand, we need this invalidation here because we are storing a table_list in the cache which is based on our assumed defaults (tableType is TABLE when not available). Ideally, whatever the localCatalog caches should be derived from the its source of truth (catalog). For now, I think its okay to have this solution if tests are reliably passing. I hope that after IMPALA-9670, we can get rid of this call. If so, can you please add a TODO to remove this once IMPALA-9670 is fixed.
http://gerrit.cloudera.org:8080/#/c/15887/7/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/15887/7/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@723
PS7, Line 723:
:
Do you its clearer if we say it like this? "The table list is populated based on tables in a given Db in catalog. If a table is not loaded at the time in catalog, we assume that the table type is "TABLE" and comment is empty. However, if after issuing this load request, if we find that our assumption was incorrect, we need to invalidate the table list immediately. The table list gets invalidated automatically when we receive the topic-update but until then, all the getTables calls will show incorrect table type and comment for this table."
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 May 2020 17:28:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6080/ : 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/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2020 13:46:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@416
PS1, Line 416: Table msTbl = table.getMetaStoreTable();
Is msTbl always guaranteed to be not null?
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@419
PS1, Line 419: getOrDefault(tableTypeStr, TABLE_TYPE_TABLE)
It is concerning that we are defaulting to TABLE type here when in fact we are not sure. Is it possible to saying TableType is "UNKNOWN" in such a case?
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 May 2020 17:33:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@416
PS1, Line 416: Table msTbl = table.getMetaStoreTable();
> No, for unloaded tables (i.e. IncompleteTable in catalog-v1, FailedLoadLoca
Thanks for the clarification.
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@419
PS1, Line 419: getOrDefault(tableTypeStr, TABLE_TYPE_TABLE)
> I agree with Quanlong - I don't like this at all, but it's not worth breaki
Thanks. Agreed, lets fix it as part of IMPALA-9670.
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 May 2020 18:07:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15887/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15887/2//COMMIT_MSG@21
PS2, Line 21: his patch fixes
: the problem by adding an additional lookup in the local cache for the
: table meta (hmsTable) of each table when creating the
: LocalIncompleteTables
If I understand correctly it is possible that if there are multiple coordinators with the load-balancer in front of them, we will may be seeing different sets of tables which are loaded and hence the table-type and comment information will keep flipping depending on which coordinator is serving the data.
Do you think it will be simpler to always fetch the tablename, type and comment when we load the tableNames? Something like changing DbInfoSelector.want_table_names to DbInfoSelector.want_table_meta where meta includes tableName, type and comment. It doesn't seem like a big overhead given the benefit it provides will be a consistent view across coordinators.
http://gerrit.cloudera.org:8080/#/c/15887/2/fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java:
http://gerrit.cloudera.org:8080/#/c/15887/2/fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java@30
PS2, Line 30: */
nit, add a class comment which specifies that the LocalInCompleteTable could optionally have the msTbl object and scenarios when it will be present.
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 May 2020 20:02:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6115/ : 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/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 9
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 May 2020 12:50:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5844/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2020 14:20:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 2:
(2 comments)
Thanks for taking a look at this! Revised the patch based on feedbacks.
http://gerrit.cloudera.org:8080/#/c/15887/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/15887/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@693
PS1, Line 693: cached table
> nit: I think it will be useful to mention the table name and database name
Done
http://gerrit.cloudera.org:8080/#/c/15887/1/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/15887/1/tests/hs2/test_hs2.py@476
PS1, Line 476: # The returned results should only contain metadata for the table and the view.
> Maybe comment on what it's looping over and the order in which the table/vi
Sure. BTW, the order doesn't matter since we verify table_type and table_remarks based on table_name.
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 May 2020 01:54:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:
http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@419
PS1, Line 419: getOrDefault(tableTypeStr, TABLE_TYPE_TABLE)
> Yeah, it's a bad default value but it's our current behavior... From the jd
I agree with Quanlong - I don't like this at all, but it's not worth breaking clients en route to the proper fix (we'd need to figure out the impact on different clients if we were going to do that).
http://gerrit.cloudera.org:8080/#/c/15887/1/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/15887/1/tests/hs2/test_hs2.py@476
PS1, Line 476: for j in range(2):
Maybe comment on what it's looping over and the order in which the table/view are returned?
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 May 2020 17:52:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 3:
(2 comments)
Revised the class comment. Working on propagating types and comments when loading table names.
http://gerrit.cloudera.org:8080/#/c/15887/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15887/2//COMMIT_MSG@21
PS2, Line 21: his patch fixes
: the problem by adding an additional lookup in the local cache for the
: table meta (hmsTable) of each table when creating the
: LocalIncompleteTables
> If I understand correctly it is possible that if there are multiple coordin
Good point! Load-balancers will cause the inconsistent results. I planned to propagate the types and comments with table names when fixing IMPALA-9670. I'll do this in the comming patch to fix the load-balancer issue.
http://gerrit.cloudera.org:8080/#/c/15887/2/fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java:
http://gerrit.cloudera.org:8080/#/c/15887/2/fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java@30
PS2, Line 30: */
> nit, add a class comment which specifies that the LocalInCompleteTable coul
Done
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2020 02:08:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6047/ : 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/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 May 2020 02:45:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6074/ : 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/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2020 02:58:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15887/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/15887/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@693
PS1, Line 693: cached table
nit: I think it will be useful to mention the table name and database name in the logs.
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 May 2020 19:14:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6084/ : 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/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 May 2020 11:55:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15887
to look at the new patch set (#9).
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
Coordinator can be in two modes: legacy mode or LocalCatalog mode.
Before IMPALA-8606, GET_TABLES required all tables to be loaded in
LocalCatalog-mode coordinator’s cache, which is a performance regression
compared to legacy mode coordinators. IMPALA-8606 changes the behavior
to only load the table names and create LocalIncompleteTable for each
table, which boosts the performance but results in all views being
returned with the default table type (TABLE). Besides this, all returned
comments are empty even if the table/view is loaded. This is a
regression since in legacy coordinators, loaded tables/views are shown
with correct table types and comments.
This is fixed by loading table types and comments along with table names
from catalogd. The cached list of table names of a DB is changed to be a
map containing the brief table metadata (name, type, comment). In case
of stale types or comments in the list, when loading the msTable of a
table, coordinator checks the type and comment and invalidates the table
list if any of them are stale.
Tests
- Add tests in test_hs2.test_get_tables and manually test it in
LocalCatalog mode.
- Run CORE tests.
Change-Id: I2180c603f061838347936f718cd4a0257d82e633
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Db.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/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
M tests/hs2/test_hs2.py
M tests/query_test/test_observability.py
12 files changed, 249 insertions(+), 78 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/15887/9
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 9
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6007/ : 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/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 May 2020 04:19:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 9:
Thanks Vihang, Tim and Anurag for the review!
--
To view, visit http://gerrit.cloudera.org:8080/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 9
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 May 2020 23:42:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET TABLES in LocalCatalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 )
Change subject: IMPALA-9669: Fix wrong types/comments of loaded tables/views for GET_TABLES in LocalCatalog
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6081/ : 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/15887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2020 13:56:03 +0000
Gerrit-HasComments: No