Skip to content

fix(bigtable): enforce StreamController cancellation and flow control in MaybePointReadCallable - #14992

Open
igorbernstein2 wants to merge 5 commits into
mainfrom
fix-maybe-point-read-cancel-flow-control
Open

igorbernstein2 wants to merge 5 commits into
mainfrom
fix-maybe-point-read-cancel-flow-control

Conversation

@igorbernstein2

Copy link
Copy Markdown
Contributor

Summary

Fixes StreamController / ResponseObserver contract violations in MaybePointReadCallable:

  1. Propagate CancellationException to responseObserver.onError(...) when StreamController.cancel() is invoked prior to RPC completion (instead of suppressing terminal callbacks and deadlocking ServerStream.iterator().hasNext()).
  2. Check cancelled.get() after responseObserver.onResponse(row) so an in-callback controller.cancel() delivers onError(CancellationException) rather than onComplete().
  3. Implement disableAutoInboundFlowControl() and request(int) using a SettableApiFuture<Void> demand gate composed via ApiFutures.transformAsync.

@igorbernstein2
igorbernstein2 requested review from a team as code owners November 12, 2026 19:50

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces manual flow control and improves cancellation propagation in MaybePointReadCallable by utilizing a SettableApiFuture to gate row delivery and eagerly starting the underlying RPC. It also adds comprehensive unit tests to verify these behaviors. The review feedback highlights two important improvements: addressing a potential race condition where cancellation occurs before the future reference is set, and ensuring the cancellation state is checked at the beginning of onSuccess to prevent delivering responses or completing the stream after cancellation.

@igorbernstein2
igorbernstein2 enabled auto-merge (squash) November 13, 2026 15:55
@igorbernstein2 igorbernstein2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 13, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 13, 2026

This branch has not been deployed

No deployments
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.

3 participants