taklwu commented on code in PR #11102:
URL: https://github.com/apache/ozone/pull/11102#discussion_r4012124117
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/ChunkInputStream.java:
##########
@@ -426,15 +435,101 @@ protected void readChunkDataIntoBuffers(ChunkInfo
readChunkInfo)
}
/**
- * Send RPC call to get the chunk from the container.
+ * Stateless positioned read of up to {@code dst.remaining()} bytes starting
+ * at {@code chunkRelativePosition} within this chunk. Unlike the buffered
+ * {@link #read} path, this does not read or mutate any of the instance's
+ * buffer/position state ({@code buffers}, {@code chunkPosition},
+ * {@code bufferOffsetWrtChunkData}, ...), so it is safe to call concurrently
+ * from multiple threads sharing the same stream.
+ *
+ * @param chunkRelativePosition start offset within this chunk
+ * @param dst destination buffer
+ * @return number of bytes copied into {@code dst}, or {@link #EOF} at EOF
+ */
+ int readPositioned(long chunkRelativePosition, ByteBuffer dst) throws
IOException {
+ if (chunkRelativePosition < 0 || chunkRelativePosition >= length) {
+ return EOF;
+ }
+ final int toRead = (int) Math.min(dst.remaining(), length -
chunkRelativePosition);
+ if (toRead == 0) {
+ return 0;
+ }
+
+ final ChunkInfo readChunkInfo = getChunkInfo(chunkRelativePosition,
toRead);
+ final long adjustedOffset = readChunkInfo.getOffset() -
chunkInfo.getOffset();
+
+ final long skip = chunkRelativePosition - adjustedOffset;
+ if (xceiverClientFactory == null) {
Review Comment:
so, this should be used by LocalChunkInputStream that's short-circuit read,
but somehow the new assertion after new SCR feature failed with test
`org.apache.hadoop.ozone.client.rpc.read.TestChunkInputStream` because this
`xceiverClientFactory` is not null when `LocalChunkInputStream` created from
`BlockInputStream#createChunkInputStream`
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/LocalChunkInputStream.java:
##########
@@ -62,7 +63,9 @@ public class LocalChunkInputStream extends ChunkInputStream
LocalChunkInputStream(ChunkInfo chunkInfo, BlockID blockId,
XceiverClientFactory xceiverClientFactory,
Supplier<Pipeline> pipelineSupplier, boolean verifyChecksum,
Supplier<Token<?>> tokenSupplier,
XceiverClientShortCircuit xceiverClientShortCircuit, FileInputStream
blockInputStream) {
- super(chunkInfo, blockId, xceiverClientFactory, pipelineSupplier,
verifyChecksum, tokenSupplier);
+ // Pass null for xceiverClientFactory such readPositioned() is taken, both
routing reads
+ // and checksum verification are done locally.
+ super(chunkInfo, blockId, null, pipelineSupplier, verifyChecksum,
tokenSupplier);
Review Comment:
before this change, even if `createChunkInputStream` should be passing a
empty `xceiverClientFactory` (null), in the test
`org.apache.hadoop.ozone.client.rpc.read.TestChunkInputStream` itself,
`xceiverClientFactory` is not null such that `ChunkInputStream#readPositioned`
didn't use the `LocalChunkInputStream#readChunk` and created a
`XceiverClientSpi` client.
it should be a test issue instead, but since `xceiverClientFactory` should
not be used with `LocalChunkInputStream`, I fixed it by enforcing the null
value when `LocalChunkInputStream` is being constructed. noted that the passing
in `xceiverClientShortCircuit` is not used anywhere at the moment.
--
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]
