Quanlong Huang 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 after the DDL operation for all DDLS from Impala clients ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/20367/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20367/6//COMMIT_MSG@7 PS6, Line 7: IMPALA-10976: Sync db/table in catalogD to latest : HMS event id after the DDL operation for all DDLS : from Impala clients nit: I think we can make it in one line and shorter, e.g. IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs http://gerrit.cloudera.org:8080/#/c/20367/4/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/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7040 PS4, Line 7040: "regarding the failed un/caching operations."); : response.getResult().setStatus( : new TStatus(TErrorCode.INTERNAL_ERROR, errorMessages)); : } else { : response.getResult().setStatus( : new TStatus(TErrorCode.OK, new ArrayList<String>())); : } : boolean syncToLatestEventId = : BackendConfig.INSTANCE.enableSyncToLatestEventOnDdls(); : / > ACK. Agree that we can keep invalidate as-is. For REFRESH, I think at least we should support "REFRESH <table>" which can simply invoke MetastoreEventsProcessor.syncToLatestEventId(). Other REFRESH statements can be skipped: * 'refresh functions <db_name>' can't be supported since we don't support function events yet (IMPALA-10273). Only Impala builtin functions can be synced in db events since their metadata are in the db parameters. * Partition level refresh needs further discussion to decide whether to sync the whole table including other partitions as well * 'refresh authorization' can be as-is since there are no Ranger policy events. http://gerrit.cloudera.org:8080/#/c/20367/6/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/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1145 PS6, Line 1145: refreshedTable.setCatalogVersion(newCatalogVersion); I think we should skip this as well when enableSyncToLatestEventOnDdls is on. MetastoreEventsProcessor.syncToLatestEventId() might already bump the version to a higher one. We also need e2e tests to capture issues like this. http://gerrit.cloudera.org:8080/#/c/20367/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1185 PS6, Line 1185: refreshedTable.setCatalogVersion(newCatalogVersion); Same issue as L1145, this might reset the catalog version to an older one when enableSyncToLatestEventOnDdls is on. -- 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: 6 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Mon, 06 Nov 2023 13:41:08 +0000 Gerrit-HasComments: Yes
