Skip to content

refactor(spanner): buffer chunks and add non-blocking isDataAvailable to GrpcValueIterator - #14699

Open
olavloite wants to merge 1 commit into
mainfrom
spanner-buffer-chunks
Open

olavloite wants to merge 1 commit into
mainfrom
spanner-buffer-chunks

Conversation

@olavloite

Copy link
Copy Markdown
Contributor

Refactor GrpcValueIterator and GrpcResultSet to support non-blocking data availability checks and bounded chunk buffering. This is Step 2 of the asynchronous stream refactoring plan, allowing reactive and async result set consumers to verify whether a full row is available in memory without blocking the calling thread on network I/O.

Summary of changes:

  • Bounded chunk buffering: GrpcValueIterator maintains an internal queue of PartialResultSet chunks, buffering only as many chunks as needed to assemble at least one complete row (including handling chunked string and list merges across multiple chunks).
  • Non-blocking isDataAvailable(): Implemented isDataAvailable() on GrpcResultSet and GrpcValueIterator. Polling uses non-blocking tryLock() coordination so checking data availability never blocks behind concurrent worker threads reading from the stream.
  • Zero-allocation hot path: Optimizes row availability checks to avoid heap allocations on the steady-state path when rows are already available in the current chunk.

Bug fixes and safety improvements included:

  • Fix getMetadata() idempotency: Calling getMetadata() multiple times or mid-stream now returns cached metadata immediately without erroneously advancing the stream or discarding chunks.
  • Prevent state resurrection on cancellation: If close() is invoked while a background stream.next() call is in flight, late-arriving chunks are safely discarded rather than repopulating state on a closed iterator.
  • Immediate surfacing of malformed metadata: Streams returning an initial chunk without valid metadata or row types immediately signal isDataAvailable() so the internal error is thrown rather than hanging the caller.
  • Clean up unused error field in GrpcResultSet.
  • Added comprehensive unit tests covering all chunked merge permutations, 0-column queries, error paths, and deterministic concurrent access.

… to GrpcValueIterator

Refactor GrpcValueIterator and GrpcResultSet to support non-blocking data availability checks and bounded chunk buffering. This is Step 2 of the asynchronous stream refactoring plan, allowing reactive and async result set consumers to verify whether a full row is available in memory without blocking the calling thread on network I/O.

Summary of changes:
- Bounded chunk buffering: GrpcValueIterator maintains an internal queue of PartialResultSet chunks, buffering only as many chunks as needed to assemble at least one complete row (including handling chunked string and list merges across multiple chunks).
- Non-blocking isDataAvailable(): Implemented isDataAvailable() on GrpcResultSet and GrpcValueIterator. Polling uses non-blocking tryLock() coordination so checking data availability never blocks behind concurrent worker threads reading from the stream.
- Zero-allocation hot path: Optimizes row availability checks to avoid heap allocations on the steady-state path when rows are already available in the current chunk.

Bug fixes and safety improvements included:
- Fix getMetadata() idempotency: Calling getMetadata() multiple times or mid-stream now returns cached metadata immediately without erroneously advancing the stream or discarding chunks.
- Prevent state resurrection on cancellation: If close() is invoked while a background stream.next() call is in flight, late-arriving chunks are safely discarded rather than repopulating state on a closed iterator.
- Immediate surfacing of malformed metadata: Streams returning an initial chunk without valid metadata or row types immediately signal isDataAvailable() so the internal error is thrown rather than hanging the caller.
- Clean up unused error field in GrpcResultSet.
- Added comprehensive unit tests covering all chunked merge permutations, 0-column queries, error paths, and deterministic concurrent access.
@olavloite
olavloite requested review from a team as code owners October 20, 2026 14:29

@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 a non-blocking isDataAvailable() mechanism to GrpcResultSet and GrpcValueIterator to determine if a complete row, EOF, or error is available without blocking on network I/O. To support this, GrpcValueIterator has been refactored to buffer incoming chunks non-blockingly using a ReentrantLock and a Deque of PartialResultSet chunks, with several fields marked as volatile for thread safety. Additionally, comprehensive unit tests have been added in GrpcResultSetTest and a new GrpcValueIteratorTest class to verify the correctness of isDataAvailable() under various streaming, chunking, and concurrent scenarios. There are no review comments, so we have no further feedback to provide.

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.

1 participant