Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/24629 )
Change subject: IMPALA-14618: Fix EventProcessor race with catalog reset ...................................................................... Patch Set 5: (5 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: Testing : - Added e2e tests by adding debug actions to inject sleeps. : - Ran test_failover_catchup_timeout_and_reset and : TestEventProcessingRace 100 times. : > It may be better to move this to another ticket + patch, especially if this Sure. Fixing this in https://gerrit.cloudera.org/c/24698/ 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: } > Is this line useful in general, or it is just for the tests? If the latter, It was useful when I was debugging the issue. I added it after the getDb() call so I can check whether it happens before or after the catalog reset thread loads the db. Later I used it for tests. Now it's still useful for showing the order with catalog reset. I'll change the test to not depend on this. http://gerrit.cloudera.org:8080/#/c/24629/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1027 PS3, Line 1027: LOG.info("EventId: {} Processing CREATE_DATABASE for db: {}", eventId, dbName); : DebugUtils.executeDebugAction(BackendConf > Is it important to do this exactly at this point? Waiting before getting dd While reproducing the issue, I add this right before the addDb() call so there is a long duration between checking the db and adding it. Now we don't call addDb() here. So I just put it before reading the db. getMetastoreDdlLock() is not a key point in this bug since catalog reset doesn't acquire this lock. I thought about moving this into addDbIfNotExists() so it still injects the delay between the check and add of the db. However, that blocks the concurrent reset while holding the catalog versionLock_ in WriteLockAndLookupDb(). We want the concurrent reset to finish before the event processing thread adds the db (wrong behavior) or skips the event (expected behavior). 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"Database {0} was not added".format(db)) : : tables = self.execute_query("show tables in {0}" > Won't this match the previous "Processing CREATE_DATABASE" line? assert_cat Oops, I should use "<=" in comparing after_time. Waiting until the sleep finish is not enough since it can't guarantee that the event has been processed/skipped. We can simplify this to wait for the log of "Database xxx was not added since it either exists or was removed since the event was generated". http://gerrit.cloudera.org:8080/#/c/24629/3/tests/custom_cluster/test_events_custom_configs.py@2418 PS3, Line 2418: self.assert_catalogd_log_contains( > Will this wait for the debug action to be finished? My assumption is thte I No, INVALIDATE METADATA doesn't need the metastoreDdlLock. -- 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: 5 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 10:38:01 +0000 Gerrit-HasComments: Yes
