Rangsh commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5540294434
Thanks @DanielLeens for the careful re-read — and for correcting the earlier durability-test claim. That distinction between same-process read-back visibility (`hflush`-level / page cache) and an actual `hsync`-family invocation is exactly right. ### What I changed **Issue 1 (Medium):** added `HdfsWriterFlushSyncPathTest`, which spies/mocks each of the three `HdfsWriter.flush()` branches and asserts: - exactly one `hsync`-family call (`hsync(UPDATE_LENGTH)` or plain `hsync()`) - `hflush()` is never called So a silent regression that downgrades a branch to `hflush()`-only (or stacks multiple syncs) would fail these tests. I also updated `HdfsWriterDurableFlushTest`'s javadoc so it no longer over-claims crash durability — it only documents mid-stream cross-handle visibility. **Issue 2 (Low / MiniDFS):** still treating a real `MiniDFSCluster` integration test as a follow-up, as discussed. The new mock tests do exercise the previously uncovered `HdfsDataOutputStream` and wrapped-`DFSOutputStream` *control-flow* branches (call counts), without adding a MiniDFS harness to this module. Happy to open a separate MiniDFS PR if maintainers want end-to-end HDFS coverage on top of that. Also noted your point on the batch-timeout side effect (effective 1s → configured 60s default): intentional contract fix, not a free lunch for operators who were relying on the old accidental fail-fast. Appreciate the thorough review again. -- 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]
