yandrey321 commented on code in PR #11783:
URL: https://github.com/apache/ozone/pull/11783#discussion_r4427326523
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStream.java:
##########
@@ -327,15 +335,56 @@ protected long remaining() {
return blockLength() - position;
}
+ /**
+ * Reads the cells covered by the buffer from the internal block streams of
their data indexes with positioned
+ * reads, so neither this stream nor its internal streams change position,
and calls may run concurrently with each
+ * other and with sequential reads. The indexes are read in parallel if the
parallel read executor is set.
+ *
+ * <p>A failed index is not switched to a spare. Instead, a {@link
BadDataLocationException} carrying every failed
+ * index is thrown, so that the caller can fall back to a reconstruction
read for this call.
+ */
@Override
- protected int readPositioned(long offset, ByteBuffer buffer) {
- throw new UnsupportedOperationException("Use ECBlockInputStreamProxy for
positioned reads");
- }
+ protected int readPositioned(long offset, ByteBuffer buffer) throws
IOException {
+ checkOpen();
Review Comment:
Yes, and it's more than a nit: it closes a small race. `readPositioned`
isn't `synchronized(this)`, so it can run alongside `close()`:
1. A pread passes the first `checkOpen()`.
2. `close()` sets `closed = true`, then `closeStreams()` closes each
per-index stream and sets its slot to null.
3. The pread calls `getOrOpenStream(index)`, finds the slot empty, and opens
a **new** stream.
The read still fails with an `IOException`, on the `checkOpen()` after
`runIndexReads`. But the new stream stays in `blockStreams[index]` after
`close()` has finished. Its read has gone through
`ChunkInputStream.acquireClient()`, so it holds an `XceiverClient` reference
that nothing releases.
I've added the check inside the `synchronized (streamLock)` block, at the
place you suggest in the comment below (see the reply there). The first
`checkOpen()` stays as an early exit, and the one after the reads stays for the
reason in its comment.
--
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]
