Use nested sessions rather than starting new connections on each transaction#4549
Use nested sessions rather than starting new connections on each transaction#4549
Conversation
|
|
||
| # If we're already in a session use a nested session rather than opening a new connection | ||
| if kwargs.get('bind') == getattr(req, 'db_connection', None) and req.db_connection.in_transaction(): | ||
| session.begin_nested() |
There was a problem hiding this comment.
Running begin_nested actually flushes the current session to the database along with a SAVEPOINT call:
When Session.begin_nested() is called, the Session first flushes all currently pending state to the database;
Since the intent of these calls is typically just to acquire a session without having one passed to the function, I'd say the better behavior would be to just return the current session. begin_nested can still easily be called anywhere we want to create savepoint/rollback behavior, such as when we run validations.
There was a problem hiding this comment.
It looks like running begin_nested effectively breaks our is_new property, which is used pretty extensively, so we definitely should not run this in the background.
This is a redux of #4549 with a slightly different approach: instead of always returning a 'nested' session (it's not actually nested -- more on that below), we just return the current session unless a flag is passed to specifically enable nesting. We add this flag to the few cases where a rollback is used in a `with Session() as session` block. True nested sessions are no longer supported in SQLAlchemy. Instead, when you run `session.begin_nested`, the session is flushed to the database along with the SAVEPOINT command. This allows you to run session.rollback() to go back to the savepoint. The problem is, we inspect session state for objects -- particularly via the `is_new` property. Since running session.begin_nested flushes the current session, `is_new` returns False for any objects that were pending when you made a 'nested' session. We don't want that in most cases, so we want to use nested sessions sparingly.
|
This branch has a bunch of opentelemetry stuff in it, so I copied the relevant code to #4552 to merge in! |
2 participants
Before this change, it's opening a whole new connection for each
with Session() as session:After this change it reuses the connection, and just opens a nested session:
