Copilot commented on code in PR #11245:
URL: https://github.com/apache/ozone/pull/11245#discussion_r4019910094
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/OzoneCryptoInputStream.java:
##########
@@ -136,6 +146,84 @@ keyName, partIndex, getLength(), numBytesToRead,
return numBytesRead;
}
+ @Override
+ public synchronized int read(ByteBuffer buf) throws IOException {
+ return super.read(buf);
+ }
+
+ @Override
+ public synchronized void seek(long pos) throws IOException {
+ super.seek(pos);
+ }
+
+ @Override
+ public synchronized long getPos() throws IOException {
+ return super.getPos();
+ }
+
+ /**
+ * Positioned read. Decryption can only happen at the Crypto buffer
boundaries, so this stream cannot
+ * read at an arbitrary position without moving its cursor. The read is
therefore serialized against the
+ * other reads on this stream: the cursor is moved to {@code position}, the
data is read through
+ * {@link #read(byte[], int, int)} (which does the Crypto buffer boundary
adjustment) and the cursor is
+ * restored before the lock is released.
+ *
+ * @param position the position to read from.
+ * @param dst the buffer to read into.
+ * @return the number of bytes copied into {@code dst}, or -1 if no byte
could be read.
+ */
+ @Override
+ public synchronized int read(long position, ByteBuffer dst) throws
IOException {
Review Comment:
These overrides only serialize the ByteBuffer positioned-read overloads.
`CryptoInputStream` also exposes the byte-array `PositionedReadable` methods,
whose inherited seek/read/restore sequence does not hold this monitor; callers
using `read(position, byte[], off, len)` can still interleave and corrupt the
shared cursor. Override the byte-array overloads (and `readFully`) or route
both APIs through one synchronized helper.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneFSInputStream.java:
##########
@@ -180,6 +183,16 @@ public int read(long position, ByteBuffer buf) throws
IOException {
}
}
+ // Fallback: stateful seek-read-restore on the shared cursor. Synchronize
the
+ // full sequence so concurrent positioned reads remain thread-safe when the
+ // native stateless path is unavailable (e.g. erasure coded keys).
+ synchronized (positionedReadLock) {
+ return readAtPositionSeekRestore(position, buf);
Review Comment:
This lock is only acquired by `read(long, ByteBuffer)`, but
`OzoneFSInputStream` inherits the byte-array positioned-read methods from
`FSInputStream`. A concurrent `FSDataInputStream.read(position, byte[], off,
len)` therefore still performs seek/read/restore without `positionedReadLock`,
so positional reads can still corrupt the shared cursor. Add byte-array
overloads that use the same lock/helper and cover that API in the concurrency
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
