Csaba Ringhofer 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 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/23942/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23942/4//COMMIT_MSG@20 PS4, Line 20: trigger heavy : operations. This always mean loading/refreshing tables/partitions, or there are also other heavy operations? http://gerrit.cloudera.org:8080/#/c/23942/4/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/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1242 PS4, Line 1242: if (catalog_.invalidateTableIfExists( : dbName_, tblName_, true /* skipIfIncomplete */) : != null) { I don't understand this "if". invalidateTableIfExists() has if (skipIfIncomplete && existingTbl instanceof IncompleteTable) { return existingTbl; } So the result will be non null even if the event is already invalidated. Isn't it the goal to log only if the table was not incomplete before this? http://gerrit.cloudera.org:8080/#/c/23942/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1978 PS4, Line 1978: Is it a good idea to evaluate this before isSelfEvent() and isOlderEvent()? Probably yes, to avoid hitting locks, but some comment about this could be useful. http://gerrit.cloudera.org:8080/#/c/23942/4/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/23942/4/tests/custom_cluster/test_events_custom_configs.py@728 PS4, Line 728: run_stmt_in_hive Is there a reason for creating the table in Hive? http://gerrit.cloudera.org:8080/#/c/23942/4/tests/custom_cluster/test_events_custom_configs.py@760 PS4, Line 760: Tests that various event types (INSERT, ADD_PARTITION, DROP_PARTITION) Shouldn't the test also check if the given event type invalidates the table? This could be done by always loading a table and waiting for events on each Hive statement. -- 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: 4 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: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Fri, 20 Feb 2026 14:46:32 +0000 Gerrit-HasComments: Yes
