Vivek1106-04 commented on issue #12339: URL: https://github.com/apache/seatunnel/issues/12339#issuecomment-5755344454
Thanks — that settles the option question for now, so let me answer the three follow-ups and record the dev@ discussion here. **1. The `thenAccept` fix — yes, it goes in this PR.** Confirmed against `CheckpointCoordinator`: `startTriggerPendingCheckpoint` registers with `thenAccept`, so when the pending future is already complete the body runs inline on the calling thread, and that body blocks on `CompletableFuture.allOf(completableFutureArray).get()` for the whole barrier ACK round-trip. The caller is `tryTriggerPendingCheckpoint`, which holds `synchronized (lock)`, so the block is taken while holding the coordinator lock. This is pre-existing — today it stalls the coordinator's own timer thread. What changes with a shared pool is the blast radius: the same stall would occupy a member-wide dispatch thread. That is exactly the coupling this PR is meant to remove, so closing it here rather than separately. Fix is `thenAcceptAsync(..., executorService)`, with a test that completes the pending future before registration and asserts the body does not run on the caller's thread. **2. The option — holding until the dev list converges.** Agreed on the shape if it lands: one `checkpoint.scheduler-dispatch-thread-num`, default `0` meaning auto, so existing deployments are unchanged. Not adding it speculatively. **3. Metrics — follow-up issue, not this PR.** Dispatch queue depth, active dispatch threads, and scheduling delay (scheduled fire time vs actual, per pipeline). Scheduling delay is the one that would justify or kill the option, so it should land before the option does, not after. **4. dev@ `[DISCUSS]` thread** https://lists.apache.org/thread/nxlg8d9hxy1jwj9pdhpyhc32ornnqrgj Zhiwei Niu raised four implementation questions there — master switchover and failover, schema evolution for CDC, single-pipeline failure inside a multi-pipeline job, and how `checkpointMinPause` and the other checkpoint options are handled. Answers are on the thread. The short version is that the scheduler is only a delay primitive: it holds no job state, leases are keyed by `(jobId, pipelineId)` so cancellation never crosses pipelines, and every policy — interval, min-pause, checkpoint and schema-change timeouts, the pending- checkpoint retry — stays in `CheckpointCoordinator`, unchanged. One item from that thread lands here as work: I am running `MysqlCDCWithSchemaChangeIT` against the branch to back the schema-evolution answer with a result rather than an assertion, and will post it. -- 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]
