DanielLeens commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5793277429
Thanks both — I did the synced-head final pass on `7f69d7f69f6` you asked for, @Rangsh, and traced the actual code rather than going from the summary. - **F1/F3 (untimed `get()`)**: confirmed. `RequestFuture.get()` is documented as "No production caller currently invokes this method," and both call sites (`queryExecuteStatus` and the batch path) use the timed overload bound by `writDataTimeoutMilliseconds` or the shared `deadlineNanos`. Agreed this is fine as-is. - **F2**: confirmed. `walEvent()`'s `catch (Exception e)` (not just `IOException`) plus `executeResponse()`'s own `try/catch` mean a `RuntimeException` from `writer.write()` can no longer take down the sole disruptor worker — the comment explaining the widened catch matches the code exactly. - **F4**: confirmed. `appendBlockedAfterWriteFailure` is set on any write exception and checked before the next append, so a torn trailer can't turn into a mid-file tear. Fail-closed as described, not reset/reopen. - **F6/F7**: confirmed — shared `deadlineNanos` in the batch path, and the timed `get()`'s documentation covers the `TimeoutException` contract. One thing worth being precise about, since @SEZ9 raised it too: the residual gap isn't the untimed `get()` call itself, it's that `catch (Exception e)` still lets an `Error` (e.g. `OutOfMemoryError`) escape and kill the worker thread. At that point every *timed* caller still resolves correctly by riding out its own timeout — that's exactly what the timeout is for — but nothing currently notices the worker itself died, so it won't restart and every append after that silently fails closed forever. Agreed this belongs in a separate follow-up rather than this PR — happy to see it opened by either of you. No further changes requested from me on `7f69d7f69f6`. Nice work tightening this up. -- 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]
