HDDS-16400. Fix thread-safety of positioned reads in OzoneFSInputStream

[taklwu]

What changes were proposed in this pull request?

followup by HDDS-15424, we have created HDDS-16400 that implements the thread-safe positional read for OzoneFSInputStream

Please describe your PR in detail:

1. ECBlockInputStream#readFully return false (use the default ExtendedInputStream#readFully), so it fail back to use the new additional synchronized lock for readSeekRestore.
2. for those non-ByteBuffer / non-ByteBufferReadable inputstream, we have to use byte[] for seekReadRestore instead of casting with ByteBufferReadable

What is the link to the Apache JIRA

• https://issues.apache.org/jira/browse/HDDS-16400

How was this patch tested?

unit tests.

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Improves thread safety for positional reads through Ozone filesystem and encrypted streams.

Changes:

• Synchronizes fallback seek/read/restore operations.
• Adds synchronized positional APIs to OzoneCryptoInputStream.
• Adds concurrent positioned-read tests and the required test dependency.

File summaries

File	Description
hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSInputStream.java	Updated as part of this pull request.
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneFSInputStream.java	Updated as part of this pull request.
hadoop-ozone/ozonefs-common/pom.xml	Updated as part of this pull request.
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/OzoneCryptoInputStream.java	Updated as part of this pull request.

Review details

Suppressed comments (2)

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/OzoneCryptoInputStream.java:84

• The synchronization added here does not cover inherited cursor-mutating operations such as skip. A concurrent skip can change CryptoInputStream's cursor or internal buffer while read(long, ByteBuffer) is between its seek/read/restore steps, so the positioned read can return corrupted data despite the new thread-safety guarantee. Override the cursor-mutating methods that are not already synchronized (at least skip), or narrow this documentation and the guarantee to the synchronized operations.

   * {@link CryptoInputStream} does not synchronize its own methods, so every method moving the cursor of
   * this stream is serialized here on the monitor of this stream. Otherwise a read or a seek could land in
   * the middle of a positioned read and see (or undo) the cursor move that read does.

hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneFSInputStream.java:42

• MultipartInputStream.readFully is not always stateless: its StreamBlockInputStream path performs a synchronized seek/read/restore sequence (see MultipartInputStream.java:197-233), while only the block path is stateless. Describing every successful ExtendedInputStream path as stateless is misleading for the concurrency guarantee; please call this the underlying positioned-read implementation or distinguish the two modes.

 * Sequential reads are not thread safe. Positioned reads use a native
 * stateless path when the underlying {@link ExtendedInputStream} supports it;
 * otherwise they fall back to a synchronized seek-read-restore sequence.

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Validate read-only ByteBuffer destinations before positioned reads to prevent state corruption after an exception.

Review details

Suppressed comments (1)

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/OzoneCryptoInputStream.java:183

• A read-only destination is not rejected before the seek/read sequence. For an unaligned or short request, read(byte[], ...) sets readPositionAdjustedBy/readLengthAdjustedBy and then dst.put(...) throws ReadOnlyBufferException; the finally restores only the cursor, leaving those adjustment fields set, so the next valid read fails the precondition at getNumBytesToRead. Check dst.isReadOnly() before starting the positioned read (as the other ByteBuffer read path does).

  public synchronized int read(long position, ByteBuffer dst) throws IOException {
    if (!dst.hasRemaining()) {
      return 0;

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved cursor-safety and byte-array positioned-read compatibility issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The byte-array overrides regress validation, EOF handling, and read statistics.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneFSInputStream.java:258

• This reimplementation loses two guarantees of the inherited readFully: it skips argument validation on the non-ExtendedInputStream path, and it treats ExtendedInputStream.readFully(...) == true as proof that the buffer is full. MultipartInputStream can return true after only reading the suffix available before EOF, so this silently leaves the array tail untouched instead of throwing. Now that the overridden read(...) uses the shared lock, delegate to the inherited validating loop.

  public void readFully(long position, byte[] buffer, int offset, int length) throws IOException {
    if (inputStream instanceof ExtendedInputStream) {
      final ByteBuffer buf = ByteBuffer.wrap(buffer, offset, length);
      try {
        if (((ExtendedInputStream) inputStream).readFully(position, buf)) {

• Files reviewed: 3/3 changed files
• Comments generated: 3
• Review effort level: Balanced

[taklwu]

@szetszwo This is the first split PR for OzoneFSInputStream, focusing mainly on input streams that do not implement ExtendedInputStream (aka EC). meanwhile we will have OzoneCryptoInputStream support thread-safe positional read in next PR.