DanielLeens opened a new pull request, #12200:
URL: https://github.com/apache/seatunnel/pull/12200
### Purpose of this pull request
Adds an E2E regression test for the Zeta engine startup
`NullPointerException` described in #10570 and fixed by #10610 ("Prevent NPE by
lazy initializing overviewMap"), as part of the ongoing "extreme-case E2E"
initiative that has already added tests such as `TelemetryStartupDeadlockIT`
(#12133) for similar startup-ordering races.
### The regression (#10570) and its fix (#10610)
Before the fix, `CheckpointMonitorService`'s constructor eagerly resolved
its backing Hazelcast `IMap`:
```java
private final IMap<Long, CheckpointOverview> overviewMap;
public CheckpointMonitorService(NodeEngine nodeEngine, int maxHistorySize) {
this.overviewMap =
nodeEngine.getHazelcastInstance().getMap(Constant.IMAP_CHECKPOINT_MONITOR);
this.maxHistorySize = maxHistorySize;
}
```
That constructor runs on every master-role node's cold start, from
`SeaTunnelServer#startMaster()`, called by `SeaTunnelServer#init(NodeEngine,
Properties)` - a Hazelcast `ManagedService` callback invoked from
`ServiceManagerImpl#initServices()`, which runs inside
`NodeEngineImpl#start()`, inside `Node#start()`, **inside
`HazelcastInstanceImpl`'s own constructor** - before the instance has finished
constructing itself and before its partition/operation infrastructure is ready.
The reporter's config additionally set a Hazelcast map-store with
`initial-mode: EAGER` on the `engine*` map family, which makes `IMap` proxy
creation synchronously call `waitUntilLoaded()` -> `invokeOnPartition()` ->
`new PartitionInvocation()` -> `new Invocation()`; that constructor NPEs
because it depends on partition/operation state this early bootstrap window has
not published yet.
Because `init()` is one sequential method and SeaTunnel's own Jetty REST
server starts strictly *after* `startMaster()` in that same method (today
`SeaTunnelServer.java:187-192`), the uncaught NPE aborted `init()` before the
Jetty-start lines ever ran - the precise mechanism behind the issue title
"blocking Jetty and other services initialization": the NPE never touches
Jetty's own code, it simply prevents every line of `init()` written after
`startMaster()` from executing at all.
PR #10610 moved the `getMap()` call out of the constructor into a
lazily-invoked, double-checked-locked accessor (`getOverviewMap()`), so the
constructor only stores references and the actual Hazelcast map access happens
on first genuine use, well after the node has joined the cluster. That exact
idiom is still present today, verbatim in shape, in
`CheckpointMonitorService.java:54-72` - though the field's static type has
since changed from a raw `IMap<Long, CheckpointOverview>` to a
`CheckpointOverviewStateStore` obtained through a `HazelcastEngineStateStores`
abstraction that independently applies the same lazy double-checked-locking
discipline to its own four backing maps. There is no feature flag gating any of
this: `CheckpointMonitorService` is constructed unconditionally by
`startMaster()` for every node whose `cluster-role` includes `MASTER` (the
engine's default `MASTER_AND_WORKER` role), so a plain multi-node cluster start
already exercises the exact call chain the bug liv
ed in.
### What the new test does
`CheckpointMonitorStartupRaceIT` (in `connector-seatunnel-e2e-base`,
mirroring `TelemetryStartupDeadlockIT`'s in-process `HazelcastInstanceImpl`
harness):
1. Starts 3 nodes genuinely concurrently via
`SeaTunnelServerStarter#createHazelcastInstance`, each with its own
pre-allocated free HTTP port (REST enabled, unlike `TelemetryStartupDeadlockIT`
which disables it - this test's whole point is proving Jetty is reachable).
2. Immediately after every node starts, reflectively asserts
`CheckpointMonitorService#overviewMap` is still `null` on every node - a
precise, source-grounded proof that the constructor never touched the
checkpoint state store during the risky `Node#start()` window. A regression
that reintroduced eager initialization would fail this assertion
deterministically, independent of whether the exact NPE reproduces in this
environment (reproducing the reporter's literal trigger - a working Hazelcast
map-store with `initial-mode: EAGER` - is not practical from this test tree).
3. Waits for the cluster to converge, then asserts every node's `/overview`
REST endpoint answers within a bounded timeout - the literal "Jetty ...
initialization" symptom from the issue.
4. Submits `stream_fakesource_to_console.conf` (a streaming job with
`checkpoint.interval = 5000` already set) and waits for it to reach `RUNNING`.
5. Waits for a real checkpoint to complete via the
`/jobs/checkpoints/{jobId}` REST endpoint, exercising
`CheckpointMonitorService#onCheckpointTriggered`/`#onCheckpointCompleted` ->
`#getOverviewMap()`'s lazy accessor for real.
6. Queries the checkpoint overview on every node (matching
`RestApiIT#testCheckpointOverviewAndHistoryApi`'s cross-node convention, since
the backing state store is a distributed Hazelcast map), then reflectively
re-checks that `overviewMap` is now populated everywhere - completing the
before/after proof.
7. A log4j2 appender attached to the root logger (the engine routes
Hazelcast's `ILogger` through log4j2) asserts no `NullPointerException` was
logged anywhere during the whole sequence - Hazelcast's own internals logged
the original NPE through `ProxyService`/`ServiceManagerImpl`, not through any
SeaTunnel-owned logger name, so a root-level capture is used rather than the
named-logger technique `TaskDeploymentPrePublicationClassLoaderLeakIT` uses.
### Why this would have caught the original bug
Any regression that reintroduces eager state-store access in
`CheckpointMonitorService`'s constructor (or that adds an equally early
Hazelcast map access anywhere else in `startMaster()`/`init()`) would make node
startup itself throw (failing step 1's `future.get()`), or would fail step 2's
reflective null-check outright, or would prevent Jetty from ever starting per
step 3 - not merely run slower. Steps 4-6 additionally prove the deferred
access path is genuinely functional, not just "never crashed".
### Verification
No local Maven build or test execution was performed for this PR (Apache
SeaTunnel local-verification policy for this session): `./mvnw spotless:apply
-pl seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base -nsu
-Dmaven.gitcommitid.skip=true` ran clean (twice, confirming the file is
syntactically valid and already formatted). GitHub CI on this PR's head is this
code's first execution.
### Does this PR introduce any user-facing change?
No, test-only.
### How was this patch tested?
New E2E test added; see above. No other files changed.
🤖 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]
