HDDS-13660. Ozone client support readVectored. by ashishkumar50 · Pull Request #9402 · apache/ozone

ashishkumar50 · 2025-12-01T08:11:14Z

What changes were proposed in this pull request?

Ozone client support readVectored which allows reading multiple file ranges in parallel.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-13660

How was this patch tested?

Tests are added

jojochuang · 2025-12-01T17:19:05Z

@yandrey321

adoroszlai · 2025-12-03T19:17:39Z

Thanks @ashishkumar50 for the patch. I have ported the corresponding contract test from Hadoop, which shows that some input validation is missing (negative length and offset, overlapping ranges, same ranges).

Please feel free to pick it from adoroszlai@ee11cf3 and include in this PR.

https://github.com/adoroszlai/ozone/actions/runs/19905480757/job/57061761264#step:13:5599

(cherry picked from commit ee11cf3)

ashishkumar50 · 2025-12-04T15:29:38Z

@adoroszlai Thanks for the review, handled comments.

yandrey321 · 2025-12-04T15:31:05Z

+          (offset, buffer) -> readRangeData(offset, buffer, initialPosition));
+    } finally {
+      // Restore position
+      synchronized (this) {


what would happen in case of concurrent readVectored() calls?

Synchronized now to avoid race condition in same concurrent stream reads.

yandrey321 · 2025-12-04T15:45:54Z

        });
+      } finally {
+        // Restore position
+        synchronized (this) {


what would happen in case of concurrent readVectored() calls?

Synchronized now to avoid race condition in same concurrent stream reads.

adoroszlai · 2025-12-17T12:52:44Z

@chungen0126 @jojochuang @yandrey321 please take a look

chungen0126

Thanks for @ashishkumar50 the patch. Are there plans to optimize read vector further? The current implementation doesn't seem to fully support fully parallel reading for different ranges yet. I also added some comments.

chungen0126 · 2026-01-15T14:39:37Z

+    );
+
+    // Restore position
+    seek(initialPosition);


I'm wondering if the seek to initialPosition is necessary here? Since the offset changes asynchronously, restoring it at this point might not work correctly. It seems like readRangeData already handles the position restoration correctly.

chungen0126 · 2026-01-15T14:40:54Z

+      );
+
+      // Restore position before returning from method
+      seek(initialPosition);


Same comment as above.

github-actions · 2026-02-06T00:05:50Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

adoroszlai · 2026-02-09T11:59:36Z

@ashishkumar50 please take a look at @chungen0126's comments.

yandrey321

These changes add a lot of synchronization on IO path, is there a write up that justify these changes? Are there any performance benchmark results that measure performance impact of these changes?

yandrey321 · 2026-02-09T14:11:26Z

+   * @apiNote This method is synchronized to prevent race conditions from
+   *          concurrent readVectored() calls on the same stream instance.
+   */
+  public synchronized void readVectored(


does it need to be syncronized?

yandrey321 · 2026-02-09T14:12:16Z

+   */
+  private void readRangeData(long offset, ByteBuffer buffer, long initialPosition)
+      throws IOException {
+    synchronized (this) {


is there a reason for not allowing concurrent reads?

yandrey321 · 2026-02-09T14:14:54Z

   */
  @Override
-  public int read(long position, ByteBuffer buf) throws IOException {
+  public synchronized int read(long position, ByteBuffer buf) throws IOException {


what is the reason for adding syncrhonized here?

yandrey321 · 2026-02-09T14:15:04Z

   */
  @Override
-  public void readFully(long position, ByteBuffer buf) throws IOException {
+  public synchronized void readFully(long position, ByteBuffer buf) throws IOException {


what is the reason for adding syncrhonized here?

yandrey321 · 2026-02-09T14:15:34Z

+   *          concurrent readVectored() calls on the same stream instance.
+   */
+  @Override
+  public synchronized void readVectored(List<? extends FileRange> ranges,


does it need to be synchronized?

github-actions · 2026-03-04T00:07:58Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

github-actions · 2026-03-12T00:07:21Z

Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.

chungen0126 · 2026-07-03T06:59:14Z

Hi @ashishkumar50 @yandrey321 @adoroszlai @jojochuang , just checking in to see if this PR is still active?

I’ve been looking into readVectored lately. To optimize it further, I think BlockInputStream and BlockStreamInputStream should implement ByteBufferPositionedReadable to reduce lock contention between inputstreams.

I'm planning to create two subtasks to track BlockInputStream and StreamBlockInputStream separately. Let me know if you have any thoughts or feedback on this!

adoroszlai · 2026-07-03T07:02:28Z

Thanks @chungen0126 for checking. I guess this was dropped due to lack of time. It would be great if you could continue work on readVectored support.

chungen0126 · 2026-07-15T02:47:07Z

Hi all,

I've submitted a PR to implement positional reads in BlockInputStream. Please take a look and review it when you are available.

Along with the implementation, I've written a microbenchmark for readVectored on this branch to verify the performance gains on the client side from reducing lock contention.

Benchmark Setup:

Platform: Local macOS
File Configuration:
FILE_SIZE = 100 * 1024 * 1024 (100 MB)
BLOCK_SIZE = 20 * 1024 * 1024 (20 MB / 5 blocks total)
CHUNK_SIZE = 4 * 1024 * 1024 (4 MB / 5 chunks per block)

Test Scenarios:

Consecutive range pattern: 20 ranges, 1 MB each
Random range pattern: 20 ranges, 1 MB each, spread randomly
Sparse range pattern: 5 ranges, 128 KB each, widely separated

Results:

--- Pattern: Random Ranges ---
Number of ranges: 4
Total read size : 4.00 MB
Vectored Read (Positional Enabled=true) : Mean: 59.60 ms | Min: 36.05 ms | Max: 74.04 ms | Throughput: 67.12 MB/s
Vectored Read (Positional Enabled=false) : Mean: 120.47 ms | Min: 96.82 ms | Max: 150.69 ms | Throughput: 33.20 MB/s

--- Pattern: Sparse Ranges ---
Number of ranges: 5
Total read size : 0.63 MB
Vectored Read (Positional Enabled=true) : Mean: 164.00 ms | Min: 96.81 ms | Max: 245.98 ms | Throughput: 3.81 MB/s
Vectored Read (Positional Enabled=false) : Mean: 164.61 ms | Min: 101.27 ms | Max: 240.58 ms | Throughput: 3.80 MB/s

Current Progress & Next Steps:

Currently, this branch only implements positional read for general reads. While working on BlockStreamRead, I ran into a few bugs. Therefore, I will follow up with another PR to implement positional read for BlockStreamRead later.

yandrey321 · 2026-07-15T13:26:17Z

Hi @ashishkumar50 @yandrey321 @adoroszlai @jojochuang , just checking in to see if this PR is still active?

I’ve been looking into readVectored lately. To optimize it further, I think BlockInputStream and BlockStreamInputStream should implement ByteBufferPositionedReadable to reduce lock contention between inputstreams.

I'm planning to create two subtasks to track BlockInputStream and StreamBlockInputStream separately. Let me know if you have any thoughts or feedback on this!

We also need to evaluate how it would work with read streaming, I can pick one some of these tasks.

ashishkr200 added 2 commits December 1, 2025 10:33

HDDS-13660. Ozone client support readVectored.

d789aef

Fix checkstyle error

272edca

jojochuang requested a review from sumitagrawl December 1, 2025 17:18

jojochuang self-requested a review December 1, 2025 17:19

adoroszlai reviewed Dec 4, 2025

View reviewed changes

adoroszlai and others added 2 commits December 4, 2025 18:07

add contract test

ba9f85e

(cherry picked from commit ee11cf3)

Fix review comments

a2eff55

yandrey321 reviewed Dec 4, 2025

View reviewed changes

Comment thread hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/MultipartInputStream.java Outdated

yandrey321 reviewed Dec 4, 2025

View reviewed changes

Comment thread hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/VectoredReadUtils.java

yandrey321 reviewed Dec 4, 2025

View reviewed changes

chungen0126 self-requested a review December 5, 2025 07:26

ashishkr200 and others added 2 commits December 9, 2025 14:00

Fix review comments

bc0bafd

Merge branch 'master' into testvector

34ad289

ashishkumar50 requested review from adoroszlai and yandrey321 January 2, 2026 04:18

chungen0126 reviewed Jan 15, 2026

View reviewed changes

github-actions Bot added the stale label Feb 6, 2026

yandrey321 suggested changes Feb 9, 2026

View reviewed changes

github-actions Bot removed the stale label Feb 10, 2026

github-actions Bot added the stale label Mar 4, 2026

github-actions Bot closed this Mar 12, 2026

Uh oh!

Conversation

ashishkumar50 commented Dec 1, 2025

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

jojochuang commented Dec 1, 2025

Uh oh!

adoroszlai commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ashishkumar50 commented Dec 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai commented Dec 17, 2025

Uh oh!

chungen0126 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Feb 6, 2026

Uh oh!

adoroszlai commented Feb 9, 2026

Uh oh!

yandrey321 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 4, 2026

Uh oh!

github-actions Bot commented Mar 12, 2026

Uh oh!

chungen0126 commented Jul 3, 2026

Uh oh!

adoroszlai commented Jul 3, 2026

Uh oh!

chungen0126 commented Jul 15, 2026

Benchmark Setup:

Test Scenarios:

Results:

Current Progress & Next Steps:

Uh oh!

yandrey321 commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

adoroszlai commented Dec 3, 2025 •

edited

Loading