davidzollo commented on PR #12031:
URL: https://github.com/apache/seatunnel/pull/12031#issuecomment-5548995351
### CI analysis for the current head (`4af856f`) — root cause found, it is
in the test, not the engine
`engine-v2-it` now fails on **both JDK 8 and JDK 11** (previously JDK 8
only), so this is deterministic:
```
CheckpointCoordinatorFailoverIT.testBatchJobCompletesAfterMasterFailoverDuringCloseHandshake
org.awaitility.core.ConditionTimeoutException:
Waiting for a partial close handshake (readyToClose=0, total=4) ==>
expected: <true> but was: <false> within 30 seconds
at CheckpointCoordinatorFailoverIT...(...:513) / lambda$...(...:524)
```
The failure is confined to this PR's own new test:
`SplitClusterPendingJobLifecycleFailoverIT` passes in this run, and
`CheckpointCoordinatorFailoverIT` passes on branches without this test.
#### Root cause
The test waits for a **steady-state** partial handshake:
```java
readyCount = getReadyToCloseCount(master, jobId, firstPipelineId)
+ getReadyToCloseCount(master, jobId, secondPipelineId);
Assertions.assertTrue(readyCount > 0 && readyCount <
CLOSE_HANDSHAKE_STARTING_SUBTASKS /* 4 */, ...);
```
`getReadyToCloseCount` reads `IMAP_RUNNING_JOB_STATE[readyToCloseImapKey]`,
which is **per pipeline** and only transient:
- `CheckpointCoordinator.readyToClose`
(`seatunnel-engine-server/.../checkpoint/CheckpointCoordinator.java:552-558`)
persists each report and, as soon as **that pipeline's** starting subtasks are
all ready, fires `tryTriggerPendingCheckpoint(COMPLETED_POINT_TYPE)`.
- When that completed point finishes, the coordinator clears the state and
deletes the key (`CheckpointCoordinator.java:1192-1201`):
`readyToCloseStartingTask.clear()` and, for any
non-`CHECKPOINT_COORDINATOR_RESET` close reason,
`runningJobStateIMap.remove(readyToCloseImapKey)`.
Because the config splits the shared-sink union into two independent
pipelines (one per `FakeSource`), each pipeline's key rises `0 -> 1 -> 2` and
is then **deleted**, independently of the other. The aggregate the test sums
therefore never settles anywhere in `(0, 4)`: it is 0 almost always, briefly 2,
then 0 again. The `total=4` invariant the assertion is written against is not
reachable as a stable state at all.
The run confirms the timing — the completed point for the fast pipeline
fires ~1.4 s after the job starts running, long before the polling loop can
catch a partial value:
```
16:32:21,224 PhysicalPlan - Job
testBatchJobCompletesAfterMasterFailoverDuringCloseHandshake
(1148294491912798209) ...
16:32:22,641 CheckpointCoordinator - skip schedule trigger checkpoint
because checkpoint type is COMPLETED_POINT_TYPE
```
A 20 ms poll cannot reliably observe a window that narrow, and once the key
is removed the count is 0 for the rest of the 30 s — exactly what the failure
message reports.
So the engine behaviour is correct here; the test's assumption about where a
partial handshake is observable is what is wrong. This is a pure test-design
defect and the fix belongs entirely in the test file.
#### Why I have not pushed a redesign yet
Making this deterministic is not a one-line change. The scenario needs a
genuine partial handshake that persists long enough to land a failover on,
which means either:
- driving a **single** pipeline whose starting subtasks report readiness at
clearly separated times (with `FakeSource` splits assigned round-robin, the
parallel subtasks of one source finish near-simultaneously, so this needs a
deliberately asymmetric source setup), or
- instrumenting a different, non-transient observation point instead of the
`readyToClose` IMap key.
I would rather agree the approach than guess: this repository's policy is
CI-only verification for these E2E paths, so an unvalidated redesign costs a
full CI cycle per attempt. I am explicitly **not** widening the 30 s timeout or
relaxing the assertion — the window being waited for does not exist in steady
state, so loosening the bound would just make the test pass without testing
anything.
Marking this as blocking pending agreement on which of the two directions
above to take.
--
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]
