DanielLeens opened a new pull request, #12185:
URL: https://github.com/apache/seatunnel/pull/12185
### Purpose of this pull request
Closes a coverage gap in #10834 ("[Bug][Zeta] Job stuck permanently after
master failover... affects BATCH / bounded source / job shutdown phase") left
open by the merged fix #10836.
#10836 fixed the root cause
(`CheckpointCoordinator#readyToCloseStartingTask` not surviving a master
failover) and added `CheckpointCoordinatorFailoverIT` tests for it, but those
tests only cover:
- `testBatchJobCompletesAfterMasterFailover`: a BATCH job, with the master
killed while rows are still flowing — well before any source has reported
ready-to-close.
- `testStreamJobContinuesAfterMasterFailover`: an UNBOUNDED streaming source
that can never finish and is cancelled by hand at the end, so it only proves
checkpoint-id continuity, never actual completion.
Neither test proves the scenario #10834 itself names: a STREAMING job whose
source is bounded reaching `FINISHED` after a master failover during its
shutdown phase. This PR adds
`testBoundedStreamJobCompletesAfterMasterFailover`, which does.
### How was this patch tested?
**New E2E test**
(`CheckpointCoordinatorFailoverIT#testBoundedStreamJobCompletesAfterMasterFailover`):
- Extends the existing shared `CheckpointableSequenceSource` E2E test source
(already used by the savepoint-restore and checkpoint-restore-with-stop suites)
with an additive, opt-in `exhausted_snapshot_delay_ms` option (default `0`, a
no-op). Once the reader has drained every split it will ever receive, this
holds `snapshotState()`'s return back for the configured duration, keeping the
final checkpoint's acknowledgement pending so the test can deterministically
observe the shutdown-phase window instead of racing it.
- Runs a `STREAMING` job over a bounded `CheckpointableSequenceSource`
(`start_offset`/`end_offset` make it finite). The reader drains its range and
signals no-more-element, which drives the engine's real completion path
(`SourceNoMoreElementOperation` → `LastCheckpointNotifyOperation` →
`readyToClose` → persisted under
`checkpoint_state_<jobId>_<pipelineId>_ready_to_close`).
- The test polls that persisted IMap entry directly (via the coordinator's
own `getReadyToCloseImapKey()`) until it is set while the job is still
`RUNNING`, then kills the master — mirroring the master-kill technique already
used by this file's other tests.
- The first member runs as a `MASTER`-only node
(`EngineConfig.ClusterRole.MASTER`) so every task of the job lives on the
surviving member, making the failover a pure master switch
(`restoreCoordinator(alreadyStarted=true)`) rather than a pipeline restart.
This is asserted directly: `restoredPipeline.getPipelineRestoreNum() == 0`
guards against the test passing for the wrong reason (a restarted enumerator
re-sending its notification would also reach `FINISHED`, without ever
exercising the fix).
- Asserts the job reaches `JobStatus.FINISHED` within a bounded wait and
that the sink holds exactly the expected row count.
**Verification evidence** (JDK 8, `-DskipUT=true -DskipIT=false
-Dit.test=CheckpointCoordinatorFailoverIT`, `./mvnw spotless:apply` run on both
changed modules before every commit):
- New test run together with all of this file's other tests
(`testBatchJobCompletesAfterMasterFailover`,
`testStreamJobContinuesAfterMasterFailover`,
`testBatchJobCompletesAfterMasterFailoverDuringCloseHandshake`): passed cleanly
across 5 separate full-suite runs on this branch (rebased onto current `dev`),
all 4 methods green in the majority of runs.
- The new `testBoundedStreamJobCompletesAfterMasterFailover` itself passed
in every one of those runs, plus additional isolated single-method runs
performed while diagnosing an unrelated timing question during development —
10+ consecutive passes total, ~47s each.
- `testBatchJobCompletesAfterMasterFailoverDuringCloseHandshake`
(pre-existing, from #12031, untouched by this PR) hit two different transient
failures during verification under heavy local back-to-back-build load (a
`NoSuchFileException` from a stale local resource copy on my end, and a `Can
not get coordinator service from an active master node` race); both were not
reproducible on a subsequent clean run and are unrelated to this PR's diff —
flagging for awareness in case they are worth a dedicated look, but out of this
PR's scope.
- Confirmed the `CheckpointableSequenceSource` change is purely additive:
the new option defaults to `0` (disabled), the reader's public constructor is
only invoked from within its own factory, and no other `.conf` fixture
(`savepoint-restore/`, `checkpoint-restore-with-stop/`) sets or is affected by
the new option.
### Does this PR introduce _any_ user-facing change?
No. Test-only change; no `src/main` files are touched.
### Check list
* [ ] If any new Jar binary package adding in your PR, please add License
Notice
* [ ] If necessary, please update the documentation
* [ ] If necessary, please update `incompatible-changes.md`
🤖 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]
