[Zodb-checkins] SVN: ZODB/trunk/src/ZODB/ Greatly simplified the internal dance to create, open, and close

Jim Fulton jim at zope.com
Thu Jun 16 16:21:27 EDT 2005


Log message for revision 30816:
  Greatly simplified the internal dance to create, open, and close
  connections.   Connections now get their database and related data
  when they are constructed.  They no longer throw away this information
  when they are closed.  (The old design that threw away the database
  and related data did so to avoid memory leaks in Python 1.)
  
  Fixed a multi-database connection-management bug. (The fix
  necessitated the simplification.)
  

Changed:
  U   ZODB/trunk/src/ZODB/Connection.py
  U   ZODB/trunk/src/ZODB/DB.py
  U   ZODB/trunk/src/ZODB/interfaces.py
  U   ZODB/trunk/src/ZODB/tests/testConnection.py
  U   ZODB/trunk/src/ZODB/tests/testZODB.py
  U   ZODB/trunk/src/ZODB/tests/testcrossdatabasereferences.py

-=-
Modified: ZODB/trunk/src/ZODB/Connection.py
===================================================================
--- ZODB/trunk/src/ZODB/Connection.py	2005-06-16 16:48:15 UTC (rev 30815)
+++ ZODB/trunk/src/ZODB/Connection.py	2005-06-16 20:21:27 UTC (rev 30816)
@@ -66,28 +66,23 @@
                IPersistentDataManager,
                ISynchronizer)
 
-    _storage = _normal_storage = _savepoint_storage = None
 
     _code_timestamp = 0
 
     ##########################################################################
     # Connection methods, ZODB.IConnection
 
-    def __init__(self, version='', cache_size=400,
-                 cache_deactivate_after=None, mvcc=True,
-                 txn_mgr=DEPRECATED_ARGUMENT,
-                 transaction_manager=None,
-                 synch=True):
+    def __init__(self, db, version='', cache_size=400):
         """Create a new Connection."""
 
-        if txn_mgr is not DEPRECATED_ARGUMENT:
-            deprecated36("use transaction_manager= instead of txn_mgr=")
-            if transaction_manager is None:
-                transaction_manager = txn_mgr
-            else:
-                raise ValueError("cannot specify both transaction_manager= "
-                                 "and txn_mgr=")
+        self._db = db
+        self._normal_storage = self._storage = db._storage
+        self.new_oid = db._storage.new_oid
+        self._savepoint_storage = None
 
+        self.transaction_manager = self._synch = self._mvcc = None
+
+
         self._log = logging.getLogger("ZODB.Connection")
         self._debug_info = ()
         self._opened = None # time.time() when DB.open() opened us
@@ -101,6 +96,7 @@
             # Unclear:  Why do we want version caches to behave this way?
 
             self._cache.cache_drain_resistance = 100
+
         self._committed = []
         self._added = {}
         self._added_during_commit = None
@@ -119,15 +115,6 @@
         # Do we need to join a txn manager?
         self._needs_to_join = True
 
-        # If a transaction manager is passed to the constructor, use
-        # it instead of the global transaction manager.  The instance
-        # variable will hold a TM instance.
-        self.transaction_manager = transaction_manager or transaction.manager
-
-        # _synch is a boolean; if True, the Connection will register
-        # with the TM to receive afterCompletion() calls.
-        self._synch = synch
-
         # _invalidated queues invalidate messages delivered from the DB
         # _inv_lock prevents one thread from modifying the set while
         # another is processing invalidations.  All the invalidations
@@ -158,7 +145,6 @@
         # If it is None, then the current revisions are acceptable.
         # If the connection is in a version, mvcc will be disabled, because
         # loadBefore() only returns non-version data.
-        self._mvcc = mvcc and not version
         self._txn_time = None
 
         # To support importFile(), implemented in the ExportImport base
@@ -167,11 +153,16 @@
         # to pass to _importDuringCommit().
         self._import = None
 
-        self.connections = None
 
+        self._reader = ObjectReader(self, self._cache, self._db.classFactory)
+
+        # Multi-database support
+        self.connections = {self._db.database_name: self}
+
+
     def add(self, obj):
         """Add a new object 'obj' to the database and assign it an oid."""
-        if self._storage is None:
+        if self._opened is None:
             raise ConnectionStateError("The database connection is closed")
 
         marker = object()
@@ -195,7 +186,7 @@
 
     def get(self, oid):
         """Return the persistent object with oid 'oid'."""
-        if self._storage is None:
+        if self._opened is None:
             raise ConnectionStateError("The database connection is closed")
 
         obj = self._cache.get(oid, None)
@@ -234,7 +225,7 @@
             self.__onCloseCallbacks = []
         self.__onCloseCallbacks.append(f)
 
-    def close(self):
+    def close(self, primary=True):
         """Close the Connection."""
         if not self._needs_to_join:
             # We're currently joined to a transaction.
@@ -254,26 +245,36 @@
                     self._log.error("Close callback failed for %s", f,
                                     exc_info=sys.exc_info())
             self.__onCloseCallbacks = None
-        self._storage = self._savepoint_storage = self._normal_storage = None
-        self.new_oid = None
+
         self._debug_info = ()
-        self._opened = None
-        # Return the connection to the pool.
-        if self._db is not None:
-            if self._synch:
-                self.transaction_manager.unregisterSynch(self)
-            self._db._closeConnection(self)
-            # _closeConnection() set self._db to None.  However, we can't
-            # assert that here, because self may have been reused (by
-            # another thread) by the time we get back here.
 
+        if self._synch:
+            self.transaction_manager.unregisterSynch(self)
+            self._synch = None
+
+        if primary:
+            for connection in self.connections.values():
+                if connection is not self:
+                    connection.close(False)
+
+            # Return the connection to the pool.
+            if self._opened is not None:
+                self._db._returnToPool(self)
+                
+                # _returnToPool() set self._opened to None.
+                # However, we can't assert that here, because self may
+                # have been reused (by another thread) by the time we
+                # get back here.
+        else:
+            self._opened = None
+
     def db(self):
         """Returns a handle to the database this connection belongs to."""
         return self._db
 
     def isReadOnly(self):
         """Returns True if the storage for this connection is read only."""
-        if self._storage is None:
+        if self._opened is None:
             raise ConnectionStateError("The database connection is closed")
         return self._storage.isReadOnly()
 
@@ -700,7 +701,7 @@
         database."""
         oid = obj._p_oid
 
-        if self._storage is None:
+        if self._opened is None:
             msg = ("Shouldn't load state for %s "
                    "when the connection is closed" % oid_repr(oid))
             self._log.error(msg)
@@ -873,8 +874,8 @@
         # return a list of [ghosts....not recently used.....recently used]
         return everything.items() + items
 
-    def _setDB(self, odb, mvcc=None, txn_mgr=DEPRECATED_ARGUMENT,
-               transaction_manager=None, synch=None):
+    def open(self, transaction_manager=None, mvcc=True, synch=True,
+             delegate=True):
         """Register odb, the DB that this Connection uses.
 
         This method is called by the DB every time a Connection
@@ -893,40 +894,38 @@
         register for afterCompletion() calls.
         """
 
-        if txn_mgr is not DEPRECATED_ARGUMENT:
-            deprecated36("use transaction_manager= instead of txn_mgr=")
-            if transaction_manager is None:
-                transaction_manager = txn_mgr
-            else:
-                raise ValueError("cannot specify both transaction_manager= "
-                                 "and txn_mgr=")
-
         # TODO:  Why do we go to all the trouble of setting _db and
         # other attributes on open and clearing them on close?
         # A Connection is only ever associated with a single DB
         # and Storage.
 
-        self._db = odb
-        self._normal_storage = self._storage = odb._storage
-        self.new_oid = odb._storage.new_oid
         self._opened = time()
-        if synch is not None:
-            self._synch = synch
-        if mvcc is not None:
-            self._mvcc = mvcc
-        self.transaction_manager = transaction_manager or transaction.manager
+        self._synch = synch
+        self._mvcc = mvcc and not self._version
+
+        if transaction_manager is None:
+            transaction_manager = transaction.manager
+        
+        self.transaction_manager = transaction_manager
+
         if self._reset_counter != global_reset_counter:
             # New code is in place.  Start a new cache.
             self._resetCache()
         else:
             self._flush_invalidations()
-        if self._synch:
-            self.transaction_manager.registerSynch(self)
-        self._reader = ObjectReader(self, self._cache, self._db.classFactory)
 
-        # Multi-database support
-        self.connections = {self._db.database_name: self}
+        if synch:
+            transaction_manager.registerSynch(self)
 
+        if self._cache is not None:
+            self._cache.incrgc() # This is a good time to do some GC
+
+        if delegate:
+            # delegate open to secondary connections
+            for connection in self.connections.values():
+                if connection is not self:
+                    connection.open(transaction_manager, mvcc, synch, False)
+
     def _resetCache(self):
         """Creates a new cache, discarding the old one.
 

Modified: ZODB/trunk/src/ZODB/DB.py
===================================================================
--- ZODB/trunk/src/ZODB/DB.py	2005-06-16 16:48:15 UTC (rev 30815)
+++ ZODB/trunk/src/ZODB/DB.py	2005-06-16 20:21:27 UTC (rev 30816)
@@ -273,7 +273,7 @@
             self.undoInfo = storage.undoInfo
 
     # This is called by Connection.close().
-    def _closeConnection(self, connection):
+    def _returnToPool(self, connection):
         """Return a connection to the pool.
 
         connection._db must be self on entry.
@@ -282,7 +282,7 @@
         self._a()
         try:
             assert connection._db is self
-            connection._db = None
+            connection._opened = None
 
             am = self._activity_monitor
             if am is not None:
@@ -560,16 +560,14 @@
                     size = self._version_cache_size
                 else:
                     size = self._cache_size
-                c = self.klass(version=version, cache_size=size, mvcc=mvcc,
-                               transaction_manager=transaction_manager)
+                c = self.klass(self, version, size)
                 pool.push(c)
                 result = pool.pop()
             assert result is not None
 
             # Tell the connection it belongs to self.
-            result._setDB(self, mvcc=mvcc, synch=synch,
-                          transaction_manager=transaction_manager)
-
+            result.open(transaction_manager, mvcc, synch)
+            
             # A good time to do some cache cleanup.
             self._connectionMap(lambda c: c.cacheGC())
 

Modified: ZODB/trunk/src/ZODB/interfaces.py
===================================================================
--- ZODB/trunk/src/ZODB/interfaces.py	2005-06-16 16:48:15 UTC (rev 30815)
+++ ZODB/trunk/src/ZODB/interfaces.py	2005-06-16 20:21:27 UTC (rev 30816)
@@ -112,10 +112,7 @@
             getTransferCounts
     """
 
-    def __init__(version='', cache_size=400,
-                 cache_deactivate_after=None, mvcc=True,
-                 transaction_manager=None,
-                 synch=True):
+    def __init__(database, version='', cache_size=400):
         """Create a new Connection.
 
         A Connection instance should by instantiated by the DB

Modified: ZODB/trunk/src/ZODB/tests/testConnection.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testConnection.py	2005-06-16 16:48:15 UTC (rev 30815)
+++ ZODB/trunk/src/ZODB/tests/testConnection.py	2005-06-16 20:21:27 UTC (rev 30816)
@@ -28,9 +28,9 @@
 
     def setUp(self):
         from ZODB.Connection import Connection
-        self.datamgr = Connection()
         self.db = StubDatabase()
-        self.datamgr._setDB(self.db)
+        self.datamgr = Connection(self.db)
+        self.datamgr.open()
         self.transaction = StubTransaction()
 
     def tearDown(self):
@@ -358,7 +358,8 @@
         >>> cn.db() is db
         True
         >>> cn.close()
-        >>> cn.db()
+        >>> cn.db() is db
+        True
         """
 
     def test_isReadOnly(self):

Modified: ZODB/trunk/src/ZODB/tests/testZODB.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testZODB.py	2005-06-16 16:48:15 UTC (rev 30815)
+++ ZODB/trunk/src/ZODB/tests/testZODB.py	2005-06-16 20:21:27 UTC (rev 30816)
@@ -165,7 +165,7 @@
         ZODB.Connection.resetCaches()
         conn.close()
         self.assert_(len(conn._cache) > 0)  # Still not flushed
-        conn._setDB(self._db)  # simulate the connection being reopened
+        conn.open()  # simulate the connection being reopened
         self.assertEqual(len(conn._cache), 0)
 
     def checkExplicitTransactionManager(self):

Modified: ZODB/trunk/src/ZODB/tests/testcrossdatabasereferences.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testcrossdatabasereferences.py	2005-06-16 16:48:15 UTC (rev 30815)
+++ ZODB/trunk/src/ZODB/tests/testcrossdatabasereferences.py	2005-06-16 20:21:27 UTC (rev 30816)
@@ -78,6 +78,50 @@
 
 """
 
+def test_connection_management_doesnt_get_caching_wrong():
+    """
+
+If a connection participates in a multidatabase, then it's
+connections must remain so that references between it's cached
+objects remain sane.
+
+    >>> import ZODB.tests.util, transaction, persistent
+    >>> databases = {}
+    >>> db1 = ZODB.tests.util.DB(databases=databases, database_name='1')
+    >>> db2 = ZODB.tests.util.DB(databases=databases, database_name='2')
+    >>> tm = transaction.TransactionManager()
+    >>> conn1 = db1.open(transaction_manager=tm)
+    >>> conn2 = conn1.get_connection('2')
+    >>> z = MyClass()
+    >>> conn2.root()['z'] = z
+    >>> tm.commit()
+    >>> x = MyClass()
+    >>> x.z = z
+    >>> conn1.root()['x'] = x
+    >>> y = MyClass()
+    >>> y.z = z
+    >>> conn1.root()['y'] = y
+    >>> tm.commit()
+
+    >>> conn1.root()['x'].z is conn1.root()['y'].z
+    True
+
+So, we have 2 objects in conn1 that point to the same object in conn2.
+Now, we'll deactivate one, close and repopen the connection, and see
+if we get the same objects:
+
+    >>> x._p_deactivate()
+    >>> conn1.close()
+    >>> conn1 = db1.open(transaction_manager=tm)
+
+    >>> conn1.root()['x'].z is conn1.root()['y'].z
+    True
+    
+    >>> db1.close()
+    >>> db2.close()
+"""
+
+
 def tearDownDbs(test):
     test.globs['db1'].close()
     test.globs['db2'].close()



More information about the Zodb-checkins mailing list