peterxcli opened a new pull request, #11724: URL: https://github.com/apache/ozone/pull/11724
## What changes were proposed in this pull request? ### Problem `BlockInputStream.readPositioned` and `StreamBlockInputStream.readPositioned` take the stream lock on every call. The sequential read of each class holds the same lock while it waits for the datanode: `BlockInputStream.readWithStrategy` during the ReadChunk RPC, and `StreamBlockInputStream.readFully` while it waits for streamed data. So a positioned read waits for any sequential read in progress on the same block. ### Changes Both classes now follow one rule. After the one-time setup of a block stream, a positioned read takes no lock. The setup stays synchronized and writes a volatile flag last. - `BlockInputStream`: once the volatile `initialized` is true, `readPositioned` reads `chunkOffsets`, `chunkStreams` and `length` without the lock. `initialize()` sets them before `initialized`, and nothing changes them after that. `initialize()` and `refreshBlockInfo()` (used only after a read error) stay synchronized. - `StreamBlockInputStream`: `needUpdateBlockLength` is volatile. `readPositioned` calls the synchronized `updateBlockLength()` only while the flag is set, and then reads `blockLength` without the lock. `xceiverClientFactory` is final. `close()` sets a `closed` flag guarded by the lock, and `checkOpen()` tests that flag. - Neither block stream checks for close in `readPositioned`. `MultipartInputStream.readPositioned`, their only production caller, checks its own volatile `closed` flag first. EC readers use `seek()` and `read()` instead. - Sequential reads do not change. One trade-off: the block stream no longer stops a positioned read that races with `close()`. On a short-circuit read, such a read retries under the read retry policy before it fails. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-17399 ## How was this patch tested? New unit tests: - `TestBlockInputStream#testPositionedReadDoesNotWaitForSequentialRead` and `TestStreamBlockInputStream#testPositionedReadDoesNotWaitForSequentialRead`: a positioned read completes while a sequential read on the same block holds the lock. The StreamBlock test also covers an under-construction block. Both tests time out without the change. - `TestBlockInputStream#testPositionedReadOnBlockWithNoChunks`: a block with no chunks returns EOF. - `TestMultipartInputStream#testPositionedReadAfterCloseFails`: after `close()`, a positioned read fails and does not reach either block stream type. The client read-path unit tests, the `TestKeyInputStream` and `TestStreamBlockInputStream` integration tests, and checkstyle pass locally. Benchmark: one host, 3 datanodes, warm page cache. One shared stream on a 256 MiB key, with 1 sequential reader and N positioned-read threads. Median of 3 runs. | Read path | Positioned reads | Before (ops/s, p50, p99) | After (ops/s, p50, p99) | |---|---|---|---| | Chunk (default) | 1 thread × 64 KiB | 2, 680 ms, 1.55 s | 1,811, 0.41 ms, 1.36 ms | | Chunk (default) | 8 threads × 64 KiB | 9, 721 ms, 4.89 s | 7,398, 0.78 ms, 2.04 ms | | Stream (`ozone.client.stream.readblock.enable`) | 1 thread × 64 KiB | 3, 309 ms, 641 ms | 586, 1.3 ms, 5.6 ms | | Stream (`ozone.client.stream.readblock.enable`) | 8 threads × 64 KiB | 10, 653 ms, 3.56 s | 2,060, 3.3 ms, 9.4 ms | With positioned reads only (1 to 32 threads), throughput does not change beyond run-to-run noise (about ±10%). -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
