Rangsh commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-6554155950
@SEZ9 thanks for going back over `b172d96d`. The pointers below are on the current head `ef99d22d1` (the F6/F4/F2/F3/F7/F5/F8 hunks landed after `b172d96d`; F1 is the new commit). **F6 — shared batch deadline.** One deadline per batch, computed once before the loop, in `IMapFileStorage.batchQueryExecuteFailsStatus`: https://github.com/apache/seatunnel/blob/ef99d22d1/seatunnel-engine/seatunnel-engine-storage/imap-storage-plugins/imap-storage-file/src/main/java/org/apache/seatunnel/engine/imap/storage/file/IMapFileStorage.java#L394-L417 Each entry waits only for the time still left. When the budget is already spent, `get(0, NANOSECONDS)` still harvests a future that has already completed, so a finished success is not reported as a failure. **F4 — writer not reused after a failed append.** `WALWorkHandler.walEvent`: the first append failure sets `appendBlockedAfterWriteFailure`. Later APPEND events are answered `done(false)` and return before `writer.write`, so nothing else is appended to a stream that may already end in a torn frame: https://github.com/apache/seatunnel/blob/ef99d22d1/seatunnel-engine/seatunnel-engine-storage/imap-storage-plugins/imap-storage-file/src/main/java/org/apache/seatunnel/engine/imap/storage/file/disruptor/WALWorkHandler.java#L93-L119 The flag is `volatile` because `isAppendPermanentlyBlocked()` reads it from the caller thread. **F2 — `executeResponse` is wrapped.** The completion call is not left outside the write `try`. `executeResponse` catches any failure from cache lookup / `future.done()` so it cannot kill the single WAL worker: https://github.com/apache/seatunnel/blob/ef99d22d1/seatunnel-engine/seatunnel-engine-storage/imap-storage-plugins/imap-storage-file/src/main/java/org/apache/seatunnel/engine/imap/storage/file/disruptor/WALWorkHandler.java#L117-L140 **F3 — callers of `get`.** The only production callers in this module are `queryExecuteStatus` and `batchQueryExecuteFailsStatus`, both on the timed overload, both treating `TimeoutException` as failure for that key. No production caller uses the untimed `get()`. **F1 — untimed `get()` is no longer an unbounded block.** `Future` requires the overload, so it cannot be removed. `RequestFuture.get()` now delegates to `get(writDataTimeoutMilliseconds, MILLISECONDS)` and, on expiry, throws `ExecutionException` whose cause is `TimeoutException` (this signature cannot throw `TimeoutException` directly). The future is created in `sendToDisruptorQueue` with the configured `writDataTimeoutMilliseconds`; the no-arg constructor uses the same default (`DEFAULT_WRITE_DATA_TIMEOUT_MILLISECONDS`, 60s). A worker killed by an `Error`, or an event that was never dispatched, therefore cannot hang a caller forever. Covered by `RequestFutureTest.untimedGetShouldUseConfiguredCapInsteadOfBlockingForever`. **F7 — method-level contract.** On `get(long, TimeUnit)`, not only the class note: timeout throws `TimeoutException` instead of returning `false`, and does not cancel the in-flight append: https://github.com/apache/seatunnel/blob/ef99d22d1/seatunnel-engine/seatunnel-engine-storage/imap-storage-plugins/imap-storage-file/src/main/java/org/apache/seatunnel/engine/imap/storage/file/future/RequestFuture.java#L98-L112 **F5 — Mockito test dependency.** Explicit `test` scope on `imap-storage-file`, not only inherited from the root POM: https://github.com/apache/seatunnel/blob/ef99d22d1/seatunnel-engine/seatunnel-engine-storage/imap-storage-plugins/imap-storage-file/pom.xml#L81-L90 **F8 — timeout is WARN, one line.** Both `queryExecuteStatus` and the batch path log `requestId` plus the timeout at WARN. The `TimeoutException` stack is DEBUG only, so a stuck worker does not emit an ERROR stack per key: https://github.com/apache/seatunnel/blob/ef99d22d1/seatunnel-engine/seatunnel-engine-storage/imap-storage-plugins/imap-storage-file/src/main/java/org/apache/seatunnel/engine/imap/storage/file/IMapFileStorage.java#L378-L385 Happy to adjust any of these on the final pass. -- 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]
