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 <[email protected]> Gerrit-Reviewer: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Tue, 19 May 2020 17:28:12 +0000 Gerrit-HasComments: Yes
