Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/23942 )
Change subject: IMPALA-14230: Add catch-up mode for event processing ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/23942/5/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/23942/5/be/src/catalog/catalog-server.cc@124 PS5, Line 124: "to speed up the event processing. " typo: double space in "up the". I would also remove "the", it's not necessary when referring to the action of event processing. http://gerrit.cloudera.org:8080/#/c/23942/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/23942/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@975 PS5, Line 975: infoLog("Catch-up Mode: Skipping event due to event lag of {}s", lag); I think this would make sense to log before we start invalidating tables rather than after. http://gerrit.cloudera.org:8080/#/c/23942/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@1042 PS5, Line 1042: for (TableWriteId tableWriteId : tableWriteIds) { nit: the formatting changes may make this harder to backport to older releases. http://gerrit.cloudera.org:8080/#/c/23942/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/23942/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@3750 PS5, Line 3750: protected boolean handleIfInCatchUpMode() { We should be able to avoid a lot of the duplication across these 3 implementations with a static method. http://gerrit.cloudera.org:8080/#/c/23942/5/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/23942/5/tests/custom_cluster/test_events_custom_configs.py@680 PS5, Line 680: catalogd_args="--hms_event_polling_interval_s=1 " nit: could we set this cluster up once and use it for multiple tests? It'd require a new test class with these args set for the class definition. -- To view, visit http://gerrit.cloudera.org:8080/23942 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib906c06346d5d3159999eeac632e1318bc060065 Gerrit-Change-Number: 23942 Gerrit-PatchSet: 5 Gerrit-Owner: Yida Wu <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 24 Feb 2026 21:52:41 +0000 Gerrit-HasComments: Yes
