Skip to content

HDDS-16323. StreamBlockInputStream: refill the pre-read window in bulk instead of once per response - #11159

Open
ss77892 wants to merge 2 commits into
apache:masterfrom
ss77892:HDDS-16323
Open

HDDS-16323. StreamBlockInputStream: refill the pre-read window in bulk instead of once per response#11159
ss77892 wants to merge 2 commits into
apache:masterfrom
ss77892:HDDS-16323

Conversation

@ss77892

@ss77892 ss77892 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

StreamBlockInputStream: refill the pre-read window in bulk instead of once per response

This change adds an early return in readBlock(): when the caller's read is already covered by what has been requested (required <= 0) and the outstanding pre-read window is still at least half full, no new request is sent. The window is refilled in bulk only once it drains below half. When required > 0 the request is always sent, so a read that needs bytes beyond the requested range is never starved.

A unit test (testPreReadWindowIsRefilledInBulk) reads a block one response at a time, as KeyInputStream does, and checks that the number of ReadBlock requests is bounded by the number of half windows and that every request carries at least half a window.

What is the link to the Apache JIRA

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

How was this patch tested?

UT added/ CI
Performance evaluation on 3 node cluster.
HDDS-16323.pdf

@chihsuan chihsuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch! @ss77892 +1, this looks good to me. 👍

final long preReadLength = preRead ? preReadSize : 0;
// Refill the pre-read window in bulk once it drains below half, instead of after every response.
// Safe to skip: required <= 0 means the DataNode still owes bytes that poll() is waiting for.
if (required <= 0 && requestedLength - position >= preReadLength / 2) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Just a thought, what do you think about giving the / 2 a name? A named constant might make the half-window trigger easier to spot later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I've added preReadRefillThreshold that is computed alongside preReadSize, so the test can reference the same value rather than repeating the / 2.

@chungen0126
chungen0126 self-requested a review August 31, 2026 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants