diff options
| -rw-r--r-- | doc/build/changelog/unreleased_14/8717.rst | 18 | ||||
| -rw-r--r-- | doc/build/changelog/unreleased_20/8717.rst | 20 | ||||
| -rw-r--r-- | doc/build/core/events.rst | 3 | ||||
| -rw-r--r-- | doc/build/core/pooling.rst | 173 | ||||
| -rw-r--r-- | lib/sqlalchemy/__init__.py | 1 | ||||
| -rw-r--r-- | lib/sqlalchemy/dialects/mssql/base.py | 55 | ||||
| -rw-r--r-- | lib/sqlalchemy/dialects/postgresql/base.py | 64 | ||||
| -rw-r--r-- | lib/sqlalchemy/engine/base.py | 4 | ||||
| -rw-r--r-- | lib/sqlalchemy/engine/create.py | 2 | ||||
| -rw-r--r-- | lib/sqlalchemy/event/legacy.py | 16 | ||||
| -rw-r--r-- | lib/sqlalchemy/events.py | 1 | ||||
| -rw-r--r-- | lib/sqlalchemy/pool/__init__.py | 1 | ||||
| -rw-r--r-- | lib/sqlalchemy/pool/base.py | 178 | ||||
| -rw-r--r-- | lib/sqlalchemy/pool/events.py | 43 | ||||
| -rw-r--r-- | test/engine/test_deprecations.py | 58 | ||||
| -rw-r--r-- | test/engine/test_logging.py | 20 | ||||
| -rw-r--r-- | test/engine/test_pool.py | 133 | ||||
| -rw-r--r-- | test/profiles.txt | 24 |
18 files changed, 688 insertions, 126 deletions
diff --git a/doc/build/changelog/unreleased_14/8717.rst b/doc/build/changelog/unreleased_14/8717.rst new file mode 100644 index 000000000..ee5fa5949 --- /dev/null +++ b/doc/build/changelog/unreleased_14/8717.rst @@ -0,0 +1,18 @@ +.. change:: + :tags: bug, engine, regression + :tickets: 8717 + + Fixed issue where the :meth:`.PoolEvents.reset` event hook would not be + called when a :class:`.Connection` were closed which already called + ``.rollback()`` on its own transaction, due to an enhancement in the 1.4 + series that ensures ``.rollback()`` is only called once in this scenario, + rather than twice. This would prevent custom pool reset schemes from being + used within this hook. This was a regression that appeared in version 1.4. + + For version 1.4, the :meth:`.PoolEvents.checkin` likely remains a better + event to use for custom "reset" implementations. Version 2.0 will feature + an improved version of :meth:`.PoolEvents.reset` which is called for + additional scenarios such as termination of asyncio connections, and is + also passed contextual information about the reset, to allow for "custom + connection reset" schemes which can respond to different reset scenarios in + different ways. diff --git a/doc/build/changelog/unreleased_20/8717.rst b/doc/build/changelog/unreleased_20/8717.rst new file mode 100644 index 000000000..806759627 --- /dev/null +++ b/doc/build/changelog/unreleased_20/8717.rst @@ -0,0 +1,20 @@ +.. change:: + :tags: usecase, engine + :tickets: 8717 + + Added new parameter :paramref:`.PoolEvents.reset.reset_state` parameter to + the :meth:`.PoolEvents.reset` event, with deprecation logic in place that + will continue to accept event hooks using the previous set of arguments. + This indicates various state information about how the reset is taking + place and is used to allow custom reset schemes to take place with full + context given. + + Within this change a fix that's also backported to 1.4 is included which + re-enables the :meth:`.PoolEvents.reset` event to continue to take place + under all circumstances, including when :class:`.Connection` has already + "reset" the connection. + + The two changes together allow custom reset schemes to be implemented using + the :meth:`.PoolEvents.reset` event, instead of the + :meth:`.PoolEvents.checkin` event (which continues to function as it always + has). diff --git a/doc/build/core/events.rst b/doc/build/core/events.rst index 4452ae7f5..364552807 100644 --- a/doc/build/core/events.rst +++ b/doc/build/core/events.rst @@ -17,6 +17,9 @@ Connection Pool Events .. autoclass:: sqlalchemy.events.PoolEvents :members: +.. autoclass:: sqlalchemy.events.PoolResetState + :members: + .. _core_sql_events: SQL Execution and Connection Events diff --git a/doc/build/core/pooling.rst b/doc/build/core/pooling.rst index cc19e96af..1121919db 100644 --- a/doc/build/core/pooling.rst +++ b/doc/build/core/pooling.rst @@ -123,43 +123,124 @@ however and in particular is not supported with asyncio DBAPI drivers. Reset On Return --------------- -The pool also includes the a "reset on return" feature which will call the -``rollback()`` method of the DBAPI connection when the connection is returned -to the pool. This is so that any existing -transaction on the connection is removed, not only ensuring that no existing -state remains on next usage, but also so that table and row locks are released -as well as that any isolated data snapshots are removed. This ``rollback()`` -occurs in most cases even when using an :class:`_engine.Engine` object, -except in the case when the :class:`_engine.Connection` can guarantee -that a ``rollback()`` has been called immediately before the connection -is returned to the pool. - -For most DBAPIs, the call to ``rollback()`` is very inexpensive and if the +The pool includes "reset on return" behavior which will call the ``rollback()`` +method of the DBAPI connection when the connection is returned to the pool. +This is so that any existing transactional state is removed from the +connection, which includes not just uncommitted data but table and row locks as +well. For most DBAPIs, the call to ``rollback()`` is inexpensive, and if the DBAPI has already completed a transaction, the method should be a no-op. -However, for DBAPIs that incur performance issues with ``rollback()`` even if -there's no state on the connection, this behavior can be disabled using the -``reset_on_return`` option of :class:`_pool.Pool`. The behavior is safe -to disable under the following conditions: - -* If the database does not support transactions at all, such as using - MySQL with the MyISAM engine, or the DBAPI is used in autocommit - mode only, the behavior can be disabled. -* If the pool itself doesn't maintain a connection after it's checked in, - such as when using :class:`.NullPool`, the behavior can be disabled. -* Otherwise, it must be ensured that: - - * the application ensures that all :class:`_engine.Connection` - objects are explicitly closed out using a context manager (i.e. ``with`` - block) or a ``try/finally`` style block - * connections are never allowed to be garbage collected before being explicitly - closed. - * the DBAPI connection itself, e.g. ``connection.connection``, is not used - directly, or the application ensures that ``.rollback()`` is called - on this connection before releasing it back to the connection pool. - -The "reset on return" step may be logged using the ``logging.DEBUG`` + + +Disabling Reset on Return for non-transactional connections +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +For very specific cases where this ``rollback()`` is not useful, such as when +using a connection that is configured for +:ref:`autocommit <dbapi_autocommit_understanding>` or when using a database +that has no ACID capabilities such as the MyISAM engine of MySQL, the +reset-on-return behavior can be disabled, which is typically done for +performance reasons. This can be affected by using the +:paramref:`_pool.Pool.reset_on_return` parameter of :class:`_pool.Pool`, which +is also available from :func:`_sa.create_engine` as +:paramref:`_sa.create_engine.pool_reset_on_return`, passing a value of ``None``. +This is illustrated in the example below, in conjunction with the +:paramref:`.create_engine.isolation_level` parameter setting of +``AUTOCOMMIT``:: + + non_acid_engine = create_engine( + "mysql://scott:tiger@host/db", + pool_reset_on_return=None, + isolation_level="AUTOCOMMIT", + ) + +The above engine won't actually perform ROLLBACK when connections are returned +to the pool; since AUTOCOMMIT is enabled, the driver will also not perform +any BEGIN operation. + + +Custom Reset-on-Return Schemes +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +"reset on return" consisting of a single ``rollback()`` may not be sufficient +for some use cases; in particular, applications which make use of temporary +tables may wish for these tables to be automatically removed on connection +checkin. Some (but notably not all) backends include features that can "reset" +such tables within the scope of a database connection, which may be a desirable +behavior for connection pool reset. Other server resources such as prepared +statement handles and server-side statement caches may persist beyond the +checkin process, which may or may not be desirable, depending on specifics. +Again, some (but again not all) backends may provide for a means of resetting +this state. The two SQLAlchemy included dialects which are known to have +such reset schemes include Microsoft SQL Server, where an undocumented but +widely known stored procedure called ``sp_reset_connection`` is often used, +and PostgreSQL, which has a well-documented series of commands including +``DISCARD`` ``RESET``, ``DEALLOCATE``, and ``UNLISTEN``. + +.. note: next paragraph + example should match mssql/base.py example + +The following example illustrates how to replace reset on return with the +Microsoft SQL Server ``sp_reset_connection`` stored procedure, using the +:meth:`.PoolEvents.reset` event hook. The +:paramref:`_sa.create_engine.pool_reset_on_return` parameter is set to ``None`` +so that the custom scheme can replace the default behavior completely. The +custom hook implementation calls ``.rollback()`` in any case, as it's usually +important that the DBAPI's own tracking of commit/rollback will remain +consistent with the state of the transaction:: + + from sqlalchemy import create_engine + from sqlalchemy import event + + mssql_engine = create_engine( + "mssql+pyodbc://scott:tiger^5HHH@mssql2017:1433/test?driver=ODBC+Driver+17+for+SQL+Server", + # disable default reset-on-return scheme + pool_reset_on_return=None, + ) + + + @event.listens_for(mssql_engine, "reset") + def _reset_mssql(dbapi_connection, connection_record, reset_state): + if not reset_state.terminate_only: + dbapi_connection.execute("{call sys.sp_reset_connection}") + + # so that the DBAPI itself knows that the connection has been + # reset + dbapi_connection.rollback() + +.. versionchanged:: 2.0.0b3 Added additional state arguments to + the :meth:`.PoolEvents.reset` event and additionally ensured the event + is invoked for all "reset" occurrences, so that it's appropriate + as a place for custom "reset" handlers. Previous schemes which + use the :meth:`.PoolEvents.checkin` handler remain usable as well. + +.. seealso:: + + * :ref:`mssql_reset_on_return` - in the :ref:`mssql_toplevel` documentation + * :ref:`postgresql_reset_on_return` in the :ref:`postgresql_toplevel` documentation + + + + +Logging reset-on-return events +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Logging for pool events including reset on return can be set +``logging.DEBUG`` log level along with the ``sqlalchemy.pool`` logger, or by setting -``echo_pool='debug'`` with :func:`_sa.create_engine`. +:paramref:`_sa.create_engine.echo_pool` to ``"debug"`` when using +:func:`_sa.create_engine`:: + + >>> from sqlalchemy import create_engine + >>> engine = create_engine("postgresql://scott:tiger@localhost/test", echo_pool="debug") + +The above pool will show verbose logging including reset on return:: + + >>> c1 = engine.connect() + DEBUG sqlalchemy.pool.impl.QueuePool Created new connection <connection object ...> + DEBUG sqlalchemy.pool.impl.QueuePool Connection <connection object ...> checked out from pool + >>> c1.close() + DEBUG sqlalchemy.pool.impl.QueuePool Connection <connection object ...> being returned to pool + DEBUG sqlalchemy.pool.impl.QueuePool Connection <connection object ...> rollback-on-return + Pool Events ----------- @@ -607,32 +688,22 @@ API Documentation - Available Pool Implementations -------------------------------------------------- .. autoclass:: sqlalchemy.pool.Pool - - .. automethod:: __init__ - .. automethod:: connect - .. automethod:: dispose - .. automethod:: recreate + :members: .. autoclass:: sqlalchemy.pool.QueuePool - - .. automethod:: __init__ - .. automethod:: connect + :members: .. autoclass:: SingletonThreadPool - - .. automethod:: __init__ + :members: .. autoclass:: AssertionPool - - .. automethod:: __init__ + :members: .. autoclass:: NullPool - - .. automethod:: __init__ + :members: .. autoclass:: StaticPool - - .. automethod:: __init__ + :members: .. autoclass:: ManagesConnection :members: diff --git a/lib/sqlalchemy/__init__.py b/lib/sqlalchemy/__init__.py index 5823239dc..b22d7ca8c 100644 --- a/lib/sqlalchemy/__init__.py +++ b/lib/sqlalchemy/__init__.py @@ -52,6 +52,7 @@ from .pool import ( from .pool import NullPool as NullPool from .pool import Pool as Pool from .pool import PoolProxiedConnection as PoolProxiedConnection +from .pool import PoolResetState as PoolResetState from .pool import QueuePool as QueuePool from .pool import SingletonThreadPool as SingleonThreadPool from .pool import StaticPool as StaticPool diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py index 5d42d98e3..a338ba27a 100644 --- a/lib/sqlalchemy/dialects/mssql/base.py +++ b/lib/sqlalchemy/dialects/mssql/base.py @@ -480,6 +480,61 @@ different isolation level settings. See the discussion at :ref:`dbapi_autocommit` +.. _mssql_reset_on_return: + +Temporary Table / Resource Reset for Connection Pooling +------------------------------------------------------- + +The :class:`.QueuePool` connection pool implementation used +by the SQLAlchemy :class:`.Engine` object includes +:ref:`reset on return <pool_reset_on_return>` behavior that will invoke +the DBAPI ``.rollback()`` method when connections are returned to the pool. +While this rollback will clear out the immediate state used by the previous +transaction, it does not cover a wider range of session-level state, including +temporary tables as well as other server state such as prepared statement +handles and statement caches. An undocumented SQL Server procedure known +as ``sp_reset_connection`` is known to be a workaround for this issue which +will reset most of the session state that builds up on a connection, including +temporary tables. + +To install ``sp_reset_connection`` as the means of performing reset-on-return, +the :meth:`.PoolEvents.reset` event hook may be used, as demonstrated in the +example below. The :paramref:`_sa.create_engine.pool_reset_on_return` parameter +is set to ``None`` so that the custom scheme can replace the default behavior +completely. The custom hook implementation calls ``.rollback()`` in any case, +as it's usually important that the DBAPI's own tracking of commit/rollback +will remain consistent with the state of the transaction:: + + from sqlalchemy import create_engine + from sqlalchemy import event + + mssql_engine = create_engine( + "mssql+pyodbc://scott:tiger^5HHH@mssql2017:1433/test?driver=ODBC+Driver+17+for+SQL+Server", + + # disable default reset-on-return scheme + pool_reset_on_return=None, + ) + + + @event.listens_for(mssql_engine, "reset") + def _reset_mssql(dbapi_connection, connection_record, reset_state): + if not reset_state.terminate_only: + dbapi_connection.execute("{call sys.sp_reset_connection}") + + # so that the DBAPI itself knows that the connection has been + # reset + dbapi_connection.rollback() + +.. versionchanged:: 2.0.0b3 Added additional state arguments to + the :meth:`.PoolEvents.reset` event and additionally ensured the event + is invoked for all "reset" occurrences, so that it's appropriate + as a place for custom "reset" handlers. Previous schemes which + use the :meth:`.PoolEvents.checkin` handler remain usable as well. + +.. seealso:: + + :ref:`pool_reset_on_return` - in the :ref:`pooling_toplevel` documentation + Nullability ----------- MSSQL has support for three levels of column nullability. The default diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py index 5eab8f47c..482e36594 100644 --- a/lib/sqlalchemy/dialects/postgresql/base.py +++ b/lib/sqlalchemy/dialects/postgresql/base.py @@ -233,6 +233,70 @@ SERIALIZABLE isolation. .. versionadded:: 1.4 added support for the ``postgresql_readonly`` and ``postgresql_deferrable`` execution options. +.. _postgresql_reset_on_return: + +Temporary Table / Resource Reset for Connection Pooling +------------------------------------------------------- + +The :class:`.QueuePool` connection pool implementation used +by the SQLAlchemy :class:`.Engine` object includes +:ref:`reset on return <pool_reset_on_return>` behavior that will invoke +the DBAPI ``.rollback()`` method when connections are returned to the pool. +While this rollback will clear out the immediate state used by the previous +transaction, it does not cover a wider range of session-level state, including +temporary tables as well as other server state such as prepared statement +handles and statement caches. The PostgreSQL database includes a variety +of commands which may be used to reset this state, including +``DISCARD``, ``RESET``, ``DEALLOCATE``, and ``UNLISTEN``. + + +To install +one or more of these commands as the means of performing reset-on-return, +the :meth:`.PoolEvents.reset` event hook may be used, as demonstrated +in the example below. The implementation +will end transactions in progress as well as discard temporary tables +using the ``CLOSE``, ``RESET`` and ``DISCARD`` commands; see the PostgreSQL +documentation for background on what each of these statements do. + +The :paramref:`_sa.create_engine.pool_reset_on_return` parameter +is set to ``None`` so that the custom scheme can replace the default behavior +completely. The custom hook implementation calls ``.rollback()`` in any case, +as it's usually important that the DBAPI's own tracking of commit/rollback +will remain consistent with the state of the transaction:: + + + from sqlalchemy import create_engine + from sqlalchemy import event + + postgresql_engine = create_engine( + "postgresql+pyscopg2://scott:tiger@hostname/dbname", + + # disable default reset-on-return scheme + pool_reset_on_return=None, + ) + + + @event.listens_for(postgresql_engine, "reset") + def _reset_mssql(dbapi_connection, connection_record, reset_state): + if not reset_state.terminate_only: + dbapi_connection.execute("CLOSE ALL") + dbapi_connection.execute("RESET ALL") + dbapi_connection.execute("DISCARD TEMP") + + # so that the DBAPI itself knows that the connection has been + # reset + dbapi_connection.rollback() + +.. versionchanged:: 2.0.0b3 Added additional state arguments to + the :meth:`.PoolEvents.reset` event and additionally ensured the event + is invoked for all "reset" occurrences, so that it's appropriate + as a place for custom "reset" handlers. Previous schemes which + use the :meth:`.PoolEvents.checkin` handler remain usable as well. + +.. seealso:: + + :ref:`pool_reset_on_return` - in the :ref:`pooling_toplevel` documentation + .. _postgresql_alternate_search_path: Setting Alternate Search Paths on Connect diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index 1ec307297..8cbc0163c 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -1220,7 +1220,9 @@ class Connection(ConnectionEventsTarget, inspection.Inspectable["Inspector"]): # as we just closed the transaction, close the connection # pool connection without doing an additional reset if skip_reset: - cast("_ConnectionFairy", conn)._close_no_reset() + cast("_ConnectionFairy", conn)._close_special( + transaction_reset=True + ) else: conn.close() diff --git a/lib/sqlalchemy/engine/create.py b/lib/sqlalchemy/engine/create.py index a9b388d71..ef5463972 100644 --- a/lib/sqlalchemy/engine/create.py +++ b/lib/sqlalchemy/engine/create.py @@ -469,7 +469,7 @@ def create_engine(url: Union[str, "_url.URL"], **kwargs: Any) -> Engine: .. seealso:: - :paramref:`_pool.Pool.reset_on_return` + :ref:`pool_reset_on_return` :param pool_timeout=30: number of seconds to wait before giving up on getting a connection from the pool. This is only used diff --git a/lib/sqlalchemy/event/legacy.py b/lib/sqlalchemy/event/legacy.py index 8ae9b7035..3d4341410 100644 --- a/lib/sqlalchemy/event/legacy.py +++ b/lib/sqlalchemy/event/legacy.py @@ -194,9 +194,9 @@ def _version_signature_changes( ) -> str: since, args, conv = dispatch_collection.legacy_signatures[0] return ( - "\n.. deprecated:: %(since)s\n" - " The :class:`.%(clsname)s.%(event_name)s` event now accepts the \n" - " arguments ``%(named_event_arguments)s%(has_kw_arguments)s``.\n" + "\n.. versionchanged:: %(since)s\n" + " The :meth:`.%(clsname)s.%(event_name)s` event now accepts the \n" + " arguments %(named_event_arguments)s%(has_kw_arguments)s.\n" " Support for listener functions which accept the previous \n" ' argument signature(s) listed above as "deprecated" will be \n' " removed in a future release." @@ -204,7 +204,15 @@ def _version_signature_changes( "since": since, "clsname": parent_dispatch_cls.__name__, "event_name": dispatch_collection.name, - "named_event_arguments": ", ".join(dispatch_collection.arg_names), + "named_event_arguments": ", ".join( + ":paramref:`.%(clsname)s.%(event_name)s.%(param_name)s`" + % { + "clsname": parent_dispatch_cls.__name__, + "event_name": dispatch_collection.name, + "param_name": param_name, + } + for param_name in dispatch_collection.arg_names + ), "has_kw_arguments": ", **kw" if dispatch_collection.has_kw else "", } ) diff --git a/lib/sqlalchemy/events.py b/lib/sqlalchemy/events.py index 4a8a52337..4e81ceec5 100644 --- a/lib/sqlalchemy/events.py +++ b/lib/sqlalchemy/events.py @@ -11,6 +11,7 @@ from __future__ import annotations from .engine.events import ConnectionEvents from .engine.events import DialectEvents +from .pool import PoolResetState from .pool.events import PoolEvents from .sql.base import SchemaEventTarget from .sql.events import DDLEvents diff --git a/lib/sqlalchemy/pool/__init__.py b/lib/sqlalchemy/pool/__init__.py index c86d3dded..8eb2ae4ab 100644 --- a/lib/sqlalchemy/pool/__init__.py +++ b/lib/sqlalchemy/pool/__init__.py @@ -29,6 +29,7 @@ from .base import ConnectionPoolEntry as ConnectionPoolEntry from .base import ManagesConnection as ManagesConnection from .base import Pool as Pool from .base import PoolProxiedConnection as PoolProxiedConnection +from .base import PoolResetState as PoolResetState from .base import reset_commit as reset_commit from .base import reset_none as reset_none from .base import reset_rollback as reset_rollback diff --git a/lib/sqlalchemy/pool/base.py b/lib/sqlalchemy/pool/base.py index 41f2f03b2..18ff66e5d 100644 --- a/lib/sqlalchemy/pool/base.py +++ b/lib/sqlalchemy/pool/base.py @@ -13,6 +13,7 @@ from __future__ import annotations from collections import deque +import dataclasses from enum import Enum import threading import time @@ -46,6 +47,51 @@ if TYPE_CHECKING: from ..sql._typing import _InfoType +@dataclasses.dataclass(frozen=True) +class PoolResetState: + """describes the state of a DBAPI connection as it is being passed to + the :meth:`.PoolEvents.reset` connection pool event. + + .. versionadded:: 2.0.0b3 + + """ + + __slots__ = ("transaction_was_reset", "terminate_only", "asyncio_safe") + + transaction_was_reset: bool + """Indicates if the transaction on the DBAPI connection was already + essentially "reset" back by the :class:`.Connection` object. + + This boolean is True if the :class:`.Connection` had transactional + state present upon it, which was then not closed using the + :meth:`.Connection.rollback` or :meth:`.Connection.commit` method; + instead, the transaction was closed inline within the + :meth:`.Connection.close` method so is guaranteed to remain non-present + when this event is reached. + + """ + + terminate_only: bool + """indicates if the connection is to be immediately terminated and + not checked in to the pool. + + This occurs for connections that were invalidated, as well as asyncio + connections that were not cleanly handled by the calling code that + are instead being garbage collected. In the latter case, + operations can't be safely run on asyncio connections within garbage + collection as there is not necessarily an event loop present. + + """ + + asyncio_safe: bool + """Indicates if the reset operation is occurring within a scope where + an enclosing event loop is expected to be present for asyncio applications. + + Will be False in the case that the connection is being garbage collected. + + """ + + class ResetStyle(Enum): """Describe options for "reset on return" behaviors.""" @@ -168,39 +214,46 @@ class Pool(log.Identified, event.EventTarget): logging. :param reset_on_return: Determine steps to take on - connections as they are returned to the pool, which were - not otherwise handled by a :class:`_engine.Connection`. - - reset_on_return can have any of these values: - - * ``"rollback"`` - call rollback() on the connection, - to release locks and transaction resources. - This is the default value. The vast majority - of use cases should leave this value set. - * ``True`` - same as 'rollback', this is here for - backwards compatibility. - * ``"commit"`` - call commit() on the connection, - to release locks and transaction resources. - A commit here may be desirable for databases that - cache query plans if a commit is emitted, - such as Microsoft SQL Server. However, this - value is more dangerous than 'rollback' because - any data changes present on the transaction - are committed unconditionally. - * ``None`` - don't do anything on the connection. - This setting may be appropriate if the database / DBAPI - works in pure "autocommit" mode at all times, or if the - application uses the :class:`_engine.Engine` with consistent - connectivity patterns. See the section - :ref:`pool_reset_on_return` for more details. - - * ``False`` - same as None, this is here for - backwards compatibility. + connections as they are returned to the pool, which were + not otherwise handled by a :class:`_engine.Connection`. + Available from :func:`_sa.create_engine` via the + :paramref:`_sa.create_engine.pool_reset_on_return` parameter. + + :paramref:`_pool.Pool.reset_on_return` can have any of these values: + + * ``"rollback"`` - call rollback() on the connection, + to release locks and transaction resources. + This is the default value. The vast majority + of use cases should leave this value set. + * ``"commit"`` - call commit() on the connection, + to release locks and transaction resources. + A commit here may be desirable for databases that + cache query plans if a commit is emitted, + such as Microsoft SQL Server. However, this + value is more dangerous than 'rollback' because + any data changes present on the transaction + are committed unconditionally. + * ``None`` - don't do anything on the connection. + This setting may be appropriate if the database / DBAPI + works in pure "autocommit" mode at all times, or if + a custom reset handler is established using the + :meth:`.PoolEvents.reset` event handler. + + * ``True`` - same as 'rollback', this is here for + backwards compatibility. + * ``False`` - same as None, this is here for + backwards compatibility. + + For further customization of reset on return, the + :meth:`.PoolEvents.reset` event hook may be used which can perform + any connection activity desired on reset. .. seealso:: :ref:`pool_reset_on_return` + :meth:`.PoolEvents.reset` + :param events: a list of 2-tuples, each of the form ``(callable, target)`` which will be passed to :func:`.event.listen` upon construction. Provided here so that event listeners @@ -671,7 +724,9 @@ class _ConnectionRecord(ConnectionPoolEntry): rec.fairy_ref = ref = weakref.ref( fairy, - lambda ref: _finalize_fairy(None, rec, pool, ref, echo, True) + lambda ref: _finalize_fairy( + None, rec, pool, ref, echo, transaction_was_reset=False + ) if _finalize_fairy else None, ) @@ -864,7 +919,7 @@ def _finalize_fairy( weakref.ref[_ConnectionFairy] ], # this is None when called directly, not by the gc echo: Optional[log._EchoFlagType], - reset: bool = True, + transaction_was_reset: bool = False, fairy: Optional[_ConnectionFairy] = None, ) -> None: """Cleanup for a :class:`._ConnectionFairy` whether or not it's already @@ -906,11 +961,7 @@ def _finalize_fairy( if dbapi_connection is not None: if connection_record and echo: pool.logger.debug( - "Connection %r being returned to pool%s", - dbapi_connection, - ", transaction state was already reset by caller" - if not reset - else "", + "Connection %r being returned to pool", dbapi_connection ) try: @@ -923,8 +974,13 @@ def _finalize_fairy( echo, ) assert fairy.dbapi_connection is dbapi_connection - if reset and can_manipulate_connection: - fairy._reset(pool) + + fairy._reset( + pool, + transaction_was_reset=transaction_was_reset, + terminate_only=detach, + asyncio_safe=can_manipulate_connection, + ) if detach: if connection_record: @@ -1293,29 +1349,55 @@ class _ConnectionFairy(PoolProxiedConnection): def _checkout_existing(self) -> _ConnectionFairy: return _ConnectionFairy._checkout(self._pool, fairy=self) - def _checkin(self, reset: bool = True) -> None: + def _checkin(self, transaction_was_reset: bool = False) -> None: _finalize_fairy( self.dbapi_connection, self._connection_record, self._pool, None, self._echo, - reset=reset, + transaction_was_reset=transaction_was_reset, fairy=self, ) def _close(self) -> None: self._checkin() - def _reset(self, pool: Pool) -> None: + def _reset( + self, + pool: Pool, + transaction_was_reset: bool, + terminate_only: bool, + asyncio_safe: bool, + ) -> None: if pool.dispatch.reset: - pool.dispatch.reset(self.dbapi_connection, self._connection_record) + pool.dispatch.reset( + self.dbapi_connection, + self._connection_record, + PoolResetState( + transaction_was_reset=transaction_was_reset, + terminate_only=terminate_only, + asyncio_safe=asyncio_safe, + ), + ) + + if not asyncio_safe: + return + if pool._reset_on_return is reset_rollback: - if self._echo: - pool.logger.debug( - "Connection %s rollback-on-return", self.dbapi_connection - ) - pool._dialect.do_rollback(self) + if transaction_was_reset: + if self._echo: + pool.logger.debug( + "Connection %s reset, transaction already reset", + self.dbapi_connection, + ) + else: + if self._echo: + pool.logger.debug( + "Connection %s rollback-on-return", + self.dbapi_connection, + ) + pool._dialect.do_rollback(self) elif pool._reset_on_return is reset_commit: if self._echo: pool.logger.debug( @@ -1396,7 +1478,7 @@ class _ConnectionFairy(PoolProxiedConnection): if self._counter == 0: self._checkin() - def _close_no_reset(self) -> None: + def _close_special(self, transaction_reset: bool = False) -> None: self._counter -= 1 if self._counter == 0: - self._checkin(reset=False) + self._checkin(transaction_was_reset=transaction_reset) diff --git a/lib/sqlalchemy/pool/events.py b/lib/sqlalchemy/pool/events.py index 47ab106d7..0bf69420e 100644 --- a/lib/sqlalchemy/pool/events.py +++ b/lib/sqlalchemy/pool/events.py @@ -15,6 +15,7 @@ from typing import Union from .base import ConnectionPoolEntry from .base import Pool from .base import PoolProxiedConnection +from .base import PoolResetState from .. import event from .. import util @@ -189,22 +190,46 @@ class PoolEvents(event.Events[Pool]): """ + @event._legacy_signature( + "2.0", + ["dbapi_connection", "connection_record"], + lambda dbapi_connection, connection_record, reset_state: ( + dbapi_connection, + connection_record, + ), + ) def reset( self, dbapi_connection: DBAPIConnection, connection_record: ConnectionPoolEntry, + reset_state: PoolResetState, ) -> None: """Called before the "reset" action occurs for a pooled connection. This event represents when the ``rollback()`` method is called on the DBAPI connection - before it is returned to the pool. The behavior of "reset" can - be controlled, including disabled, using the ``reset_on_return`` - pool argument. - + before it is returned to the pool or discarded. + A custom "reset" strategy may be implemented using this event hook, + which may also be combined with disabling the default "reset" + behavior using the :paramref:`_pool.Pool.reset_on_return` parameter. + + The primary difference between the :meth:`_events.PoolEvents.reset` and + :meth:`_events.PoolEvents.checkin` events are that + :meth:`_events.PoolEvents.reset` is called not just for pooled + connections that are being returned to the pool, but also for + connections that were detached using the + :meth:`_engine.Connection.detach` method as well as asyncio connections + that are being discarded due to garbage collection taking place on + connections before the connection was checked in. + + Note that the event **is not** invoked for connections that were + invalidated using :meth:`_engine.Connection.invalidate`. These + events may be intercepted using the :meth:`.PoolEvents.soft_invalidate` + and :meth:`.PoolEvents.invalidate` event hooks, and all "connection + close" events may be intercepted using :meth:`.PoolEvents.close`. The :meth:`_events.PoolEvents.reset` event is usually followed by the - :meth:`_events.PoolEvents.checkin` event is called, except in those + :meth:`_events.PoolEvents.checkin` event, except in those cases where the connection is discarded immediately after reset. :param dbapi_connection: a DBAPI connection. @@ -213,8 +238,16 @@ class PoolEvents(event.Events[Pool]): :param connection_record: the :class:`.ConnectionPoolEntry` managing the DBAPI connection. + :param reset_state: :class:`.PoolResetState` instance which provides + information about the circumstances under which the connection + is being reset. + + .. versionadded:: 2.0 + .. seealso:: + :ref:`pool_reset_on_return` + :meth:`_events.ConnectionEvents.rollback` :meth:`_events.ConnectionEvents.commit` diff --git a/test/engine/test_deprecations.py b/test/engine/test_deprecations.py index f7602f98a..f6fa21f29 100644 --- a/test/engine/test_deprecations.py +++ b/test/engine/test_deprecations.py @@ -304,6 +304,64 @@ def select1(db): return str(select(1).compile(dialect=db.dialect)) +class ResetEventTest(fixtures.TestBase): + def _fixture(self, **kw): + dbapi = Mock() + return ( + dbapi, + pool.QueuePool(creator=lambda: dbapi.connect("foo.db"), **kw), + ) + + def _engine_fixture(self, **kw): + dbapi = Mock() + + return dbapi, create_engine( + "postgresql://", + module=dbapi, + creator=lambda: dbapi.connect("foo.db"), + _initialize=False, + ) + + def test_custom(self): + dbapi, p = self._fixture(reset_on_return=None) + + @event.listens_for(p, "reset") + def custom_reset(dbapi_conn, record): + dbapi_conn.special_reset_method() + + c1 = p.connect() + with expect_deprecated( + 'The argument signature for the "PoolEvents.reset" event ' + "listener has changed as of version 2.0" + ): + c1.close() + + assert dbapi.connect().special_reset_method.called + assert not dbapi.connect().rollback.called + assert not dbapi.connect().commit.called + + @testing.combinations(True, False, argnames="use_engine_transaction") + def test_custom_via_engine(self, use_engine_transaction): + dbapi, engine = self._engine_fixture(reset_on_return=None) + + @event.listens_for(engine, "reset") + def custom_reset(dbapi_conn, record): + dbapi_conn.special_reset_method() + + c1 = engine.connect() + if use_engine_transaction: + c1.begin() + + with expect_deprecated( + 'The argument signature for the "PoolEvents.reset" event ' + "listener has changed as of version 2.0" + ): + c1.close() + assert dbapi.connect().rollback.called + + assert dbapi.connect().special_reset_method.called + + class EngineEventsTest(fixtures.TestBase): __requires__ = ("ad_hoc_engines",) __backend__ = True diff --git a/test/engine/test_logging.py b/test/engine/test_logging.py index 7cf67c220..277248617 100644 --- a/test/engine/test_logging.py +++ b/test/engine/test_logging.py @@ -553,6 +553,14 @@ class PoolLoggingTest(fixtures.TestBase): conn = None conn = q.connect() + conn._close_special(transaction_reset=True) + conn = None + + conn = q.connect() + conn._close_special(transaction_reset=False) + conn = None + + conn = q.connect() conn = None del conn lazy_gc() @@ -563,13 +571,19 @@ class PoolLoggingTest(fixtures.TestBase): [ "Created new connection %r", "Connection %r checked out from pool", - "Connection %r being returned to pool%s", + "Connection %r being returned to pool", + "Connection %s rollback-on-return", + "Connection %r checked out from pool", + "Connection %r being returned to pool", "Connection %s rollback-on-return", "Connection %r checked out from pool", - "Connection %r being returned to pool%s", + "Connection %r being returned to pool", + "Connection %s reset, transaction already reset", + "Connection %r checked out from pool", + "Connection %r being returned to pool", "Connection %s rollback-on-return", "Connection %r checked out from pool", - "Connection %r being returned to pool%s", + "Connection %r being returned to pool", "Connection %s rollback-on-return", "%s connection %r", ] diff --git a/test/engine/test_pool.py b/test/engine/test_pool.py index f28da14be..9d9c3a429 100644 --- a/test/engine/test_pool.py +++ b/test/engine/test_pool.py @@ -9,8 +9,10 @@ from unittest.mock import patch import weakref import sqlalchemy as tsa +from sqlalchemy import create_engine from sqlalchemy import event from sqlalchemy import pool +from sqlalchemy import PoolResetState from sqlalchemy import select from sqlalchemy import testing from sqlalchemy.engine import default @@ -1909,14 +1911,143 @@ class ResetOnReturnTest(PoolTestBase): pool.QueuePool(creator=lambda: dbapi.connect("foo.db"), **kw), ) - def test_plain_rollback(self): + def _engine_fixture(self, **kw): + dbapi = Mock() + + return dbapi, create_engine( + "postgresql://", + module=dbapi, + creator=lambda: dbapi.connect("foo.db"), + _initialize=False, + ) + + @testing.combinations("detach", "invalidate", "return") + def test_custom(self, extra_step): + dbapi, p = self._fixture(reset_on_return=None) + + @event.listens_for(p, "reset") + def custom_reset(dbapi_conn, record, reset_state): + dbapi_conn.special_reset_method(reset_state) + + c1 = p.connect() + + if extra_step == "detach": + c1.detach() + elif extra_step == "invalidate": + c1.invalidate() + c1.close() + + special_event = mock.call.special_reset_method( + PoolResetState( + transaction_was_reset=False, + terminate_only=extra_step == "detach", + asyncio_safe=True, + ) + ) + if extra_step == "detach": + expected = [special_event, mock.call.close()] + elif extra_step == "invalidate": + expected = [mock.call.close()] + else: + expected = [special_event] + eq_(dbapi.connect().mock_calls, expected) + + assert not dbapi.connect().rollback.called + assert not dbapi.connect().commit.called + + @testing.combinations(True, False, argnames="assert_w_event") + @testing.combinations(True, False, argnames="use_engine_transaction") + def test_custom_via_engine(self, assert_w_event, use_engine_transaction): + dbapi, engine = self._engine_fixture(reset_on_return=None) + + if assert_w_event: + + @event.listens_for(engine, "reset") + def custom_reset(dbapi_conn, record, reset_state): + dbapi_conn.special_reset_method(reset_state) + + c1 = engine.connect() + if use_engine_transaction: + c1.begin() + c1.close() + assert dbapi.connect().rollback.called + + if assert_w_event: + special_event = mock.call.special_reset_method( + PoolResetState( + transaction_was_reset=use_engine_transaction, + terminate_only=False, + asyncio_safe=True, + ) + ) + + if use_engine_transaction: + expected = [mock.call.rollback(), special_event] + else: + expected = [special_event, mock.call.rollback()] + eq_(dbapi.connect().mock_calls, expected) + + @testing.combinations(True, False, argnames="assert_w_event") + def test_plain_rollback(self, assert_w_event): dbapi, p = self._fixture(reset_on_return="rollback") + if assert_w_event: + + @event.listens_for(p, "reset") + def custom_reset(dbapi_conn, record, reset_state): + dbapi_conn.special_reset_method(reset_state) + c1 = p.connect() c1.close() assert dbapi.connect().rollback.called assert not dbapi.connect().commit.called + if assert_w_event: + special_event = mock.call.special_reset_method( + PoolResetState( + transaction_was_reset=False, + terminate_only=False, + asyncio_safe=True, + ) + ) + + expected = [special_event, mock.call.rollback()] + eq_(dbapi.connect().mock_calls, expected) + + @testing.combinations(True, False, argnames="assert_w_event") + @testing.combinations(True, False, argnames="use_engine_transaction") + def test_plain_rollback_via_engine( + self, assert_w_event, use_engine_transaction + ): + dbapi, engine = self._engine_fixture(reset_on_return="rollback") + + if assert_w_event: + + @event.listens_for(engine, "reset") + def custom_reset(dbapi_conn, record, reset_state): + dbapi_conn.special_reset_method(reset_state) + + c1 = engine.connect() + if use_engine_transaction: + c1.begin() + c1.close() + assert dbapi.connect().rollback.called + + if assert_w_event: + special_event = mock.call.special_reset_method( + PoolResetState( + transaction_was_reset=use_engine_transaction, + terminate_only=False, + asyncio_safe=True, + ) + ) + + if use_engine_transaction: + expected = [mock.call.rollback(), special_event] + else: + expected = [special_event, mock.call.rollback()] + eq_(dbapi.connect().mock_calls, expected) + def test_plain_commit(self): dbapi, p = self._fixture(reset_on_return="commit") diff --git a/test/profiles.txt b/test/profiles.txt index 949ec4b26..0b7c5c3e3 100644 --- a/test/profiles.txt +++ b/test/profiles.txt @@ -273,18 +273,18 @@ test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_connection_execute # TEST: test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mariadb_mysqldb_dbapiunicode_cextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mariadb_mysqldb_dbapiunicode_nocextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mariadb_pymysql_dbapiunicode_cextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mariadb_pymysql_dbapiunicode_nocextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mssql_pyodbc_dbapiunicode_cextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mssql_pyodbc_dbapiunicode_nocextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_oracle_cx_oracle_dbapiunicode_cextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_oracle_cx_oracle_dbapiunicode_nocextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_postgresql_psycopg2_dbapiunicode_cextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_postgresql_psycopg2_dbapiunicode_nocextensions 101 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_sqlite_pysqlite_dbapiunicode_cextensions 99 -test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_sqlite_pysqlite_dbapiunicode_nocextensions 99 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mariadb_mysqldb_dbapiunicode_cextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mariadb_mysqldb_dbapiunicode_nocextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mariadb_pymysql_dbapiunicode_cextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mariadb_pymysql_dbapiunicode_nocextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mssql_pyodbc_dbapiunicode_cextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_mssql_pyodbc_dbapiunicode_nocextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_oracle_cx_oracle_dbapiunicode_cextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_oracle_cx_oracle_dbapiunicode_nocextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_postgresql_psycopg2_dbapiunicode_cextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_postgresql_psycopg2_dbapiunicode_nocextensions 106 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_sqlite_pysqlite_dbapiunicode_cextensions 105 +test.aaa_profiling.test_resultset.ExecutionTest.test_minimal_engine_execute x86_64_linux_cpython_3.10_sqlite_pysqlite_dbapiunicode_nocextensions 105 # TEST: test.aaa_profiling.test_resultset.ResultSetTest.test_contains_doesnt_compile |