Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors AsyncResultSetImpl to replace its blocking CountDownLatch-based synchronization model with a non-blocking, cooperatively scheduled state machine. This change ensures that executor threads (particularly single-threaded executors) are yielded instead of blocked when the buffer is full or when waiting for stream data, effectively preventing potential deadlocks. Additionally, nullability annotations have been integrated, and extensive unit and integration tests have been added to validate the new non-blocking behavior and various concurrency edge cases. I have no further feedback to provide as there are no review comments to assess.
Asynchronous queries share a bounded background thread pool to read rows from query streams and dispatch them to application callbacks. Previously, each active query held onto a background thread for its entire lifetime, parking that thread whenever the query was waiting for more data from the server, waiting for a full row buffer to be drained, or paused by the application callback. Running many concurrent or paused queries could exhaust the shared thread pool and stall other queries waiting for an available worker thread. This change switches asynchronous query execution to an event-driven model so background threads are only used while actively moving buffered rows: - Event-driven row production: Background tasks read rows only while stream data is already buffered locally and space remains in the row queue, releasing the worker thread immediately whenever a query is waiting on the network, the row buffer is full, or consumption is paused. Incoming stream data, buffer consumption, and resume calls re-schedule row production as needed. - Serialized stream startup and consumption: Completes stream initialization before row production begins so underlying query iterators are never accessed concurrently across threads, while preserving startup error translation for queries that inline transaction creation. - Prompt cancellation and clean lifecycle termination: Delivers cancellation directly to paused or in-flight callbacks without requiring a separate resume call, closes underlying streams outside internal locks, and waits for all active tasks to finish before completing the result future and releasing executor resources. - Strict callback thread scoping and interrupt handling: Restricts cursor advancement and row reads in asynchronous mode to the active callback invocation thread, and cancels the query while preserving the thread interrupt status if a blocking list conversion is interrupted.
a546877 to
6efbee2
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors AsyncResultSetImpl to implement a non-blocking, event-driven streaming architecture. It replaces the blocking queue with an ArrayDeque and introduces explicit state tracking to allow producer and callback tasks to yield and reschedule on the executor rather than pinning threads when the buffer is full or waiting for stream data. The changes also improve state validation, implement schema methods directly on the result set, correct exception caching during delegate initialization, and add comprehensive concurrency and lifecycle tests. I have no feedback to provide as there are no review comments.
Asynchronous queries share a bounded background thread pool to read rows from query streams and dispatch them to application callbacks. Previously, each active query held onto a background thread for its entire lifetime, parking that thread whenever the query was waiting for more data from the server, waiting for a full row buffer to be drained, or paused by the application callback. Running many concurrent or paused queries could exhaust the shared thread pool and stall other queries waiting for an available worker thread.
This change switches asynchronous query execution to an event-driven model so background threads are only used while actively moving buffered rows: