SEZ9 commented on PR #11495: URL: https://github.com/apache/seatunnel/pull/11495#issuecomment-6357400513
Thanks for the dev sync, @zhangshenghang. Since `66f46aa37` is only a merge of `dev` and the PR-owned hunks are identical to what I reviewed at `68f3b7c5b7`, my previous verdict stands and I'm not re-opening the review. For completeness, here is where the earlier points are on this head: - **F1 (worker-side stale contexts)** – resolved: `clearPipelineMetricsForRestore` cleans the task-group contexts on the surviving workers before removing the pipeline's snapshots. - **F2 (race with the periodic metrics report)** – accepted as a documented best-effort fix. An attempt-fenced store can be a follow-up. - **F3 (semantics change for streaming jobs)** – resolved via the incompatible-changes note (en/zh) and the REST job-lifecycle docs. - **F4–F7 (test coverage, test scaffolding, fully-qualified names, helper consolidation)** – unchanged and all non-blocking. If you'd like to pick up any of them in this PR, great; otherwise a short note that they're deferred is enough, and F7 may already be moot given the current two-step shape of `clearPipelineMetricsForRestore`, so feel free to just confirm. Good to hear `StateTransitionCleanupTest` passes 13/13 on the merged tree. Nothing further from my side. <!-- streview-comment:1975 --> -- 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]
