SEZ9 commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-6593596114
@Rangsh thanks for the pointers on `ef99d22d1`. **F1 / F3** – Understood: `Future` requires the untimed `get()`, it now delegates to the timed `get` with the configured `writDataTimeoutMilliseconds`, and both callers in `IMapFileStorage` treat a timeout as a failed write. That is the right shape for the fix; I will confirm it against the ranges you linked before marking these resolved. On the `writDataTimeoutMilliseconds` field name missing the `e`: do you plan to rename it here, or leave it for a follow-up? **F2 / F4** – You mention these were covered earlier; could you point me at the specific comment or commit so I can verify against the diff? **F5** – Your link got cut off at `seatunnel-engine/seatunne`, so I could not open it. If Mockito is already an explicit test dependency of `imap-storage-file`, no pom change is needed and F5 is closed. Could you re-paste the full link? Still open from my earlier review: - **F6** – `batchQueryExecuteFailsStatus` waits the full write timeout per entry sequentially, so a stuck worker can block `storeAll`/`deleteAll` for N × `writDataTimeoutMilliseconds`. A shared deadline across the batch, or failing fast once the first entry times out, would keep the worst case bounded. If the per-entry wait is intentional, a short comment explaining why is also fine. - **F7** – Please add a method-level Javadoc on `RequestFuture.get(timeout, unit)` stating that it now throws `TimeoutException` instead of returning `false`. - **F8** – Logging a full `TimeoutException` stack trace at ERROR on every timed-out wait in `queryExecuteStatus` will be noisy under a stuck worker; a WARN with the message only seems sufficient since the caller already fails the write. Once F6–F8 are addressed (or you push back with reasoning) and the F2/F4/F5 pointers are in, I will do a final pass. <!-- streview-comment:2228 --> -- 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]
