linghengqian opened a new pull request, #2544: URL: https://github.com/apache/shardingsphere-elasticjob/pull/2544
Fixes #2543 . Changes proposed in this pull request: - Fixes the flaky tests caused by registry center races. - The `build` job in `.github/workflows/maven.yml` has been failing intermittently on `windows-latest`, `macos-latest` and the GraalVM leg. It reproduces at `2a747ab788f9ff0c1cde0d123cd7206bba480531` and the exception varies from run to run: `IllegalStateException: Client is not started or has been closed`, `org.apache.curator.framework.api.CuratorClosedException: Expected state [STARTED] was [STOPPED]` and `org.apache.shardingsphere.elasticjob.kernel.exception.JobConfigurationException: JobConfiguration was not found`. - `build` runs with `-T1C`, so several modules run tests at the same time and each module boots its own embedded registry center. When the CPU is shared the client session goes unstable and a read races the client shutdown. That is the trigger, but the failures only surface because the three places below are not prepared for it. - `ZookeeperRegistryCenter.close()` closed the caches and the client without deregistering the listeners it had registered, and the two listener maps were never emptied. `close()` now removes the data and connection-state listeners and clears the cache map before closing anything. `get()`, `getDirectly()`, `getChildrenKeys()`, `getNumChildren()` and `isExisted()` check the client state and return their normal not-found result instead of bubbling an exception, so a caller that reads after the center goes away degrades the same way as a read of a missing node. `watch()` no longer calls `listenable()` on a null cache, it logs and ignores the path. The new `ZookeeperRegistryCenterClosedTest` covers this; reverted to the old implementation the class fails with the very exceptions in the issue. - `AbstractDistributeOnceElasticJobListener` polled `isRegisterStartSuccess()` and `isRegisterCompleteSuccess()` in unbounded loops. There is now a single deadline measured before the registration is submitted and shared by the polling and the wait phases, so the register and wait together cannot exceed the configured timeout. A timed-out registration cleans up its `/started` or `/completed` nodes and throws `JobSystemException`, which matches the wait phase and stops ephemeral nodes from leaking. - `TriggerListenerManager` removed the trigger flag before checking whether the job could be triggered. The flag is the request itself, so a job that is shutting down lost its trigger for good and a missing schedule controller threw a null pointer in the watch thread. The registry is validated first; the flag is removed only when the job is triggered. While the job is running the flag is now kept instead of being dropped, which keeps the request visible for the stacked trigger noted in the existing TODO, and `triggerAllInstances()` already clears the whole trigger root so nothing stale accumulates. - `DistributeOnceElasticJobListenerTest` 6 → 10, `TriggerListenerManagerTest` 6 → 7, and 7 tests added for the closed registry center. The 500 ms pause in `waitForCacheClose()` is now documented, because Curator removes the watches of a closed cache asynchronously. `PersistentWatcher.reset()` in Curator 5.9.0 still retries without backoff under session churn; that is upstream and only the blast radius is reduced here. - `-T1C` is left in the workflow. Dropping it would slow the matrix on every pull request, and the module that actually failed was not an E2E suite. `test/e2e` is a single module on this branch, so the three `*Suite` classes mentioned in #2543 do not exist here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
