SEPURI-SAI-KRISHNA opened a new pull request, #18541:
URL: https://github.com/apache/dolphinscheduler/pull/18541
<!--Thanks very much for contributing to Apache DolphinScheduler, we are
happy that you want to help us improve DolphinScheduler! -->
## Was this PR generated or assisted by AI?
YES. The missing field resets were found and the fix and unit tests were
drafted with AI
assistance (Claude Code); the behaviour, the comparison against the sibling
factories and
the final code were reviewed and verified by me.
## Purpose of the pull request
Closes #18540.
`FailedRecoverTaskInstanceFactory` recreates a `FAILURE` / `KILL` task
instance when a
workflow is recovered with "Recover failed tasks". It clones the old
instance, clears part
of the runtime state and inserts it as a new row — but it never resets
`retryTimes`,
`startTime`, `endTime`, `pid` or `alertFlag`, all of which
`cloneTaskInstance` copies.
Because a task that failed after exhausting its retries has `retryTimes ==
maxRetryTimes`,
and `TaskExecution#isTaskInstanceCanRetry()` is `retryTimes <
maxRetryTimes`, the
recreated instance starts with a fully consumed retry budget and is never
retried on the
recovery run, however many retries the task definition asks for.
The two sibling factories already do this correctly —
`FirstRunTaskInstanceFactory` sets
`retryTimes(0)`, `startTime(null)`, `endTime(null)`, `alertFlag(NO)`, and
`RetryTaskInstanceFactory` sets `startTime(null)`, `endTime(null)`, `pid(0)`
while
deliberately incrementing `retryTimes`. This change brings the recover
factory in line
with them.
The stale `startTime` / `endTime` are user visible too: until the task
actually starts, the
new instance is shown with the previous attempt's timestamps, and
`TaskInstanceServiceImpl#queryTaskListPaging` derives its duration from them.
## Brief change log
- `FailedRecoverTaskInstanceFactory#createTaskInstance`: reset `retryTimes`
to `0` and
clear `startTime`, `endTime`, `pid` and `alertFlag` on the recreated task
instance.
- Added `FailedRecoverTaskInstanceFactoryTest`.
## Verify this pull request
This change added tests and can be verified as follows:
- Added `FailedRecoverTaskInstanceFactoryTest` with three cases: the retry
budget is
restored, the runtime state of the failed attempt is cleared, and the
origin instance is
still marked invalid while the new one is inserted.
```bash
./mvnw -pl dolphinscheduler-master -am clean test \
-Dtest=FailedRecoverTaskInstanceFactoryTest \
-Dsurefire.failIfNoSpecifiedTests=false
```
Verified locally:
- The new tests fail on `dev` and pass with this change. On `dev` the
recreated instance
reports `retryTimes = 3` where `0` is expected, and keeps the `startTime`
of the failed
attempt where `null` is expected.
- The full `dolphinscheduler-master` suite passes: 98 tests, 0 failures, 0
errors.
- `./mvnw -pl dolphinscheduler-master spotless:check` passes.
## Pull Request Notice
[Pull Request
Notice](https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)
If your pull request contains incompatible change, you should also add it to
`docs/docs/en/guide/upgrade/incompatible.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]
