Repository navigation
database.read and database.execute_sql should not return the session to the pool till the stream is closed #3768
Description
Cloud Spanner API says that at any point in time there can be at most one active transaction. But the way database.read and database.execute_query have been implemented, they return the session to the pool immediately. Now that session can be reused for doing something else even before the stream returned by previous database.read has completed. This needs to be fixed so that the sessions is returned to the pool only after the iterator has been completely consumed or closed.
This needs to be done before Beta.
Activity
vkedia commentedon Aug 8, 2017
cc @lukesneeringer @bjwatson
bjwatson commentedon Aug 8, 2017
Thanks @vkedia. At a 100K' level, this does sound like a significant usability issue. I'm okay with this being Beta blocking, unless @lukesneeringer or @tseaver have an opinion to the contrary.
vkedia commentedon Aug 8, 2017
@bjwatson This also might be a breaking change depending on how this resolved. For eg the easiest resolution would be to just remove these methods since one can always do the same thing by using a Snapshot.
bjwatson commentedon Aug 8, 2017
@vkedia That sounds like an easy fix, but definitely surface breaking.
tseaver commentedon Aug 9, 2017
I don't think using
Database.snapshotsolves the problem. Using the current methods:versus using a snapshot:
The 'StreamedResultSet' we return wraps a low-level GAPIC entity with no existing / obvious link to our session. Given that the user may not iterate to the end (or at all), we would likely have to had some machinery for a "deferred return to pool" and trigger that from
StreamedResultSet.__del__.The other option would be to require that user applications complete all iteration over the
StreamedResultSetinside thewith database.snapshot() as snapshot:block.vkedia commentedon Aug 9, 2017
It seems fair to me to expect that user will not access the results object outside of the context block. We can possibly do something to invalidate the iterators when the snapshot is checked back in.
tseaver commentedon Aug 10, 2017
Via #3787.