From: Mike Bayer Date: Sat, 1 Feb 2020 17:27:09 +0000 (-0500) Subject: Do away with pool._refs X-Git-Tag: rel_1_4_0b1~535 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c797056413230cc5c11bc458e5f7760063c2682e;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Do away with pool._refs This collection was added only for the benefit of unit tests and is unnecessary for the pool to function. As SQLAlchemy 2.0 will be removing the automatic handling of connections that are garbage collection, remove this collection so that we ultimately don't need a weakref handler to do anything within the pool. The handler will do nothing other than emit a warning that a connection was dereferenced without being explicitly returned to the pool, invalidated, or detached. Change-Id: I4ca196270d5714efbac44dbf6f034e8c7f0af58a --- diff --git a/lib/sqlalchemy/pool/__init__.py b/lib/sqlalchemy/pool/__init__.py index 058d595c8b..eb0d375173 100644 --- a/lib/sqlalchemy/pool/__init__.py +++ b/lib/sqlalchemy/pool/__init__.py @@ -21,7 +21,6 @@ from . import events # noqa from .base import _ConnectionFairy # noqa from .base import _ConnectionRecord # noqa from .base import _finalize_fairy # noqa -from .base import _refs # noqa from .base import Pool from .base import reset_commit from .base import reset_none diff --git a/lib/sqlalchemy/pool/base.py b/lib/sqlalchemy/pool/base.py index 508d036309..bdb981699a 100644 --- a/lib/sqlalchemy/pool/base.py +++ b/lib/sqlalchemy/pool/base.py @@ -425,7 +425,6 @@ class _ConnectionRecord(object): lambda ref: _finalize_fairy and _finalize_fairy(None, rec, pool, ref, echo), ) - _refs.add(rec) if echo: pool.logger.debug( "Connection %r checked out from pool", dbapi_connection @@ -594,7 +593,6 @@ def _finalize_fairy( been garbage collected. """ - _refs.discard(connection_record) if ref is not None: if connection_record.fairy_ref is not ref: @@ -633,9 +631,6 @@ def _finalize_fairy( connection_record.checkin() -_refs = set() - - class _ConnectionFairy(object): """Proxies a DBAPI connection and provides return-on-dereference @@ -918,7 +913,6 @@ class _ConnectionFairy(object): if self._connection_record is not None: rec = self._connection_record - _refs.remove(rec) rec.fairy_ref = None rec.connection = None # TODO: should this be _return_conn? diff --git a/lib/sqlalchemy/testing/assertions.py b/lib/sqlalchemy/testing/assertions.py index a465bedd49..c74259bdf8 100644 --- a/lib/sqlalchemy/testing/assertions.py +++ b/lib/sqlalchemy/testing/assertions.py @@ -13,13 +13,12 @@ import warnings from . import assertsql from . import config +from . import engines from . import mock -from . import util as testutil from .exclusions import db_spec from .util import fail from .. import exc as sa_exc from .. import orm -from .. import pool from .. import schema from .. import types as sqltypes from .. import util @@ -183,49 +182,8 @@ def global_cleanup_assertions(): _assert_no_stray_pool_connections() -_STRAY_CONNECTION_FAILURES = 0 - - def _assert_no_stray_pool_connections(): - global _STRAY_CONNECTION_FAILURES - - # lazy gc on cPython means "do nothing." pool connections - # shouldn't be in cycles, should go away. - testutil.lazy_gc() - - # however, once in awhile, on an EC2 machine usually, - # there's a ref in there. usually just one. - if pool._refs: - - # OK, let's be somewhat forgiving. - _STRAY_CONNECTION_FAILURES += 1 - - print( - "Encountered a stray connection in test cleanup: %s" - % str(pool._refs) - ) - # then do a real GC sweep. We shouldn't even be here - # so a single sweep should really be doing it, otherwise - # there's probably a real unreachable cycle somewhere. - testutil.gc_collect() - - # if we've already had two of these occurrences, or - # after a hard gc sweep we still have pool._refs?! - # now we have to raise. - if pool._refs: - err = str(pool._refs) - - # but clean out the pool refs collection directly, - # reset the counter, - # so the error doesn't at least keep happening. - pool._refs.clear() - _STRAY_CONNECTION_FAILURES = 0 - warnings.warn( - "Stray connection refused to leave " "after gc.collect(): %s" % err - ) - elif _STRAY_CONNECTION_FAILURES > 10: - assert False, "Encountered more than 10 stray connections" - _STRAY_CONNECTION_FAILURES = 0 + engines.testing_reaper.assert_all_closed() def eq_regex(a, b, msg=None): diff --git a/lib/sqlalchemy/testing/engines.py b/lib/sqlalchemy/testing/engines.py index ff4f89606e..4f413915a5 100644 --- a/lib/sqlalchemy/testing/engines.py +++ b/lib/sqlalchemy/testing/engines.py @@ -12,7 +12,6 @@ import warnings import weakref from . import config -from . import uses_deprecated from .util import decorator from .. import event from .. import pool @@ -24,7 +23,13 @@ class ConnectionKiller(object): self.testing_engines = weakref.WeakKeyDictionary() self.conns = set() + def add_pool(self, pool): + event.listen(pool, "connect", self.connect) + event.listen(pool, "checkout", self.checkout) + event.listen(pool, "invalidate", self.invalidate) + def add_engine(self, engine): + self.add_pool(engine.pool) self.testing_engines[engine] = True def connect(self, dbapi_conn, con_record): @@ -75,7 +80,6 @@ class ConnectionKiller(object): else: self._stop_test_ctx_aggressive() - @uses_deprecated() def _stop_test_ctx_minimal(self): self.close_all() @@ -85,7 +89,6 @@ class ConnectionKiller(object): if rec is not config.db: rec.dispose() - @uses_deprecated() def _stop_test_ctx_aggressive(self): self.close_all() for conn, rec in list(self.conns): @@ -265,9 +268,6 @@ def testing_engine(url=None, options=None): engine.pool._timeout = 0 engine.pool._max_overflow = 0 if use_reaper: - event.listen(engine.pool, "connect", testing_reaper.connect) - event.listen(engine.pool, "checkout", testing_reaper.checkout) - event.listen(engine.pool, "invalidate", testing_reaper.invalidate) testing_reaper.add_engine(engine) return engine diff --git a/test/aaa_profiling/test_pool.py b/test/aaa_profiling/test_pool.py index d74dcf6277..538eee45d4 100644 --- a/test/aaa_profiling/test_pool.py +++ b/test/aaa_profiling/test_pool.py @@ -1,4 +1,3 @@ -from sqlalchemy import pool as pool_module from sqlalchemy.pool import QueuePool from sqlalchemy.testing import AssertsExecutionResults from sqlalchemy.testing import fixtures @@ -18,13 +17,6 @@ class QueuePoolTest(fixtures.TestBase, AssertsExecutionResults): def close(self): pass - def teardown(self): - # the tests leave some fake connections - # around which don't necessarily - # get gc'ed as quickly as we'd like all the time, - # particularly for non-refcount gc - pool_module._refs.clear() - def setup(self): # create a throwaway pool which # has the effect of initializing diff --git a/test/engine/test_pool.py b/test/engine/test_pool.py index e4d6dba573..cfe20f5ec0 100644 --- a/test/engine/test_pool.py +++ b/test/engine/test_pool.py @@ -725,6 +725,8 @@ class QueuePoolTest(PoolTestBase): def _do_testqueuepool(self, useclose=False): p = self._queuepool_fixture(pool_size=3, max_overflow=-1) + reaper = testing.engines.ConnectionKiller() + reaper.add_pool(p) def status(pool): return ( @@ -772,8 +774,8 @@ class QueuePoolTest(PoolTestBase): lazy_gc() self.assert_(status(p) == (3, 2, 0, 1)) c1.close() - lazy_gc() - assert not pool._refs + + reaper.assert_all_closed() def test_timeout_accessor(self): expected_timeout = 123 @@ -837,7 +839,7 @@ class QueuePoolTest(PoolTestBase): assert t < 14, "Not all timeouts were < 14 seconds %r" % timeouts def _test_overflow(self, thread_count, max_overflow): - gc_collect() + reaper = testing.engines.ConnectionKiller() dbapi = MockDBAPI() mutex = threading.Lock() @@ -850,6 +852,7 @@ class QueuePoolTest(PoolTestBase): p = pool.QueuePool( creator=creator, pool_size=3, timeout=2, max_overflow=max_overflow ) + reaper.add_pool(p) peaks = [] def whammy(): @@ -873,8 +876,7 @@ class QueuePoolTest(PoolTestBase): self.assert_(max(peaks) <= max_overflow) - lazy_gc() - assert not pool._refs + reaper.assert_all_closed() def test_overflow_reset_on_failed_connect(self): dbapi = Mock()