Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24629 )
Change subject: IMPALA-14618: Fix EventProcessor race with catalog reset ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/24629/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24629/3//COMMIT_MSG@78 PS3, Line 78: This patch also adds a missing comma in the default value of : common_hms_event_types. This is used in passing the EventTypeSkipList to : HMS while fetching specific types of events. The bug won't impact : correctness since the filters are also evaluated in client side : (catalogd). It may be better to move this to another ticket + patch, especially if this one takes a long time to merge. http://gerrit.cloudera.org:8080/#/c/24629/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/24629/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1026 PS3, Line 1026: LOG.info("EventId: {} Processing CREATE_DATABASE for db: {}", eventId, dbName); Is this line useful in general, or it is just for the tests? If the latter, the tests could rely on log line from the debug action. http://gerrit.cloudera.org:8080/#/c/24629/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1027 PS3, Line 1027: DebugUtils.executeDebugAction(BackendConfig.INSTANCE.debugActions(), : DebugUtils.ADD_DB_AFTER_CHECK_DELAY); Is it important to do this exactly at this point? Waiting before getting ddl lock seems more powerful to me. + same for addTableIfNotRemovedLater http://gerrit.cloudera.org:8080/#/c/24629/3/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/24629/3/tests/custom_cluster/test_events_custom_configs.py@2395 PS3, Line 2395: self.assert_catalogd_log_contains( : "INFO", r"^\S+ \S+ {0} .*{1}".format(event_thread_id, db), : expected_count=-1, after_time=last_log_time) Won't this match the previous "Processing CREATE_DATABASE" line? assert_catalogd_log_contains accepts lines where time==after_time IMO the cleanest would be to wait for log lines that mark the start and end of the sleep from the debug action. http://gerrit.cloudera.org:8080/#/c/24629/3/tests/custom_cluster/test_events_custom_configs.py@2418 PS3, Line 2418: self.execute_query("invalidate metadata") Will this wait for the debug action to be finished? My assumption is thte INVALIDATE METADATA needs the ddl lock, so it will wait for it. This may worth a comment. http://gerrit.cloudera.org:8080/#/c/24629/3/tests/custom_cluster/test_events_custom_configs.py@2422 PS3, Line 2422: self.assert_catalogd_log_contains( Same as line 2395 -- To view, visit http://gerrit.cloudera.org:8080/24629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa0833c09cd7ec2461a0f2d1db7ab5a72ab6822b Gerrit-Change-Number: 24629 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Nandor Kollar <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Mon, 17 Aug 2026 06:55:25 +0000 Gerrit-HasComments: Yes
