Vivek1106-04 opened a new pull request, #12623:
URL: https://github.com/apache/seatunnel/pull/12623
### Purpose of this pull request
Fixes #12622.
`PhysicalVertex.forceStop()` moved a task that is not deployed to `CANCELED`
without completing its `taskFuture`. Such a task has its state process stopped,
so `updateTaskState(CANCELED)` -> `stateProcess()` returns at the `isRunning`
check before the `CANCELED` branch that completes the future. `SubPlan` counts
finished tasks off those futures, so the pipeline never saw the task end. In
the savepoint case from #12511 the pipeline then restarted with nothing to
deploy and stayed `RUNNING`, leaving the job in `DOING_SAVEPOINT`.
`forceStop()` now starts the state process first when it is not running, the
same way `SubPlan`'s `FAILING`/`CANCELING` branch calls
`task.startPhysicalVertex()` before `task.cancel()`. A task that is already
running is unaffected.
This also closes the window that #12511's `isNeedRestore()` re-check leaves
open: a stop decided after that check and before the restarted tasks are
deployed. The two changes are independent and touch different files.
### Does this PR introduce _any_ user-facing change?
No. A force-stopped task that was not deployed now reports `CANCELED` to its
pipeline instead of leaving it waiting. No config options or defaults change.
### How was this patch tested?
New test in `StateTransitionCleanupTest`:
- `testForceStopCompletesTheFutureOfATaskWhoseStateProcessIsStopped`: takes
a vertex from a fresh `PhysicalPlan` (state `CREATED`, state process stopped,
no slot), calls `initStateFuture()` and `forceStop()`, and expects the future
to complete with `CANCELED` and the task state to be `CANCELED`.
Red on `dev` (`af80a6704`), with the production change reverted:
`StateTransitionCleanupTest.testForceStopCompletesTheFutureOfATaskWhoseStateProcessIsStopped:99
>> Timeout`, and the log shows `state process is not start`.
Green on this branch, plus the other test classes that go through the stop
paths:
```
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 - in
...dag.physical.StateTransitionCleanupTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 1 - in
...checkpoint.SavePointTest (existing @Disabled)
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0 - in
...checkpoint.SavePointBusySourceTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 - in
...checkpoint.CheckpointErrorRestoreEndTest
Tests run: 37, Failures: 0, Errors: 0, Skipped: 2 - in
...CoordinatorServiceTest (existing skips)
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 - in
...CoordinatorServiceWithCancelPendingJobTest
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0 - in
...master.JobMasterTest
```
Local runs on JDK 17. `./mvnw spotless:apply` shows no changes and `./mvnw
-DskipTests verify` passes for the module.
### Check list
* [ ] If any new Jar binary package adding in your PR, please add License
Notice according
[New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/developer/new-license.md)
* [ ] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [ ] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
--
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]
