chore(spanner): wait until queries have started before closing ReadContext#12727
chore(spanner): wait until queries have started before closing ReadContext#12727
Conversation
…ntext Prevents a ReadContext from being closed before all queries have actually started. This extra check is needed now that queries in read-only transactions are started using the background executor when the async API is used.
olavloite
force-pushed
the
spanner-wait-for-queries-to-start-before-close
branch
from
fe4c184 to
6a2c9db
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the locking mechanism in AbstractReadContext.java by replacing the intrinsic object lock with a ReentrantLock and a Condition variable to manage pending asynchronous operations. It also introduces an AtomicInteger to track pending starts and ensures that the close() method waits for all pending operations to complete. I have reviewed the implementation and agree with the feedback provided regarding a potential resource leak: if the AsyncResultSetImpl constructor throws an exception, the pendingStarts counter will not be decremented, causing the close() method to hang. The suggested fix to wrap the constructor in a try-catch block is necessary to ensure the counter is correctly decremented in failure scenarios.
...spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java
Outdated
Show resolved
Hide resolved
olavloite
force-pushed
the
spanner-wait-for-queries-to-start-before-close
branch
from
abdf020 to
92e7777
Compare
olavloite
force-pushed
the
spanner-wait-for-queries-to-start-before-close
branch
from
92e7777 to
fb98e5d
Compare
1 participant
Prevents a ReadContext from being closed before all queries have actually started. This extra check is needed now that queries in read-only transactions are started using the background executor when the async API is used.
Follow-up for #12715