Yida Wu 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 6:

(5 comments)

This patch also updated COMMIT_TXN and ABORT_TXN transaction related tests in 
catch-up mode to improve accuracy

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 necess
Done


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:
> I think this would make sense to log before we start invalidating tables ra
Done


http://gerrit.cloudera.org:8080/#/c/23942/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@1042
PS5, Line 1042:       Map<TableName, AcidTableWriteInfo> tableNameToWriteInfos 
= new HashMap<>();
> nit: the formatting changes may make this harder to backport to older relea
Done


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:               event.getMessage());
> We should be able to avoid a lot of the duplication across these 3 implemen
Moved handleIfInCatchUpMode() to the parent class MetastoreEvent to handle 
shared logic, and added a new invalidateTablesInCatchUpMode() for subclasses to 
implement. This can reduce the duplication


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:       impalad_args="--use_local_catalog=false",
> nit: could we set this cluster up once and use it for multiple tests?
Added a new class TestEventProcessingCatchupMode. Because COMMIT_TXN is a 
cluster wide event and the test depends on logs to verify, I also made some 
changes to test_catchup_mode_commit_txn to adapt it to the non restarting 
cluster



--
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: 6
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: Wed, 25 Feb 2026 04:27:13 +0000
Gerrit-HasComments: Yes

Reply via email to