DanielLeens commented on PR #12446: URL: https://github.com/apache/seatunnel/pull/12446#issuecomment-6538142026
Addressed both blocking items from the 2026-10-22 review and synced with `dev`. New head: `d8f644bf6e7b5fc6b89da821a76f9cf9bbec0fd0` (commit [d8f644b](https://github.com/apache/seatunnel/pull/12446/commits/d8f644bf6e7b5fc6b89da821a76f9cf9bbec0fd0) on top of merge `fb853fb` of `origin/dev` `3ac2d1cec6a`, which brings in #12756 and #12848; the merge had no conflicts). **Issue 1 - timeout diagnostics have no host-side deadline** (`AbstractTestFlinkContainer.java`) - Each thread dump request now runs on its own daemon thread and is waited for at most `THREAD_DUMP_TIMEOUT_SECONDS` = 60 s on the host (`AbstractTestFlinkContainer.java:225` constant with the rationale, `:302-332` `dumpJvmThreads`). A request that misses the deadline is logged and abandoned, `dump.cancel(true)` + `shutdownNow()` run in `finally` (`:329-330`), and the next role is still attempted (`:251-252`), so a stalled Docker exec can no longer block past the job deadline or hide the second dump. Timeouts and dump failures are only logged; the original `IllegalStateException` for the job timeout is always thrown. Worst-case host-side bound is therefore 30 min + 2 x 60 s. Container teardown remains the cleanup boundary for the remote processes; the Javadoc says so explicitly. - The daemon-executor construction is shared by the job wait and the dump requests via `newDaemonExecutor` (`:338`). **Issue 2 - no deterministic coverage of the timeout/diagnostic path** (`AbstractTestFlinkContainerTest.java`) - The job and dump deadlines are exposed as package-private getters `getJobTimeout()` / `getThreadDumpTimeout()` (`AbstractTestFlinkContainer.java:285-295`); production values are unchanged (30 min / 60 s), the existing `TestFlinkContainer` in the test overrides them to 200 ms and stubs the submission (`AbstractTestFlinkContainerTest.java:243-288`). - `shouldFailAfterJobDeadlineAndBoundEachThreadDumpRequest` (`:144`): job never returns, JobManager dump request blocks forever, TaskManager dump request throws. Asserts under a 10 s preemptive budget that `executeJob` throws `IllegalStateException` caused by `TimeoutException`, and that both containers received the `kill -QUIT` exec. It fails if the job deadline is removed, if the stalled dump is not bounded on the host (the call would exceed the budget), if the second dump is skipped after the first stalled, or if the dump failure replaces the timeout exception. - `shouldReturnJobResultWithoutThreadDumpsWhenJobFinishesInTime` (`:188`): same `ExecResult` instance is returned and no dump exec is issued. - `shouldRethrowJobFailureUnchanged` (`:208`): an `IOException` from the submission is rethrown as the same instance (guards the `ExecutionException` unwrapping), no dump exec is issued. Local checks were limited to `./mvnw spotless:apply` / `spotless:check` on `seatunnel-e2e/seatunnel-e2e-common` (both clean); compile and tests are left to CI per the validation policy. Fork Build run for the new head: https://github.com/DanielLeens/seatunnel/actions/runs/41162470372 (queued at the time of writing). -- 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]
