1996fanrui opened a new pull request, #29031: URL: https://github.com/apache/flink/pull/29031
## What is the purpose of the change `DefaultCheckpointPlanCalculator` uses `Execution.isFinished()`, which returns `state.isTerminal()`, to decide whether a task is finished. This treats *any* terminal task — `FINISHED`, `FAILED` or `CANCELED` — as finished. During a failover some tasks may be `FAILED`/`CANCELED`. The calculator then classifies them as finished and excludes them from `tasksToWaitFor`, producing a checkpoint plan that expects fewer acks than it should (in the extreme, an empty plan). `"terminal"` is not the same as `"finished"`. `PendingCheckpoint` currently rejects an empty `tasksToWaitFor`, so the worst case is masked today, but the classification is wrong at its source and relying on that downstream guard leaves the issue latent. ## Brief change log - `collectTaskRunningStatus`: classify explicitly — a non-terminal task is running; a terminal task must be genuinely `FINISHED` (`checkState`). - `calculateCheckpointPlan`: add `checkNoTasksFailedOrCanceled()` — abort the checkpoint with a graceful `CheckpointException` when a task is `FAILED`/`CANCELED` (a failover is in progress) instead of building a plan from a transient state. - Remove the misleading `Execution.isFinished()` (its only caller). The check is condition-based, not mode-based, so it is correct for both streaming and batch. `calculateCheckpointPlan` and all task state transitions run on the JobManager main thread, so the pre-check and plan computation are atomic — a failover lands either before it (caught) or after (queued), never in between. ## Verifying this change Added `DefaultCheckpointPlanCalculatorTest#testPlanAbortedWhenTaskFailedWhileOthersFinished`: builds a graph with one `FINISHED` and one `FAILED` task and asserts the plan is aborted with `NOT_ALL_REQUIRED_TASKS_RUNNING`. ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): **no** - The public API: **no** - The serializers: **no** - The runtime per-record code paths (performance sensitive): **no** - Anything that affects deployment or recovery: **yes** (checkpoint plan calculation) - The S3 file system connector: **no** ## Documentation - Does this pull request introduce a new feature: **no** -- 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]
