Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors AsyncResultSetImpl to introduce an onStartOrClose callback, allowing callers like MultiUseReadOnlyTransaction to track the query lifecycle more reliably. It simplifies the transaction logic by removing the anonymous class override and tracking state internally within AsyncResultSetImpl using synchronized monitor flags (startOrCloseNotified, delegateInitialized, delegateClosed). Additionally, it ensures that calling next() after closing the result set throws an IllegalStateException, and adds comprehensive unit tests to verify these behaviors. There are no review comments, and the implementation looks solid, so I have no feedback to provide.
62b05bd to
aa1681b
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces an onStartOrClose callback to AsyncResultSetImpl to better track the query lifecycle, allowing callers like MultiUseReadOnlyTransaction to simplify pending start tracking. It also refactors delegate initialization and closing logic to prevent resource leaks and adds comprehensive unit tests. The review feedback highlights two important improvements: first, checking this.closed instead of delegateClosed in getDelegate() to prevent unnecessary RPC streams when closed immediately after setting a callback; second, wrapping the onStartOrClose callback execution in a try-catch block to avoid masking primary exceptions.
…sets Decouple multi-use read-only transactions from internal async result set implementation details by introducing a dedicated lifecycle callback. Key improvements in this change: - Replaces the anonymous subclass in read-only transactions with a clean callback that signals when an asynchronous query either begins execution or is closed without starting. - Lazily resolves the underlying query stream so that closing an async query before it begins execution no longer starts the query or opens an RPC stream, preventing unnecessary network calls and session allocations. - Permits safe inspection of query statistics and metadata after a result set has been closed. - Adds comprehensive unit tests for the decoupled lifecycle, post-close inspections, and closed-state error handling.
aa1681b to
7d470a9
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors AsyncResultSetImpl to introduce an onStartOrClose callback, allowing callers like MultiUseReadOnlyTransaction to track the query lifecycle more reliably. It simplifies the transaction logic by removing the custom anonymous subclass and replacing it with this callback. Additionally, the changes improve resource management, ensure proper delegate closing, add robust error handling for callback execution, and introduce comprehensive unit tests to verify these behaviors. No review comments were provided, so there is no feedback to address.
Decouple multi-use read-only transactions from internal async result set implementation details by introducing a dedicated lifecycle callback.
Key improvements in this change: