Repository navigation
refactor(spanner): add non-blocking polling to GrpcStreamIterator - #14696
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces non-blocking data availability checks (isDataAvailable and canPollWithoutErrorOrBlocking) to the Spanner result set iterator, along with a mechanism to buffer and flush stream messages to asynchronous listeners once the underlying gRPC call is initialized. Feedback on these changes highlights a critical race condition and out-of-order delivery risk in GrpcStreamIterator when flushing buffered messages concurrently with incoming stream activity. To ensure thread-safe, sequential, and duplicate-free delivery, the reviewer recommends serializing listener notifications using an explicit ReentrantLock and tracking a notification count.
Add non-blocking polling and readiness checks to CloseableIterator and
GrpcStreamIterator to allow consumers to drain buffered results without
blocking worker threads on network I/O.
Key changes:
- Non-blocking polling API on CloseableIterator:
- isDataAvailable(): Returns true if a chunk, EOF, or error is buffered
in memory and ready to read without waiting on network I/O.
- canPollWithoutErrorOrBlocking(): Returns true if hasNext() or next()
can be called immediately to yield data or EOF without throwing an error.
- Call initialization gating and race prevention:
- In asynchronous mode, message notifications are held until the underlying
StreamingCall is initialized via setCall(), preventing consumers from
processing rows before call context and cancellation hooks are set.
- Both setCall() and registerListener() flush buffered messages to the
listener, ensuring order-independent initialization.
- Documented StreamMessageListener with @internalapi, clarifying that
implementations must be thread-safe and idempotent.
- Test coverage:
- Added unit tests for interface defaults, buffer draining, EOF, delayed
pre-setCall error/completion, and multi-threaded concurrency races.
a8f09c3 to
9955f5e
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces isDataAvailable() and canPollWithoutErrorOrBlocking() to the CloseableIterator interface and implements them in GrpcStreamIterator to support non-blocking checks on stream data availability. It also refactors StreamMessageListener registration and message flushing to handle asynchronous initialization safely, supported by comprehensive unit tests. Feedback suggests synchronizing registerListener and the flush in setCall on a common lock to prevent inefficient, concurrent duplicate flushes of buffered messages.
Add non-blocking polling and readiness checks to CloseableIterator and GrpcStreamIterator to allow consumers to drain buffered results without blocking worker threads on network I/O.
Key changes:
Non-blocking polling API on CloseableIterator:
Call initialization gating and race prevention:
Test coverage: