SEZ9 commented on PR #11602: URL: https://github.com/apache/seatunnel/pull/11602#issuecomment-5691922650
Thanks for the follow-up on this. I've gone through the current head (`a283df0d52`) against the earlier state at `bc6aefe094`, and the departed-worker change in `getCurrJobMetrics(..., failOnIncompleteResult)` looks right to me: when `nodeEngine.getClusterService().getMember(address) == null`, unconditionally skipping and logging the worker instead of throwing `FinalMetricsCollectionException` is the minimal fix for the terminal path — a member that has already left the cluster can never respond, so spending the `SubPlan` retry budget on it only delayed cleanup. Keeping `HazelcastInstanceNotActiveException`, `TimeoutException`, `InterruptedException` and the generic `Exception` branch under `failOnIncompleteResult` is also the correct split, since those are genuinely transient. The `JobMasterTest` fix (stubbed `EngineConfig` returning `Constant.DEFAULT_METRICS_FETCH_TIMEOUT_MS` instead of `null`) closes the NPEs from the previous round as well. On the remaining blocker from the last review — the `SplitClusterFaultToleranceIT.testStreamJobCancelResolvesWhenWorkerCrashesBeforeCancelAck` `expected: <CANCELED> but was: <FAILED>` mismatch — the analysis in the thread shows it reproduces on plain `dev` (the `memberRemoved` → `failedTaskOnMemberRemoved` → `makeTasksFailed` race on `CANCELING` vertices), with a separate fix tracked in #12311. I agree that is not this PR's bug and should not block it. Two concrete asks before I do the final pass: 1. Once #12311 lands on `dev`, please rebase this branch so we can get a clean CI run on this exact change set without the unrelated flake muddying the result. 2. Please confirm that the realtime (best-effort) path in `CoordinatorService.getRunningJobMetrics()` takes the same skip-and-log behaviour for departed members, so the two modes only differ in how they treat transient failures, not in how they treat members that have left. With those in place I expect to move this to approve. <!-- streview-comment:1091 --> -- 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]
