You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Peikai Zheng (Code Review)" <ge...@cloudera.org> on 2018/07/09 02:46:58 UTC

[Impala-ASF-CR] IMPALA-5035 Impala should not load INDEX TABLEs from HMS

Peikai Zheng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10869


Change subject: IMPALA-5035 Impala should not load INDEX_TABLEs from HMS
......................................................................

IMPALA-5035 Impala should not load INDEX_TABLEs from HMS

Block loading INDEX_TABLEs from HMS.
The INDEX_TABLEs were cached as 'Incompleted table' entry, before.
Now, the INDEX_TABLEs are not cached actually.

Before Changes:
> show tables;
user;  // external table
hive_index_tbl; // index table on user
region; // external table
> show create table hive_index_tbl;
Query: show create table hive_index_tbl
ERROR: AnalysisException: org.apache.impala.catalog.TableLoadingException: Unsupported table type 'INDEX_TABLE' for: hive_index_tbl
CAUSED BY: TableLoadingException: Unsupported table type 'INDEX_TABLE' for: hive_index_tbl

After Changes:
> show tables;
user; // external table
region; //external table
> show create talbe hive_index_tbl;
Query: show create table hive_index_tbl
ERROR: AnalysisException: Table does not exist: hive_index_tbl

Change-Id: I6e2ccbeee3e612eab96d89a211d15fc119b08d70
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
4 files changed, 16 insertions(+), 17 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/10869/4
-- 
To view, visit http://gerrit.cloudera.org:8080/10869
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6e2ccbeee3e612eab96d89a211d15fc119b08d70
Gerrit-Change-Number: 10869
Gerrit-PatchSet: 4
Gerrit-Owner: Peikai Zheng <bn...@gmail.com>

[Impala-ASF-CR] IMPALA-5035 Impala should not load INDEX TABLEs from HMS

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10869 )

Change subject: IMPALA-5035 Impala should not load INDEX_TABLEs from HMS
......................................................................


Patch Set 5:

(4 comments)

In addition to the one implementation concern, I'm not sure that I agree with the behavior change here. It seems inconsistent in a few ways:

1) for other unsupported table types, we still list them, and fail to load. Why should index tables be treated differently than unsupported serdes? Won't this produce confusing behavior if someone attempts to create a table with a conflicting name?

2) Seems inconsistent with Hive as well. 'SHOW TABLES IN functional;' in hive does show hive_index_tbl. Why introduce the inconsistency between engines?

Perhaps we'd be better off just throwing a more useful exception like "Hive index tables are not supported" if we think that's better than the 'Unsupported table type: INDEX_TABLE' message?

http://gerrit.cloudera.org:8080/#/c/10869/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10869/5//COMMIT_MSG@28
PS5, Line 28: egion; //external table
it seems you have some typos here. Did you not just copy-paste this from the shell?


http://gerrit.cloudera.org:8080/#/c/10869/5//COMMIT_MSG@29
PS5, Line 29: > show create talbe hive_index_tbl;
here too


http://gerrit.cloudera.org:8080/#/c/10869/5//COMMIT_MSG@31
PS5, Line 31: ERROR: AnalysisException: Table does not exist: hive_index_tbl
what is the behavior if you try to create a table with the same name that conflicts with a Hive index table?


http://gerrit.cloudera.org:8080/#/c/10869/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/10869/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1055
PS5, Line 1055:       List<org.apache.hadoop.hive.metastore.api.Table> tableList =
              :               msClient.getHiveClient().getTableObjectsByName(dbName, tableNameList);
I'm worried about the performance of this in metastores with large numbers of tables. Loading the full table object at this stage is a lot more expensive than just the names. In a recent case I was working on with a production cluster there were many millions of tables and putting this here would be very harmful to performance.



-- 
To view, visit http://gerrit.cloudera.org:8080/10869
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e2ccbeee3e612eab96d89a211d15fc119b08d70
Gerrit-Change-Number: 10869
Gerrit-PatchSet: 5
Gerrit-Owner: Peikai Zheng <bn...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 09 Jul 2018 17:53:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5035 Impala should not load INDEX TABLEs from HMS

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has abandoned this change. ( http://gerrit.cloudera.org:8080/10869 )

Change subject: IMPALA-5035 Impala should not load INDEX_TABLEs from HMS
......................................................................


Abandoned

I'm going to mark this as abandoned for now so that it doesn't show up in open CR searches. Feel free to revive it if needed.
-- 
To view, visit http://gerrit.cloudera.org:8080/10869
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I6e2ccbeee3e612eab96d89a211d15fc119b08d70
Gerrit-Change-Number: 10869
Gerrit-PatchSet: 5
Gerrit-Owner: Peikai Zheng <bn...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] IMPALA-5035 Impala should not load INDEX TABLEs from HMS

Posted by "Peikai Zheng (Code Review)" <ge...@cloudera.org>.
Peikai Zheng has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/10869 )

Change subject: IMPALA-5035 Impala should not load INDEX_TABLEs from HMS
......................................................................

IMPALA-5035 Impala should not load INDEX_TABLEs from HMS

Block loading INDEX_TABLEs from HMS.
The INDEX_TABLEs were cached as 'Incompleted table' entry, before.
Now, the INDEX_TABLEs are not cached actually.

Before Changes:
> show tables;
user;  // external table
hive_index_tbl; // index table on user
region; // external table
> show create table hive_index_tbl;
Query: show create table hive_index_tbl
ERROR: AnalysisException: org.apache.impala.catalog.TableLoadingException:
Unsupported table type 'INDEX_TABLE' for: hive_index_tbl
CAUSED BY: TableLoadingException: Unsupported table type 'INDEX_TABLE'
for: hive_index_tbl

After Changes:
> show tables;
user; // external table
egion; //external table
> show create talbe hive_index_tbl;
Query: show create table hive_index_tbl
ERROR: AnalysisException: Table does not exist: hive_index_tbl

Jenkins pre-review-test:
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/2674/testReport/

Change-Id: I6e2ccbeee3e612eab96d89a211d15fc119b08d70
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
4 files changed, 16 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/10869/5
-- 
To view, visit http://gerrit.cloudera.org:8080/10869
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6e2ccbeee3e612eab96d89a211d15fc119b08d70
Gerrit-Change-Number: 10869
Gerrit-PatchSet: 5
Gerrit-Owner: Peikai Zheng <bn...@gmail.com>