Csaba Ringhofer 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 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/21175/4/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/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4214 PS4, Line 4214: if (val == null || val.isEmpty()) return false; Why do we return in this case? Shouldn't we also check the db flag? http://gerrit.cloudera.org:8080/#/c/21175/4/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/21175/4/tests/custom_cluster/test_events_custom_configs.py@1361 PS4, Line 1361: -hms_event_polling_interval_s=5") Is it necessary to have lowered hms polling frequency in this test? Couldn't it be a serialized normal (not custom cluster) test? http://gerrit.cloudera.org:8080/#/c/21175/4/tests/custom_cluster/test_events_custom_configs.py@1366 PS4, Line 1366: txn_db = "txn_db" Couldn't we use the unique_database fixture? -- 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: 4 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: Tue, 11 Feb 2025 12:47:48 +0000 Gerrit-HasComments: Yes
