connection close() questions

[Kent]

Mike, et al.,

I've got some questions about closing connections.  I suspect my framework may be at fault, but there is potentially a sqlalchemy issue here as well.

See attached script with nested transaction and explicit connection.close().

Things are even more complex because versions have handled this differently in the past:

• on rel_0_9_1 and ealier, the conn.close() always actually emitted a DBAPI ROLLBACK, but on rel_0_9_2+, the previous call to .begin_nested() now prevents the DBAPI ROLLBACK call, even though the close() is on the connection itself.  I'm not sure if that was an intended change, but it seems .close() on a connection should always cause ROLLBACK, no?
• rel_1_3_9 and earlier this code raises sqlalchemy.exc.ResourceClosedError on the last DBSession.close() as it invokes the registered 'rollback' event with an already-closed connection, but on current master (1.4.0b1) there is no exception since a rollback isn't attempted, leaving the db connection in idle transaction.

On all versions since rel_0_9_1, even after both of the script's finally clauses (close() statements) but before the program terminates, the transaction is still left in transaction in the database, though the connection's been checked back into the pool.

As far as whether my code here is badly formed, my question is: is it wrong to mix session closing and connection closing or should that be fine?

(My actual application is obviously more complex, with zope.sqlalchemy & transaction and frameworks; I boiled it down to this script for demo purposes and removed those libraries, making this code look weirder.)

Thanks in advance!

Kent

[Mike Bayer]

Haven't looked deeply but so far what you need to know is that conn.close() *ALWAYS* rolls back the transaction, just not at the Engine level, it's at the connection pool level so you won't see it when logging / event hooking on the Engine.  turn on echo_pool and you will see this, in modern versions:

2020-05-13 11:25:45,106 DEBUG sqlalchemy.pool.impl.QueuePool Connection <connection object at 0x7f660c367b90; dsn: 'user=scott password=xxx dbname=test host=localhost', closed: 0> being returned to pool

2020-05-13 11:25:45,107 DEBUG sqlalchemy.pool.impl.QueuePool Connection <connection object at 0x7f660c367b90; dsn: 'user=scott password=xxx dbname=test host=localhost', closed: 0> rollback-on-return, via agent

your DBSession is not going to close the connection because you have it bound directly to that connection, rather than to the engine, so it assumes it is participating in a larger transaction.   1.4 does amend this behavior to be more clear cut as we are doing away with the "nested" behaviors of Connection.  So yes I would not be relying upon DBSession.close() as a means of transaction control if the session is bound to a connection directly.  If the session is bound to a connection I would advise ensuring that connection is in a transaction on the outside that you are managing.

--

SQLAlchemy -

The Python SQL Toolkit and Object Relational Mapper

 

http://www.sqlalchemy.org/

 

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example. See http://stackoverflow.com/help/mcve for a full description.

---

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/a9d73e26-84c6-4dcf-bb6d-82f541fa1f6d%40googlegroups.com.

Attachments:

• connection-close.py
  

[Kent Bower]

In this script, conn.close() does not call rollback on the transaction.  It isn't just a logging issue as I've verified from the database that the session was not rolled back.

You received this message because you are subscribed to a topic in the Google Groups "sqlalchemy" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sqlalchemy/Lit5HWFiC0U/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sqlalchemy+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/6f84e3a9-7d28-44e6-9e13-3f541aac95e4%40www.fastmail.com.

[Mike Bayer]

On Wed, May 13, 2020, at 11:39 AM, Kent Bower wrote:

In this script, conn.close() does not call rollback on the transaction.  It isn't just a logging issue as I've verified from the database that the session was not rolled back.

I can confirm that in master only where things have changed dramatically.   Should not be the case for any released version, please confirm

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/CANnF6aFqH7nrtDVd5CrchnoP%2Bz2pY-utazLAcQm%2BfBje785gTA%40mail.gmail.com.

[Mike Bayer]

nevermind, you've managed to find a case that trips it up for the connection pool

release today

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/b7ff30e4-722c-4324-98ca-6d1afa1693ba%40www.fastmail.com.

[Kent Bower]

LOL, you're welcome, I'm such a great tester, aren't I?

Anyway, everything after 0.9.2 behaved this way.  0.9.1 did a rollback.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/95ebb408-7bad-4c0e-a14b-b6fce9a4ada3%40www.fastmail.com.

[Mike Bayer]

feel free to test the patch at:

https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/1965

this patch includes that if the transaction state at the engine level gets screwed up, the pool will warn and still make sure it does a real rollback.    you should not see this warning however.

in 2.0, the whole "reset" logic is simplified so that none of this complexity will be there.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/CANnF6aETpA9v_qRrkZyaxxO9cJJQM%3Dy-yjjQ5VfKs0RS3yDCAg%40mail.gmail.com.

[Kent Bower]

Very good, will do when I find time.

Thank you!

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/f6e171e0-a4c9-4642-9823-d2fa559920ab%40www.fastmail.com.

[Mike Bayer]

this is getting released today in any case so, just look for any more warnings or conditions like this.  the most important part is getting the test coverage in so as I refactor for 1.4 / 2.0 the behavioral contract is maintained.  thanks!

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/CANnF6aHBBobuDveptaQvnSJxKj3v4ML0rO9bpp9onZ8pitZ_aw%40mail.gmail.com.

[Kent Bower]

Returned to pool in rolled back state now, thanks.

However, the script I sent in this post now hits the "SAWarning: Reset agent is not active.  This should not occur unless there was already a connectivity error in progress." on the conn.close() call.

Did you expect that because my usage pattern is "illegal" so to speak?  (The Warning isn't quite accurate regarding "... unless there was already a connectivity error in progress ")

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/c9928ece-aece-4265-9cf9-096143016126%40www.fastmail.com.

[Mike Bayer]

On Thu, May 14, 2020, at 8:35 AM, Kent Bower wrote:

Returned to pool in rolled back state now, thanks.

However, the script I sent in this post now hits the "SAWarning: Reset agent is not active.  This should not occur unless there was already a connectivity error in progress." on the conn.close() call.

Did you expect that because my usage pattern is "illegal" so to speak?  (The Warning isn't quite accurate regarding "... unless there was already a connectivity error in progress ")

it means it's still broken, unfortunately.    it's not supposed to do that now.    but it's not as much of an emergency since it is working around the problem.

i will try your standalone test case again.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/CANnF6aGKrCnFutPkHwcwV9s5yLj7NH83Tfspo6h06Nh5%2B4fAHQ%40mail.gmail.com.