SEZ9 commented on PR #12265: URL: https://github.com/apache/seatunnel/pull/12265#issuecomment-5923706187
Thanks for keeping the PR synced with `dev` and for the precise description throughout. My understanding of the current state at head `8f5e089dd2`: the change from `ebdf7cbf0` to `9197c3bc5` was a single `dev` merge touching none of the files this PR owns, and the two `dev` merges since `9197c3bc58` only brought in upstream content in the few PR-owned files that differ. The PR's own hunks are unchanged, so the earlier source-level conclusions still hold. The design itself reads well to me: `executorService` for admission only, a cached direct-handoff `lifecycleExecutor` for `JobMaster.run()`, restore fan-out and cancel/stop/savepoint/history work, a dedicated single-thread executor for the pending-job scheduler, all three torn down and recreated together across step-down/reactivation, removal from the running-master map by instance identity, and the parallel `job_lifecycle_thread_pool_*` metric family. The `core=max=1` example (a blocked admission thread no longer blocks savepoint/cancel of a running job) is exactly the user-facing behavior I want from this slice. Two small asks before merging: 1. Could you share the CI status for `8f5e089dd2`? If anything is red, a link to the failing job and a note on whether it's related to this change would help. 2. Please confirm the incompatible-changes entries for this PR are still accurate after the merges — in particular that the new `job_lifecycle_thread_pool_*` metrics and the admission-only behavior of `max-thread-num` are documented there and in the user-facing config docs. Once those are covered I'm happy to approve and merge. <!-- streview-comment:1442 --> -- 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]
