wombatu-kun opened a new pull request, #9926:
URL: https://github.com/apache/paimon/pull/9926
### Purpose
`HadoopFileIO` hands out a stream that does not implement
`VectoredReadable`, so all ten `instanceof VectoredReadable` gates in the tree
take the fallback branch: `seek` plus a sequential read, four of them under a
`synchronized` block on the shared stream. `LocalFileIO` has implemented the
interface since #3369, `HadoopFileIO` never did.
That fallback is expensive because `HadoopSeekableInputStream.seek` turns a
forward gap of up to `MIN_SKIP_BYTES` (1 MiB) into `skipFully`, and no Hadoop
stream overrides `skip`, so `java.io.InputStream.skip` applies and that reads
into a scratch buffer and discards. A 4 KiB page 600 KiB ahead costs 604 KiB
off the wire.
`pread` now comes from Hadoop's `PositionedReadable`, which
`FSDataInputStream` is guaranteed to provide: its constructor rejects a stream
that is not both `Seekable` and `PositionedReadable`.
**When the new branch is taken, and how often.** At all ten gates, whenever
the stream came from `HadoopFileIO` on a non object store:
`CachingSeekableInputStream`, `BlockCache`, `FileBasedBloomFilter`,
`FMIndexFile`, `ParquetFileReader`, `RecordReaderUtils`, `BlockPrefetcher`,
`MosaicInputFileAdapter`, `VFSInputStream` and the vector index adapter. On
HDFS that is every Parquet and ORC read, every bloom filter probe and every
vector index search.
**Why the scheme is checked rather than the module.** `HadoopFileIO` is not
the `hdfs://` implementation, it is the universal Hadoop fallback in
`FileIO.get`, so object stores reach it too. A block file system implements a
real positional read (`DFSInputStream` has its own, unsynchronized, over its
own `BlockReader`); an object store does not, and inherits `FSInputStream`'s
emulation, `synchronized { seek(pos); read; finally seek(oldPos); }`. On OSS
that is two `reopen` calls per read. Object stores want Hadoop's own
`readVectored`, which does not exist before 3.3.5 while this project compiles
against 2.8.5; separate change.
Also fixes `VectoredReadUtils.readSingleRange`, which this change is what
makes reachable: it caught `Exception`, so an `Error` left the range's future
uncompleted and the submitted task's own `Future` was dropped. `FSError`
extends `Error` and `RawLocalFileSystem` raises it for any `IOException`, which
turned a read failure into a permanent block in ORC's untimed `get()` and the
vector index's `join()`.
### Benchmark
MiniDFSCluster, a beam of 32 scattered 4 KiB pages, 200 measured rounds per
run, median of 3 runs each, measured back to back:
| | before | after |
| --- | --- | --- |
| mean | 20.36 ms | 8.00 ms |
| p50 | 19.31 ms | 7.31 ms |
| p90 | 25.15 ms | 10.89 ms |
| p99 | 49.32 ms | 45.76 ms |
Bytes the stream is asked for over 200 scattered 4 KiB reads, counted at the
`FSDataInputStream` boundary:
| gap between reads | before | after |
| --- | --- | --- |
| 64 KiB | 13,045,760 | 819,200 |
| 256 KiB | 52,170,752 | 819,200 |
| 768 KiB | 156,504,064 | 819,200 |
### Tests
`FileIOBehaviorTestBase` had no read-path test at all; four were added,
covering the capability, `pread`'s return contract, that it leaves `getPos`
alone, and eight concurrent readers. They run for `LocalFileIO`, for
`HadoopFileIO` over a Hadoop local FS and for HDFS on `MiniDFSCluster`.
`HadoopFileIOVectoredReadTest` pins that `pread` reaches `PositionedReadable`
rather than `seek`, and the scheme gate. Reverting both production changes
fails 11 tests.
--
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]
