Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20367 )
Change subject: IMPALA-10976: Sync db/table in catalogD to latest HMS event id for all DDLS from impala shell ...................................................................... Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/20367/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20367/1//COMMIT_MSG@8 PS1, Line 8: impala shell I think that impala shell is misleading here, as any client initiate DDLs. http://gerrit.cloudera.org:8080/#/c/20367/1//COMMIT_MSG@10 PS1, Line 10: from Impala shel Same as for the title, IMO "performed by Impala" would be better. http://gerrit.cloudera.org:8080/#/c/20367/1//COMMIT_MSG@14 PS1, Line 14: Impala shell same as for the title http://gerrit.cloudera.org:8080/#/c/20367/1//COMMIT_MSG@20 PS1, Line 20: HIVE-27499 Do you think that this feature can practically work before HIVE-27499? My impression is that the current solution adds some overhead (getting the table from HMS once more during DDLs) while it doesn't solve the issue completely, as in theory anything can happen in HMS between the RPC and the further processing of the DDL in Impala. http://gerrit.cloudera.org:8080/#/c/20367/1//COMMIT_MSG@24 PS1, Line 24: Set enable_sync_to_latest_event_on_ddls to true to use this feature. This flag is not exposed yet, we can only set it in tests. Do you plan to use a global flag for this or a query option? http://gerrit.cloudera.org:8080/#/c/20367/1/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/20367/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1369 PS1, Line 1369: the Db with the given metastore database This function seems to be about tables, not databases. http://gerrit.cloudera.org:8080/#/c/20367/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/20367/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1012 PS1, Line 1012: msTbl = getTableFromMetaStore(tableName); Calling an HMS RPC when the global catalog version lock is locked is problematic - this means that practically no catalog operation could run in parallel. Different functions seem to be inconsistent about this some do the RPC after unlocking the version lock. http://gerrit.cloudera.org:8080/#/c/20367/1/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/20367/1/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@1488 PS1, Line 1488: impala shell We are not using the shell here (or any Impala client) here, right? My understanding is that tests call the functions of CatalogOpExecutor directly. http://gerrit.cloudera.org:8080/#/c/20367/1/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@1498 PS1, Line 1498: BackendConfig.INSTANCE.setHMSPollingIntervalInSeconds(10); Will this have any effect? We seem check this only once, when the event processor is initialized: https://github.com/apache/impala/blob/9adfe0587cfaf3364c8a3672b1c60b198ef35a89/fe/src/main/java/org/apache/impala/service/JniCatalog.java#L210 -- To view, visit http://gerrit.cloudera.org:8080/20367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf Gerrit-Change-Number: 20367 Gerrit-PatchSet: 1 Gerrit-Owner: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Comment-Date: Wed, 16 Aug 2023 15:08:42 +0000 Gerrit-HasComments: Yes