DanielLeens commented on PR #12311: URL: https://github.com/apache/seatunnel/pull/12311#issuecomment-6624354746
Thanks @SEZ9. Answers against the unchanged head `65ef1bde182`, point by point. **F1, master-failover reschedule.** Here is the trace: - `JobMaster#getJobStatus` (`JobMaster.java:1099-1101`) delegates to `PhysicalPlan#getJobStatus` (`PhysicalPlan.java:355-357`), which reads `runningJobStateIMap.get(jobId)`. That is the persisted job status, not a pipeline status. - `SubPlan#restorePipelineState` (`SubPlan.java:565-581`) restores the vertex states and then calls `updatePipelineState(PipelineStatus.CANCELING)` for a pipeline that was not yet RUNNING. It changes only the pipeline state. It never writes the job status. - So when the new master cancels tasks after a failover, the job status is whatever was persisted before the switch, normally RUNNING. `resolveLostMemberState` (`CoordinatorService.java:2234-2249`) returns CANCELED only for `JobStatus.CANCELING`, so those vertices resolve to FAILED, as they did before this PR. The job status would only be CANCELING if the user had already cancelled the job before the failover, and in that case CANCELED is the correct outcome. Test coverage: `CoordinatorServiceLostMemberResolutionTest#engineInitiatedCancelStillResolvesToFailed` (`:81-92`) asserts FAILED for a CANCELING vertex under every job status except CANCELING, and under `null`. To be precise, no IT combines a master switch with a worker loss and asserts the end state. I'm not adding one in this PR, for two reasons. The resolver's only new input is the job status, and the trace above shows restore never sets it. Also, what the engine does after a vertex FAILS (restore and reschedule, or ending FAILED) is the existing path, which this PR does not change. **F2, CANCELING plus RUNNING sibling on the lost member.** `testStreamJobCancelResolvesWhenWorkerCrashesBeforeCancelAck` runs `pending_jobs_streaming_lifecycle.conf` (FakeSource `parallelism = 2`) on a single worker, so the lost member hosts every task vertex. It shuts the worker down as soon as the first vertex is seen CANCELING (`SplitClusterFaultToleranceIT.java:443-454`). Because `SubPlan` cancels sequentially, the second vertex is normally still RUNNING at that moment. The IT does not assert that, though, because it depends on timing. The deterministic sibling case is pinned by `#notYetCancelledVerticesOfUserCancelledJobResolveToCanceled` (`:63-73`). I'll add a Javadoc note to the IT that states exactly this split. **F3, WARN identifiers.** These are already in the message. The WARN at `CoordinatorService.java:2171-2181` prints the `TaskGroupLocation`, and its `toString` (`TaskGroupLocation.java:73-82`) is `jobId=..., pipelineId=..., taskGroupId=...`. The message also includes the lost member address, the job status, the previous vertex state and the resolved state. **F4, restore Javadoc.** Already on this head: `CoordinatorService.java:2221-2227` says a pipeline that went FAILING without a failed task still ends FAILED, because `SubPlan#getPipelineEndState` checks the FAILING pipeline status. **F5/F6, naming.** The public `failedTaskOnMemberRemoved` Javadoc (`CoordinatorService.java:2111-2118`) already says that despite its name it can resolve a vertex to CANCELED. The only IT Javadoc that names it (`SplitClusterFaultToleranceIT.java:358-370`) describes both the CANCELED branch and the FAILED branch. I'm keeping the method name to avoid churn in a public entry point. **F7, guarantee wording.** Fair. Line 367 says the job "is cancelled by the user", which is the intent, not the condition the code checks. In the follow-up commit I'll reword it to say the CANCELED outcome holds because the job status is CANCELING when the member-removed event is processed, and that the same condition covers the RUNNING siblings. Follow-up commit (Javadoc only, no production change): the F7 rewording and the F2 sibling-shape note, both in the `testStreamJobCancelResolvesWhenWorkerCrashesBeforeCancelAck` Javadoc. -- 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]
