SEZ9 commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-6105458590
Thanks for the follow-up on `ab04db33a`. Since the earlier approval was on `1cbac0086`, I went back over F1–F8 against the latest head. Where things stand from my side: - **F1 / F3 / F7 — `RequestFuture.get()` contract:** moving to deadline-bounded waits with a real `TimeoutException` is the right direction. Two asks: (a) confirm no caller on the WAL path still uses the untimed `get()`, since that one is now an unbounded block if the worker dies via `Error` or the event is never dispatched; (b) put the `throws TimeoutException` contract on the method Javadoc itself, not only in the class-level note. - **F2 / F4 — `WALWorkHandler`:** could you point me to the diff that makes APPEND fail-closed after a write failure so I can confirm the torn-record concern is covered? Also, please confirm `executeResponse()` is guarded so an exception there cannot kill the single disruptor worker — the caller-side timeout+remove race makes that path reachable. - **F6 — `batchQueryExecuteFailsStatus`:** please confirm the batch wait uses a single shared deadline across entries rather than the full `writDataTimeoutMilliseconds` per entry; otherwise a stuck worker still blocks `storeAll`/`deleteAll` for N × timeout. - **F8 — logging in `queryExecuteStatus`:** a full `TimeoutException` stack trace at ERROR on every timed-out wait is noisy; a WARN with the message only would be sufficient. Your call, just say which you prefer. - **F5 — Mockito dependency:** I still don't see a `pom.xml` change adding a Mockito test dependency to the imap-storage-file module alongside `HdfsWriterFlushSyncPathTest`. If it arrives transitively, point me to where; otherwise please add it explicitly. <!-- streview-comment:1678 --> -- 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]
