SEZ9 commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-6244666693
@Rangsh thanks for the nudge, and apologies for the delay on my side. On F4 (`WALWorkHandler` reusing the writer after a write failure): I saw the note in this thread that `WALWorkHandlerSurvivabilityTest` on `ab04db33a` already covers the APPEND write-failure case (first future completes `false` without `onEvent` throwing, a second APPEND also completes `false`, the writer is invoked once via `times(1)`, and `isAppendBlockedAfterWriteFailure()` stays true). That is the coverage I was after, and a `times(1)` assertion would catch the sticky flag being removed. I will verify that against the diff at `ab04db33a` myself before closing F4, so no action needed from you there. For the remaining items from the earlier review (F1, F2, F3, F5, F6, F7, F8) I don't see a status update in this thread yet. Could you post a short per-item note: addressed on `ab04db33a`, intentionally left as-is (with reasoning), or still pending? The ones I most want to settle: - F1/F3/F7: the untimed `RequestFuture.get()` becoming unbounded, and the timed `get()` now throwing `TimeoutException` instead of returning `false` (plus documenting that on the method itself). - F2: the single disruptor worker dying if `executeResponse()` throws. - F5: whether a Mockito test dependency is added to the `imap-storage-file` module for `HdfsWriterFlushSyncPathTest`. - F6/F8: the sequential full-timeout wait in `batchQueryExecuteFailsStatus`, and the ERROR-level stack trace on every timed-out wait in `queryExecuteStatus`. Once I have that status I'll do a final pass promptly. <!-- streview-comment:1833 --> -- 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]
