SEZ9 commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5673579613
Thanks for the clean repost — the `flush()`/`hsync` sentence reads clearly now, and the new `WALWorkHandlerSurvivabilityTest.flushFailureFromHdfsWriterShouldFailCloseSubsequentAppend` shape (real `HdfsWriter`, mocked `FSDataOutputStream` where only `hsync()` throws, same fail-close assertion plus `verify(out, times(1)).hsync()`) is exactly the coverage I was missing for the torn-record concern. Good that there's no path where flush/sync throws but the sticky flag isn't set. On the scope note — agreed, I'll treat both as in scope for this pass rather than deferred: 1. **`RequestFuture.get()` wait behavior** — timed `get(timeout, unit)` with the `TimeoutException` contract, shared batch deadline for `storeAll`/`deleteAll`, method-level Javadoc, and production callers on the timed overload only. That covers the unbounded-wait, the compatibility change, the sequential N × timeout on batch ops, and the method-level docs point. I'll confirm on the synced head that no production caller still uses the untimed `get()`. 2. **`WALWorkHandler`** — `catch (Exception)`, guarded `executeResponse()`, and sticky fail-close after write/flush failure. With the survivability test above, that closes the worker-death and writer-reuse points from my side once I re-read the head. One thing: your comment got cut off again mid-sentence at "covered by `WALWorkHa`" — anything after item 2 didn't come through. Could you repost the remainder? Specifically I still don't see where these landed: - The Mockito test dependency for the `imap-storage-file` module (`HdfsWriterFlushSyncPathTest` and the new survivability test both need it) — is there a `pom.xml` change on the current head, or does it come in transitively? - The ERROR-level full stack trace on every timed-out wait in `queryExecuteStatus` — did you drop that to a WARN/short message, or keep it as is intentionally? Once I have those two answers I'll do the synced-head pass and close out the remaining items. <!-- streview-comment:1048 --> -- 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]
