SEZ9 commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5862160690
@Rangsh — thanks for confirming. The final pass on `7f69d7f69f6` is complete and the approval is recorded on that head, so nothing further is needed from you on this PR. Closing out F1–F8 against `7f69d7f69f6`: - **F1** — the untimed `RequestFuture.get()` has no production callers in `imap-storage-file`. - **F2 / F7** — confirmed via direct code and Javadoc inspection. - **F3** — both callers of the timed `get()` were rewritten in this diff to the `TimeoutException` contract, so the semantic change is contained. - **F4** — writer reuse after a write failure is fail-closed via the sticky `appendBlockedAfterWriteFailure` flag, covered by `DefaultReaderTornTrailingRecordTest`, `DefaultReaderTornMidFileRecordTest` and `WALWorkHandlerSurvivabilityTest`. - **F5** — the Mockito test-scope dependency is declared explicitly in `imap-storage-file/pom.xml`. - **F6** — `IMapFileStorageBatchDeadlineTest` asserts the shared batch deadline directly. - **F8** — timeouts log a single WARN (full exception at DEBUG), with ERROR + stack trace reserved for the unexpected `catch (Exception e)` branch. On the follow-up: agreed that the `Error` escaping `writer.write()` / dead-worker case belongs in #12492 rather than here — thanks for opening it. Keeping this PR scoped to the race and timeout-correctness fix was the right call. No remaining asks from my side. <!-- streview-comment:1361 --> -- 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]
