davidzollo commented on PR #12027:
URL: https://github.com/apache/seatunnel/pull/12027#issuecomment-5548995175
### CI analysis for the current head (`db88893`) — blocking, needs a
decision before this PR can go green
`engine-v2-it` fails on **both JDK 8 and JDK 11** with the same signature,
so this is deterministic, not flaky:
```
SplitClusterPendingJobLifecycleFailoverIT.testPendingJobNotDuplicatedAcrossRepeatedMasterFailover
org.awaitility.core.ConditionTimeoutException: ... expected: <FINISHED> but
was: <PENDING> within 3 minutes
at
SplitClusterPendingJobLifecycleFailoverIT.assertJobStatusWithTimeout(...:689)
at
SplitClusterPendingJobLifecycleFailoverIT.testPendingJobNotDuplicatedAcrossRepeatedMasterFailover(...:399)
```
The failure is confined to this PR's own new test:
`CheckpointCoordinatorFailoverIT` passes in this run, and
`SplitClusterPendingJobLifecycleFailoverIT` passes on other branches that do
not carry this test.
#### What the run actually shows
The pending-job scheduler is **healthy** for the whole 3-minute window — it
keeps retrying the contested job every 3s right up to the end of the test:
```
15:34:49,577 WARN ResourceRequestHandler - Apply resource not success for
job: 1148279132178677761,
required: 1 slots, applied: 0 slots, releasing slots: [],
remaining: 1 slots not assigned
```
So the epoch machinery this PR is guarding is not what breaks. The job never
gets its single slot because the worker never has one free. Tracing the holder
job (`1148279132178612225`) explains why:
1. `15:31:41` / `15:31:44` — after the 4 master flaps, the **holder job is
itself back in the pending queue** and failing its own pre-check:
`Pre resource application failed for job: 1148279132178612225, success:
0, failed: 4/4`.
The holder was deployed and RUNNING, but the successor coordinator has
lost its resource ownership and is now re-requesting all 4 slots — against
slots its own still-running tasks occupy.
2. `15:31:47.711` — `cancelJob()` takes effect at the coordinator (`state
process is stopped`), so `assertEventuallyCanceled` passes on job **status**.
3. `15:31:47.893` — 182 ms later the contested job's next pre-check still
reports `remaining: 1 slots not assigned`.
4. `15:31:48.92` — the test shuts the active master down (~1.2 s after the
cancel).
5. `15:34:50` (teardown, ~3 min later) — the holder's tasks are **still
alive on worker 5803**: `TaskExecutionService` warnings plus
`MultiTableWriterRunnable error when write row ...`.
So the holder reached CANCELED at the coordinator while its tasks kept
running on the worker, and because the coordinator no longer held any resource
record for it, nothing ever told the worker to stop them. All 4 static slots
(`slotNum=4`, `dynamicSlot=false`) stay leaked, and the contested job can never
be scheduled.
#### Why I am not pushing a fix yet
Two separate things are tangled here and they need different calls:
- **This is not the defect I1 guards.** The test asserts FINISHED as a proxy
for "dispatched exactly once". What it actually tripped over is
resource-ownership recovery: after repeated failover, an already-deployed job
can be re-queued as PENDING with its slot ownership lost, and cancelling it
then leaves orphaned tasks plus permanently leaked slots. That is a real
robustness gap worth its own issue, but treating it inside this regression-test
PR would take the diff deep into the failover/resource-manager path — well past
a minimal, reviewable change, and past what I can pin to a specific method and
line range with the evidence I have.
- **The test also over-trusts a proxy condition.**
`assertEventuallyCanceled` checks job status only; the test then assumes worker
slots are free and hands off the master 1.2 s later. Status and slot accounting
are not the same thing, so the test creates the race it then fails on.
I am deliberately **not** raising the 180s timeout: the scheduler retried
continuously for the full 3 minutes and the slot was never released, so a
longer timeout would hide the finding rather than fix anything.
#### Proposed direction (needs a maintainer call)
1. Confirm whether the orphaned-slot / still-running-tasks behaviour after
`cancel` + immediate master loss is a defect that should be tracked and fixed
separately in `seatunnel-engine-server`. If so I will open a dedicated issue
with this evidence rather than widen this PR.
2. Independently, restructure this test so the final handoff no longer
depends on that path — the scenario needs the contested job's resources to be
observably available before the last master shutdown, rather than inferring it
from the holder's job status.
Flagging this as blocking until (1) is decided, since the answer determines
whether the fix belongs in this PR or outside it.
--
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]
