JingsongLi commented on PR #9133: URL: https://github.com/apache/paimon/pull/9133#issuecomment-5231438594
Could we avoid introducing `new_range_input_stream` and keep a single stream API instead? A cleaner design would be for Jindo’s `new_input_stream` to return a lazy adapter that: - opens the native Jindo stream on the first `read`, `seek`, or positional read; - implements the regular file interface (`read`, `seek`, `tell`, and `close`); - exposes native `pread`/`read_at`; - declares whether positional reads are safe to execute concurrently. This follows the capability-based design used by POSIX `pread`, Hadoop `PositionedReadable`, Go `ReaderAt`, and PyArrow `NativeFile`. It also avoids adding and manually forwarding another FileIO method through Caching, Resolving, RESTToken, and future wrappers. One important detail is that the range-read path must not go through `pyarrow.PythonFile`: I verified that `PythonFile.read_at()` performs `seek + read` on the wrapped object, even when the wrapped object itself provides `read_at`. Therefore, the Jindo adapter should be returned directly by `new_input_stream` so that coalesced reads can invoke native `pread`, while regular callers can continue using the standard stream methods. I also noticed two correctness issues: 1. `_RangeStreamPool.close()` stops at the first close failure after clearing the pool. This can leak all remaining streams and may mask the original read exception. Please attempt to close every stream and then propagate the first close error. 2. `_HdfsReaderAdapter.read_at()` directly calls `hdfs-native`’s `read_range`. That implementation panics when `offset + length` exceeds the file length, whereas the previous `seek + read` behavior returned the remaining bytes. Please clamp the requested range to EOF and add an out-of-range test. -- 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]
