SEZ9 commented on PR #10874: URL: https://github.com/apache/seatunnel/pull/10874#issuecomment-5611806470
@DanielLeens thanks for re-checking the raw review body and pasting the remainder inline — that clears up the rendering/collapse confusion, no apology needed. Per finding: - **F1/F8, F4/F6, F5/F7** — What you describe (the 300s deadline in `waitUntilTableQueueDrained` throwing `IOException` on timeout with the table left in `pendingCloseTableIds`, the `InterruptedException` branch restoring the interrupt flag, the early return in `handleCloseTableEvent` for already-closed tables, the `failurePolicy.continueOtherTables()` check before throwing, and the accompanying tests) matches what I was looking for. I'll re-read the diff to confirm each of these before marking them resolved. - **F2** — Same read as you: still open, since `closeTable()` calls `sinkWriter.close()` without an interposed `prepareCommit`/flush, so uncommitted 2PC data for that table is dropped. Not blocking on a code fix in this PR, but please add it to the PR description as a known, tracked limitation before merge, and let me know once that's done. - **F3** — Your explanation is cut off on my side right after "`queue.poll()` ... is outside". Could you repost the rest of the F3 paragraph (the synchronization trace and why deferral is acceptable)? I'd like the full reasoning before treating it as deferred rather than open. Outstanding from my side: (1) the F2 limitation in the PR description, and (2) the rest of your F3 write-up. <!-- streview-comment:934 --> -- 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]
