Rangsh commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5538217701
Thanks @DanielLeens for the thorough review — especially for independently tracing the HDFS multi-sync path, the `RequestFuture` batch timeout discrepancy, and the Disruptor worker-thread risk in `WALWorkHandler`. On the two Low items: 1. **HDFS-branch coverage (`HdfsDataOutputStream` / wrapped `DFSOutputStream`)** — Agreed it’s a gap. I’m leaving it out of this PR on purpose: the module has no `MiniDFSCluster` harness today, and adding one would be a heavier follow-up than the current LocalFileSystem durability test (`HdfsWriterDurableFlushTest`). The single-sync control flow for those branches is covered by code inspection against the Hadoop `Syncable` contract, as you verified. 2. **`volatile success` comment** — Added a one-line note explaining why `volatile` is required so it isn’t treated as decorative later. Appreciate the merge recommendation and the re-weighting of the correctness fixes vs the CV headline. Happy to follow up with a MiniDFS-focused test in a separate PR if maintainers want that coverage next. -- 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]
