make historical connection cache story a bit more predictable: you set a total...

make historical connection cache story a bit more predictable: you set a total number of historical connections, not one per serial.  Also remove "historical future" connections--I should have seen seen/remembered lastTransaction.  Now attempts to create historical future connections raise an error, as they should.

NEWS.txt

@@ -59,8 +59,7 @@ Blobs
 - (3.9.0a1) Fixed bug #129921: getSize() function in BlobStorage could not
   deal with garbage files
 
-- (unreleased, after 3.9.0a1) Fixed bug in which MVCC would not work for
-  blobs.
+- (3.9.0a1) Fixed bug in which MVCC would not work for blobs.
 
 BTrees
 ------

src/ZODB/Connection.py

@@ -136,7 +136,7 @@ class Connection(ExportImport, object):
         # During commit, all objects go to either _modified or _creating:
 
         # Dict of oid->flag of new objects (without serial), either
-        # added by add() or implicitely added (discovered by the
+        # added by add() or implicitly added (discovered by the
         # serializer during commit). The flag is True for implicit
         # adding. Used during abort to remove created objects from the
         # _cache, and by persistent_id to check that a new object isn't
@@ -322,9 +322,8 @@ class Connection(ExportImport, object):
 
     def invalidate(self, tid, oids):
         """Notify the Connection that transaction 'tid' invalidated oids."""
-        if self.before is not None and tid > self.before:
-            # this is an historical connection, and the tid is after the
-            # freeze.  Invalidations are irrelevant.
+        if self.before is not None:
+            # this is an historical connection.  Invalidations are irrelevant.
             return
         self._inv_lock.acquire()
         try:
@@ -824,28 +823,11 @@ class Connection(ExportImport, object):
 
         if self.before is not None:
             # Load data that was current before the time we have.
-            if self._txn_time is not None: # MVCC for readonly future conn.
-                before = self._txn_time
-                has_invalidated = True
-            else:
-                before = self.before
-                has_invalidated = False
+            before = self.before
             t = self._storage.loadBefore(obj._p_oid, before)
             if t is None:
-                raise POSKeyError()
+                raise POSKeyError() # historical connection!
             p, serial, end = t
-            if not has_invalidated and end is None:
-                # MVCC: make sure another thread has not beaten us to the punch
-                self._inv_lock.acquire()
-                try:
-                    txn_time = self._txn_time
-                finally:
-                    self._inv_lock.release()
-                if txn_time is not None and txn_time < before:
-                    t = self._storage.loadBefore(obj._p_oid, txn_time)
-                    if t is None:
-                        raise POSKeyError()
-                    p, serial, end = t
         
         else:
             # There is a harmless data race with self._invalidated.  A

src/ZODB/historical_connections.txt

@@ -131,9 +131,9 @@ no matter what you pass into ``db.open``.
 Configuration
 =============
 
-Like normal connections, the database lets you set how many historical
-connections can be active without generating a warning for a given serial, and
-how many objects should be kept in each connection's object cache.
+Like normal connections, the database lets you set how many total historical
+connections can be active without generating a warning, and
+how many objects should be kept in each historical connection's object cache.
 
     >>> db.getHistoricalPoolSize()
     3
@@ -182,32 +182,72 @@ Let's actually look at these values at work by shining some light into what
 has been a black box up to now.  We'll actually do some white box examination
 of what is going on in the database, pools and connections.
 
-First we'll clean out all the old historical pools so we have a clean slate.
+Historical connections are held in a single connection pool with mappings
+from the ``before`` TID to available connections.  First we'll put a new
+pool on the database so we have a clean slate.
 
     >>> historical_conn.close()
-    >>> db.removeHistoricalPool(at=now)
-    >>> db.removeHistoricalPool(at=historical_serial)
-    >>> db.removeHistoricalPool(before=serial)
+    >>> from ZODB.DB import KeyedConnectionPool
+    >>> db.historical_pool = KeyedConnectionPool(
+    ...     db.historical_pool.size, db.historical_pool.timeout)
 
-Now lets look what happens to the pools when we create an historical
+Now lets look what happens to the pool when we create and close an historical
 connection.
 
-    >>> pools = db._pools
-    >>> len(pools)
-    1
-    >>> pools.keys()
-    ['']
+    >>> pool = db.historical_pool
+    >>> len(pool.all)
+    0
+    >>> len(pool.available)
+    0
     >>> historical_conn = db.open(
     ...     transaction_manager=transaction1, before=serial)
-    >>> len(pools)
-    2
-    >>> set(pools.keys()) == set(('', serial))
+    >>> len(pool.all)
+    1
+    >>> len(pool.available)
+    0
+    >>> historical_conn in pool.all
+    True
+    >>> historical_conn.close()
+    >>> len(pool.all)
+    1
+    >>> len(pool.available)
+    1
+    >>> pool.available.keys()[0] == serial
+    True
+    >>> len(pool.available.values()[0])
+    1
+
+Now we'll open and close two for the same serial to see what happens to the
+data structures.
+
+    >>> historical_conn is db.open(
+    ...     transaction_manager=transaction1, before=serial)
     True
-    >>> pool = pools[serial]
     >>> len(pool.all)
     1
     >>> len(pool.available)
     0
+    >>> transaction2 = transaction.TransactionManager()
+    >>> historical_conn2 = db.open(
+    ...     transaction_manager=transaction2, before=serial)
+    >>> len(pool.all)
+    2
+    >>> len(pool.available)
+    0
+    >>> historical_conn2.close()
+    >>> len(pool.all)
+    2
+    >>> len(pool.available)
+    1
+    >>> len(pool.available.values()[0])
+    1
+    >>> historical_conn.close()
+    >>> len(pool.all)
+    2
+    >>> len(pool.available)
+    1
+    >>> len(pool.available.values()[0])
+    2
 
 If you change the historical cache size, that changes the size of the
 persistent cache on our connection.
@@ -218,26 +258,18 @@ persistent cache on our connection.
     >>> historical_conn._cache.cache_size
     1500
 
-Now let's look at pool sizes.  We'll set it to two, then make and close three
+Now let's look at pool sizes.  We'll set it to two, then open and close three
 connections.  We should end up with only two available connections.
 
     >>> db.setHistoricalPoolSize(2)
 
-    >>> transaction2 = transaction.TransactionManager()
+    >>> historical_conn = db.open(
+    ...     transaction_manager=transaction1, before=serial)
     >>> historical_conn2 = db.open(
     ...     transaction_manager=transaction2, before=serial)
-    >>> len(pools)
-    2
-    >>> len(pool.all)
-    2
-    >>> len(pool.available)
-    0
-
     >>> transaction3 = transaction.TransactionManager()
     >>> historical_conn3 = db.open(
-    ...     transaction_manager=transaction3, before=serial)
-    >>> len(pools)
-    2
+    ...     transaction_manager=transaction3, at=historical_serial)
     >>> len(pool.all)
     3
     >>> len(pool.available)
@@ -248,23 +280,35 @@ connections.  We should end up with only two available connections.
     3
     >>> len(pool.available)
     1
+    >>> len(pool.available.values()[0])
+    1
 
     >>> historical_conn2.close()
     >>> len(pool.all)
     3
     >>> len(pool.available)
     2
+    >>> len(pool.available.values()[0])
+    1
+    >>> len(pool.available.values()[1])
+    1
 
     >>> historical_conn.close()
     >>> len(pool.all)
     2
     >>> len(pool.available)
+    1
+    >>> len(pool.available.values()[0])
     2
 
+Notice it dumped the one that was closed at the earliest time.
+
 Finally, we'll look at the timeout.  We'll need to monkeypatch ``time`` for
 this.  (The funky __import__ of DB is because some ZODB __init__ shenanigans
 make the DB class mask the DB module.)
 
+    >>> db.getHistoricalTimeout()
+    400
     >>> import time
     >>> delta = 200
     >>> def stub_time():
@@ -276,8 +320,6 @@ make the DB class mask the DB module.)
 
     >>> historical_conn = db.open(before=serial)
 
-    >>> len(pools)
-    2
     >>> len(pool.all)
     2
     >>> len(pool.available)
@@ -288,35 +330,22 @@ A close or an open will do garbage collection on the timed out connections.
     >>> delta += 200
     >>> historical_conn.close()
 
-    >>> len(pools)
-    2
     >>> len(pool.all)
     1
     >>> len(pool.available)
     1
-
-An open also does garbage collection on the pools themselves.
-
-    >>> delta += 400
-    >>> conn = db.open() # normal connection
-    >>> len(pools)
+    >>> len(pool.available.values()[0])
     1
-    >>> len(pool.all)
-    0
-    >>> len(pool.available)
-    0
-    >>> serial in pools
-    False
 
 Invalidations
 =============
 
-In general, invalidations are ignored for historical connections, assuming
-that you have really specified a point in history.  This is another white box
+Invalidations are ignored for historical connections. This is another white box
 test.
 
     >>> historical_conn = db.open(
     ...     transaction_manager=transaction1, at=serial)
+    >>> conn = db.open()
     >>> sorted(conn.root().keys())
     ['first', 'second']
     >>> conn.root()['first']['count']
@@ -332,35 +361,13 @@ test.
     0
     >>> historical_conn.close()
 
-If you specify a time in the future, you get a read-only connection that
-invalidates, rather than an error.  The main reason for this is that, in some
-cases, the most recent transaction id is in the future, so there's not an easy
-way to reasonably disallow values.  Beyond that, it's useful to have readonly
-connections, though this spelling isn't quite appealing for the general case.
-This "future history" also works with MVCC.
+Note that if you try to open an historical connection to a time in the future,
+you will get an error.
 
-    >>> THE_FUTURE = datetime.datetime(2038, 1, 19)
-    >>> historical_conn = db.open(
-    ...     transaction_manager=transaction1, at=THE_FUTURE)
-    >>> conn.root()['first']['count'] += 1
-    >>> conn.root()['fourth'] = persistent.mapping.PersistentMapping()
-    >>> transaction.commit()
-    >>> len(historical_conn._invalidated)
-    2
-    >>> historical_conn.root()['first']['count'] # MVCC
-    2
-    >>> historical_conn.sync()
-    >>> len(historical_conn._invalidated)
-    0
-    >>> historical_conn.root()['first']['count']
-    3
-    >>> historical_conn.root()['first']['count'] = 0
-    >>> transaction1.commit()
+    >>> historical_conn = db.open(at=datetime.datetime.utcnow())
     Traceback (most recent call last):
     ...
-    ReadOnlyHistoryError
-    >>> transaction1.abort()
-    >>> historical_conn.close()
+    ValueError: cannot open an historical connection in the future.
 
 Warnings
 ========

src/ZODB/tests/dbopen.txt

@@ -146,7 +146,7 @@ Reaching into the internals, we can see that db's connection pool now has
 two connections available for reuse, and knows about three connections in
 all:
 
-    >>> pool = db._pools['']
+    >>> pool = db.pool
     >>> len(pool.available)
     2
     >>> len(pool.all)
@@ -219,7 +219,7 @@ closed one out of the available connection stack.
     >>> conns = [db.open() for dummy in range(6)]
     >>> len(handler.records)  # 3 warnings for the "excess" connections
     3
-    >>> pool = db._pools['']
+    >>> pool = db.pool
     >>> len(pool.available), len(pool.all)
     (0, 6)
 
@@ -297,7 +297,7 @@ Now open more connections so that the total exceeds pool_size (2):
 
     >>> conn1 = db.open()
     >>> conn2 = db.open()
-    >>> pool = db._pools['']
+    >>> pool = db.pool
     >>> len(pool.all), len(pool.available)  # all Connections are in use
     (3, 0)
 

src/ZODB/tests/testDB.py

@@ -62,85 +62,6 @@ class DBTests(unittest.TestCase):
         self.db.setCacheSize(15)
         self.db.setHistoricalCacheSize(15)
 
-    def test_removeHistoricalPool(self):
-        # Test that we can remove a historical pool
-
-        # This is white box because we check some internal data structures
-
-        serial1, root_serial1 = self.dowork()
-        now = datetime.datetime.utcnow()
-        serial2, root_serial2 = self.dowork()
-        self.failUnless(root_serial1 < root_serial2)
-        c1 = self.db.open(at=now)
-        root = c1.root()
-        root.keys() # wake up object to get proper serial set
-        self.assertEqual(root._p_serial, root_serial1)
-        c1.close() # return to pool
-        c12 = self.db.open(at=now)
-        c12.close() # return to pool
-        self.assert_(c1 is c12) # should be same
-
-        pools = self.db._pools
-
-        self.assertEqual(len(pools), 2)
-        self.assertEqual(nconn(pools), 2)
-
-        self.db.removeHistoricalPool(at=now)
-
-        self.assertEqual(len(pools), 1)
-        self.assertEqual(nconn(pools), 1)
-
-        c12 = self.db.open(at=now)
-        c12.close() # return to pool
-        self.assert_(c1 is not c12) # should be different
-
-        self.assertEqual(len(pools), 2)
-        self.assertEqual(nconn(pools), 2)
-
-    def _test_for_leak(self):
-        self.dowork()
-        now = datetime.datetime.utcnow()
-        self.dowork()
-        while 1:
-            c1 = self.db.open(at=now)
-            self.db.removeHistoricalPool(at=now)
-            c1.close() # return to pool
-
-    def test_removeHistoricalPool_while_connection_open(self):
-        # Test that we can remove a version pool
-
-        # This is white box because we check some internal data structures
-
-        self.dowork()
-        now = datetime.datetime.utcnow()
-        self.dowork()
-        c1 = self.db.open(at=now)
-        c1.close() # return to pool
-        c12 = self.db.open(at=now)
-        self.assert_(c1 is c12) # should be same
-
-        pools = self.db._pools
-
-        self.assertEqual(len(pools), 2)
-        self.assertEqual(nconn(pools), 2)
-
-        self.db.removeHistoricalPool(at=now)
-
-        self.assertEqual(len(pools), 1)
-        self.assertEqual(nconn(pools), 1)
-
-        c12.close() # should leave pools alone
-
-        self.assertEqual(len(pools), 1)
-        self.assertEqual(nconn(pools), 1)
-
-        c12 = self.db.open(at=now)
-        c12.close() # return to pool
-        self.assert_(c1 is not c12) # should be different
-
-        self.assertEqual(len(pools), 2)
-        self.assertEqual(nconn(pools), 2)
-
     def test_references(self):
 
         # TODO: For now test that we're using referencesf.  We really should