HDDS-16324. XceiverClientGrpc.streamRead should wait for flow-control readiness via onReadyHandler instead of a 10 ms poll - #11195
Open
ss77892 wants to merge 1 commit into
Open
HDDS-16324. XceiverClientGrpc.streamRead should wait for flow-control readiness via onReadyHandler instead of a 10 ms poll#11195ss77892 wants to merge 1 commit into
ss77892 wants to merge 1 commit into
Conversation
… readiness via onReadyHandler instead of a 10 ms poll Co-authored-by: Claude Opus 5.0
Contributor
Author
|
The failure is not related to the patch. |
chihsuan
reviewed
Sep 6, 2026
| throw new TimeoutIOException("Timed out waiting for stream to become ready: " + streamObserver); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| throw new InterruptedIOException("Interrupted while waiting for stream to become ready: " + streamObserver); |
Contributor
There was a problem hiding this comment.
nit: this line predates the PR, but awaitReady() now gives us a real InterruptedException to work with. Would it be worth preserving the interrupt cause here?
| final AtomicBoolean result = new AtomicBoolean(); | ||
| final Thread waiter = new Thread(() -> { | ||
| try { | ||
| waiting.countDown(); |
Contributor
There was a problem hiding this comment.
I wonder if there's a race here. countDown() fires before the waiter calls awaitReady(), so this could pass even with a broken notifyAll() if the main thread wins that race. It may be helpful to add a spurious notification case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
XceiverClientGrpc.streamRead should wait for flow-control readiness via onReadyHandler instead of a 10 ms poll
XceiverClientGrpc.streamRead() waits for the gRPC request stream to become ready (flow control) by polling isReady() in a loop with a 10 ms sleep between checks. This adds up to 10 ms of latency to every send that hits back-pressure, burns CPU on a busy client, and keeps the caller waiting for the full timeout if the call fails in the meantime, since a terminated stream never becomes ready.
PR uses the readiness callback that gRPC already provides instead of polling. initStreamRead() now wraps the reader in a ClientResponseObserver so the onReadyHandler can be registered in beforeStart(), the only place gRPC allows it. The handler wakes the sender via StreamingReadResponse.signalReady().streamRead() calls the new StreamingReadResponse.awaitReady(timeout), which blocks on a monitor until the stream is ready, the timeout expires, or the call terminates. onError/onCompleted call signalTerminated(), so a sender blocked in awaitReady() fails immediately with the termination cause instead of waiting out the timeout.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16324
How was this patch tested?
UTs has been added.