DanielLeens opened a new pull request, #12133: URL: https://github.com/apache/seatunnel/pull/12133
## What this closes Issue [apache/seatunnel#10840](https://github.com/apache/seatunnel/issues/10840) described a Zeta engine master-node cold-start deadlock: with `telemetry.metric.enabled=true`, `SeaTunnelServerStarter#initTelemetryInstance()` registers the Prometheus telemetry collectors synchronously right after a node joins the Hazelcast cluster, before that node's `CoordinatorService` has any chance to finish its own asynchronous activation. Before the fix, `JobMetricExports#collect()` and `JobThreadPoolStatusExports#collect()` called the blocking `SeaTunnelServer#getCoordinatorService()` unconditionally whenever `isMaster()==true`, which retries for up to 1.5s (3 x 500ms) while the coordinator is still initializing - each retry blocking one Hazelcast operation thread. Under a full cluster force-restart (every node restarting simultaneously, e.g. a rolling upgrade or a datacenter power cycle), enough concurrently-blocked scrape calls could pile onto the small Hazelcast operation-thread pool that `initCoordinatorService()` could never obtain a thread to finish its own IMap work, permanently deadlocking the cluster. This was fixed by merged PR [apache/seatunnel#10841](https://github.com/apache/seatunnel/pull/10841), which added a non-blocking `isCoordinatorReady()` guard (`SeaTunnelServer#isCoordinatorActive()` -> `AbstractCollector#isCoordinatorReady()`) so both collectors return empty immediately instead of blocking while the coordinator is not yet active. **The gap:** the merged unit test for the guard itself, `TelemetryCollectorCoordinatorGuardTest`, proves the guard logic in isolation against mocked `SeaTunnelServer`/`CoordinatorService` objects, one collector invocation at a time. Nothing previously started a real multi-node cluster with telemetry enabled and proved that a genuinely concurrent full-cluster restart does not deadlock end-to-end through the actual startup sequence. ## What this PR adds `TelemetryStartupDeadlockIT` (new file, `seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base`), which: 1. Starts 3 real `HazelcastInstanceImpl` nodes fully concurrently (in-process, via `SeaTunnelServerStarter`, following this module's existing `ClusterIT`/`ClusterFailureNoRestoreIT` in-process harness convention rather than the module's Docker/Testcontainers convention - see the class Javadoc for why) with `telemetry.metric.enabled=true`, reproducing the "full cluster force-restart" scenario from the issue. 2. Runs a concurrent metrics-scrape hammer against every node throughout the startup race, directly exercising `JobMetricExports`/`JobThreadPoolStatusExports#collect()` - the exact code path the bug lived in. 3. Asserts the cluster converges and a real streaming job reaches `RUNNING` within a bounded, finite timeout (the pre-fix deadlock is described as permanent, so any finite timeout is sufficient to catch a regression here). 4. Asserts `collect()` never throws and stays within a generous latency backstop throughout the race. 5. Asserts the telemetry/metrics path itself reports the running job (`job_count{type="running"}`), matching the assertion style already used by the sibling `MasterWorkerClusterSeaTunnelWithTelemetryIT#testGetMetrics`. 6. Asserts non-master nodes correctly report no job metrics, extending `TelemetryCollectorCoordinatorGuardTest`'s mock-based coverage of the same guard to a real multi-node cluster. ## Regression-verification methodology (and an honest note on its limits) Per this initiative's standing requirement to confirm a new regression test would actually have caught the bug it targets: `JobMetricExports#collect()` was temporarily reverted to the pre-fix pattern (unconditional `isMaster()` check calling the blocking `getCoordinatorService()`), `seatunnel-engine-server` was rebuilt, and this test was re-run - it failed, confirming it does catch this class of regression. The fix was then restored and this test was re-run three times, passing every time. That investigation also surfaced a separate, narrower race worth disclosing: with no prior cluster to join, the 3 fully-concurrent nodes can each briefly form their own singleton Hazelcast cluster - and briefly activate their own `CoordinatorService` as that singleton's sole master - before merging into the real 3-member cluster, at which point some step down. A scrape can land in the narrow instant between `isCoordinatorReady()` reading true and `getReadyCoordinatorService()` re-checking it, right as that node steps down, paying one legitimate ~500-700ms retry even with the fix fully intact. This is a separate, bounded, at-most-once-per-node TOCTOU gap that PR #10841 does not claim to close, not the regression it fixed - and because this scrape hammer calls `collect()` directly from test-owned threads rather than through Hazelcast's own operation-thread pool (the way a real Prometheus scrape or the `/hazelcast/rest/instance/metrics` handler would), it cannot reproduce genuine ope ration-thread-pool exhaustion. The class Javadoc documents this in full; in short, the scrape-latency assertion is a generous secondary backstop, and this test's primary, reliable regression signal is the Awaitility timeouts on cluster convergence and job scheduling, since the bug report describes the real deadlock as permanent. ## Test plan - [x] `./mvnw spotless:apply -pl seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base` - no formatting changes needed. - [x] `./mvnw install -pl seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base -am -DskipTests` - full reactor build succeeds; confirmed the new test's `.class` file exists on disk with a fresh timestamp (compilation, not just a claimed "BUILD SUCCESS"). - [x] `./mvnw verify -pl seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base -DskipUT -DskipIT=false -Dit.test=TelemetryStartupDeadlockIT` - passes reliably (3 consecutive runs, ~21-30s each). - [x] Confirmed the test fails against an artificially-reverted pre-fix build of `seatunnel-engine-server` (temporary local edit, not part of this PR), then passes again after restoring the real fix - repeated 3x for the real fix. - [ ] GitHub Actions CI on this PR (pending). 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
