]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
ensure pool.reset event always called for reset
authorMike Bayer <mike_mp@zzzcomputing.com>
Tue, 25 Oct 2022 20:00:50 +0000 (16:00 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Sun, 30 Oct 2022 19:43:19 +0000 (15:43 -0400)
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).

Change-Id: Ie17c4f55d02beb6f570b9de6b3044baffa7d6df6
Fixes: #8717
18 files changed:
doc/build/changelog/unreleased_14/8717.rst [new file with mode: 0644]
doc/build/changelog/unreleased_20/8717.rst [new file with mode: 0644]
doc/build/core/events.rst
doc/build/core/pooling.rst
lib/sqlalchemy/__init__.py
lib/sqlalchemy/dialects/mssql/base.py
lib/sqlalchemy/dialects/postgresql/base.py
lib/sqlalchemy/engine/base.py
lib/sqlalchemy/engine/create.py
lib/sqlalchemy/event/legacy.py
lib/sqlalchemy/events.py
lib/sqlalchemy/pool/__init__.py
lib/sqlalchemy/pool/base.py
lib/sqlalchemy/pool/events.py
test/engine/test_deprecations.py
test/engine/test_logging.py
test/engine/test_pool.py
test/profiles.txt

diff --git a/doc/build/changelog/unreleased_14/8717.rst b/doc/build/changelog/unreleased_14/8717.rst
new file mode 100644 (file)
index 0000000..ee5fa59
--- /dev/null
@@ -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 (file)
index 0000000..8067596
--- /dev/null
@@ -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).
index 4452ae7f5839b700c5144e9c5e4a98409536b2c1..3645528075f362adebf91347c5a1cba5525d4854 100644 (file)
@@ -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
index cc19e96af81b8696591364296a677865cd6a1a61..1121919db7515907bc9c59878fc94b49d2bf81ba 100644 (file)
@@ -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:
index 5823239dc978984780e41a6ba430930b95ee71da..b22d7ca8c651465c3c71fe44e25e5d18ac4b215e 100644 (file)
@@ -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
index 5d42d98e35bfd3be53a170efdb9e98dd706ce65d..a338ba27af136838612155f2cd60ddfd3c02665f 100644 (file)
@@ -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
index 5eab8f47c3c4740a9889fd3fb879d581a33bf0c1..482e36594a3cfd3498369f7cb830b4e513de2ddc 100644 (file)
@@ -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
index 1ec30729757dcf1f72cba1254af35c65cfc42210..8cbc0163c64422ee3d92f4a9066535f737dcf48e 100644 (file)
@@ -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()
 
index a9b388d7144e41b5e7818a51a491a0dcb85566af..ef54639723de0c9e82ff99bc8ed53c9b7306fe40 100644 (file)
@@ -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
index 8ae9b7035239b1574db064e221c61c7894fb3549..3d434141046efc967d51d33913871d95300729be 100644 (file)
@@ -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 "",
         }
     )
index 4a8a52337576785a05bde6c25e7fb97a1675bf12..4e81ceec52b233720329ad06ff304c1bedf23b20 100644 (file)
@@ -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
index c86d3ddedaf7480eeb1a10bcdd4f86352549e6c6..8eb2ae4ab3a131e8983b05c60e04d2889c492042 100644 (file)
@@ -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
index 41f2f03b278eddeddc3c5a1be304827168879139..18ff66e5de773578f731e9e119c331514aa7531d 100644 (file)
@@ -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)
index 47ab106d76cafdcaa0db48753668928c29944e06..0bf69420e2360441a3ff50dc828694ca2985dca4 100644 (file)
@@ -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`
index f7602f98a7dee74a8e9ff699f07e0771a068885f..f6fa21f29dd418a25863aa800491299caf151bc5 100644 (file)
@@ -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
index 7cf67c220e0ec297bd959c1fea204276e3256143..277248617b446747117fa7fc1a7f756990a8ca5c 100644 (file)
@@ -552,6 +552,14 @@ class PoolLoggingTest(fixtures.TestBase):
         conn.close()
         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
@@ -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",
             ]
index f28da14be336a0c796f288b2a4f04f3b413ed128..9d9c3a429d84a03f065eb2688e400c47965046af 100644 (file)
@@ -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")
 
index 949ec4b26b349a1c5d0eed31141397127f446b6c..0b7c5c3e3272e30cec63e133dd38ed95008caf25 100644 (file)
@@ -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