DanielLeens commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-6266160144
Picking up the two loose ends in this thread so nothing stalls on my side. @Rangsh — thanks for completing the cut-off F8 sentence: one WARN line per timeout carrying `requestId`, elapsed ms and the limit ms, with the `TimeoutException` stack at DEBUG, and ERROR + stack reserved for the unexpected `catch (Exception)` branch in `IMapFileStorage.queryExecuteStatus` (L331–345). That is exactly the logging shape I was after, so F8 is closed from my perspective. And to close out @SEZ9's F4 question directly, since I had the head open anyway — the answer is closest to option (b), with one precision about what the test does and does not prove: - The mitigation for the torn-record risk is the sticky fail-close, not a re-read of the file. After the first `writer.write(...)` failure, `WALWorkHandler` sets `appendBlockedAfterWriteFailure = true` (`WALWorkHandler.java:111-113`), and every later APPEND returns early at `WALWorkHandler.java:99` without touching the writer — so no further bytes are ever appended after a possibly partial record by the same handler. - `WALWorkHandlerSurvivabilityTest.assertFailCloseAfterFirstWriteFailure` (L132–184) pins exactly that: one failing APPEND followed by a second APPEND on the same handler, both futures completing with `false`, the writer invoked exactly once (`times(1)`, `writeCalls == 1`), and `isAppendBlockedAfterWriteFailure()` still true. It covers both a non-IO exception from `write` (L66) and a flush failure from `HdfsWriter` (L91). If the sticky flag were removed, the second APPEND would hit the writer again and `times(1)` would fail. - What the test does **not** assert: the writer is mocked, so the file is never read back. The claim it pins is "no append follows a failed write", not "the file is still readable". Recovering from a partially written tail record on the next restore is reader-side behaviour that this PR doesn't change, so I'd treat it as out of scope here. So my remaining items are all verified or closed: F1 (documented contract on the untimed `get()`, no production caller), F2/F5 (`26ce5cb26`), F3/F7 (`TimeoutException` + `@throws`), F4 (above), F6 (one shared `deadlineNanos` across the batch), F8 (above). That said — @SEZ9 has the final pass on this thread and I don't want to step on it. My read is that there's no source-level blocker left on `ab04db33a`, and CI on the head is green, so this is ready for whoever wants to give the last approval. Thanks @Rangsh for the itemized responses; they made this much faster to close. -- 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]
