SEZ9 commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5738724926
Thanks for the follow-up. From the thread, two of the earlier points look settled on `7f69d7f69`: - **Mockito test dependency (F5)** — an explicit `test`-scope `mockito-junit-jupiter` entry in `imap-storage-file/pom.xml`. - **Timeout logging noise (F8)** — `TimeoutException` in `queryExecuteStatus` / `batchQueryExecuteFailsStatus` now logs at WARN with `requestId` plus elapsed/limit (full stack only at DEBUG), with unexpected `Exception` still at ERROR. That's the split I was hoping for. I don't see anything in the thread yet on the remaining points from the earlier review. Could you point me at where they were addressed, or say how you'd like to handle them? 1. **F1 / F3 – `RequestFuture.get()` semantics**: the untimed `get()` moving from a 1-second-capped wait to an unbounded block still concerns me if the WAL worker dies via an `Error` or the event is never dispatched. Is there an upper bound (or a failure path that completes the future) for that case, and is the untimed `get()` still used on any hot path? 2. **F2 – `WALWorkHandler` worker death**: with the caller-side timeout-and-remove race, is `executeResponse()` guarded so a late or missing future can't take down the single disruptor worker? 3. **F4 – writer reuse after write failure**: after an arbitrary write exception, is the writer reset/reopened before the next record, so a torn record isn't left mid-file with further appends after it? 4. **F6 – `batchQueryExecuteFailsStatus` sequential waits**: with a stuck worker this can take `N × writDataTimeoutMilliseconds`. Would a shared deadline across the batch (or short-circuiting after the first timeout) work? 5. **F7 – method-level Javadoc**: a one-line note on `get(timeout, unit)` that it now throws `TimeoutException` instead of returning `false` would be enough. If any of these were intentionally left as-is, a short rationale is fine. Once they land or are explained, I'm happy to do a final pass. <!-- streview-comment:1157 --> -- 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]
