Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21175 )
Change subject: IMPALA-12829: Skip processing transaction events if the table is HMS sync disabled. ...................................................................... Patch Set 7: (7 comments) http://gerrit.cloudera.org:8080/#/c/21175/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21175/7//COMMIT_MSG@17 PS7, Line 17: we need : special logic in the process() method. Please explain the solution a little bit here. http://gerrit.cloudera.org:8080/#/c/21175/7/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/21175/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4516 PS7, Line 4516: String tblFlagVal = tbl.getParameters() : .get(MetastoreEventPropertyKey.DISABLE_EVENT_HMS_SYNC.getKey()); : if (tblFlagVal != null ) { : return Boolean.valueOf(tblFlagVal.trim()); : } > getHmsSyncProperty() is a method of MetastoreTableEvent class and CommitTXN I think we can use the static method directly, i.e. MetastoreTableEvent.getHmsSyncProperty(tbl). Or lift that method up to MetastoreEvent class. This method can also be static and be moved into MetastoreEvent class. http://gerrit.cloudera.org:8080/#/c/21175/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4523 PS7, Line 4523: if (Boolean.valueOf(dbFlagVal)) { : return true; : } : return false; nit: can be simplified to return Boolean.parseBoolean(dbFlagVal); http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py@1506 PS7, Line 1506: from tests.util.acid_txn import AcidTxn This duplicates L34. http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py@1512 PS7, Line 1512: "CREATE TABLE {}.{} (i int) partitioned by (year int) {}" Can we also test a non-partitioned table? http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py@1515 PS7, Line 1515: # self.client.execute("select * from {}.{}".format(txn_db, txn_table)) stale comment http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py@1539 PS7, Line 1539: assert partitions_refreshed_after == partitions_refreshed_before These pass even without the fix. I think the reason is the table has no partitions in Impala's catalog cache since the ADD_PARTITION events generated by the insert statement are already skipped. We need to add the partition in Impala before inserting it in Hive. -- To view, visit http://gerrit.cloudera.org:8080/21175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d0ecb3b756755bc04c66a538a9ae6b88011a019 Gerrit-Change-Number: 21175 Gerrit-PatchSet: 7 Gerrit-Owner: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Reviewer: Anonymous Coward <k.venureddy2...@gmail.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: Tue, 29 Apr 2025 08:47:51 +0000 Gerrit-HasComments: Yes