SEZ9 commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-6258122196
Thanks @Rangsh for the itemized status against `ab04db33a`. Going point by point: - **F1** — Accepted as a deliberate choice. An untimed `get()` following the plain `Future` contract is reasonable as long as no production caller uses it, and documenting that on the method is the right place for it. - **F3 / F7** — Throwing `TimeoutException` from the timed `get(long, TimeUnit)` and declaring it on that method is what I was after. I'll confirm against the diff. - **F2** — Moving the cache lookup inside the try and catching `Exception` should cover the `executeResponse()` path. I'll verify in `26ce5cb26`. - **F5** — A test-scoped `mockito-junit-jupiter` in `imap-storage-file/pom.xml` is the right fix; I'll check `26ce5cb26` for it since it isn't in the `ab04db33a`-only diff. Rename to `HdfsWriterFlushCallCountTest` noted. - **F6** — One shared `deadlineNanos` across the batch addresses the N × full-timeout concern. Will confirm in the diff. - **F8** — Your comment appears to be cut off at "One WARN line per timeout (requestId /". Could you finish that sentence? I want to confirm the full stack trace is no longer logged at ERROR for every timed-out wait and what the single WARN line carries. One remaining ask on **F4**: "no action from me" plus a pointer to `WALWorkHandlerSurvivabilityTest` reads as coverage for the worker staying alive, which is F2's concern. F4 was about the handler continuing to write with the same writer after an arbitrary write failure and the risk of a torn record mid-file. Could you briefly say whether (a) that test exercises a failed write followed by a subsequent write on the same writer and asserts the file is still readable, or (b) you consider the torn-record risk acceptable/out of scope for this PR and why? Either answer is fine — I just need it stated explicitly. Once F8's text is completed and F4 has an explicit position, I'll do a final pass over the diff and should have nothing further. <!-- streview-comment:1855 --> -- 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]
