[Checkins] [zopefoundation/ZODB] 3a493b: Kill leftovers of pre-MVCC read conflicts

[Kirill Smelkov]

  Branch: refs/heads/master
  Home:   https://github.com/zopefoundation/ZODB
  Commit: 3a493b01087639347283ac78ec41cf6e38f3e9bf
      https://github.com/zopefoundation/ZODB/commit/3a493b01087639347283ac78ec41cf6e38f3e9bf
  Author: Kirill Smelkov <kirr at nexedi.com>
  Date:   2020-07-31 (Fri, 31 Jul 2020)

  Changed paths:
    M src/ZODB/Connection.py
    M src/ZODB/POSException.py
    M src/ZODB/interfaces.py
    M src/ZODB/mvccadapter.py
    M src/ZODB/tests/testZODB.py
    M src/ZODB/tests/testmvcc.py
    M src/ZODB/transact.py

  Log Message:
  -----------
  Kill leftovers of pre-MVCC read conflicts

In the early days, before MVCC was introduced, ZODB used to raise
ReadConflictError on access to object that was simultaneously changed by
another client in concurrent transaction. However, as
doc/articles/ZODB-overview.rst says

	Since Zope 2.8 ZODB has implemented **Multi Version Concurrency Control**.
	This means no more ReadConflictErrors, each transaction is guaranteed to be
	able to load any object as it was when the transaction begun.

So today the only way to get a ReadConflictError should be

  1) at commit time for an object that was requested to stay unchanged
     via checkCurrentSerialInTransaction, and

  2) at plain access time, if a pack running simultaneously to current
     transaction, removes object revision that we try to load.

The second point is a bit unfortunate, since when load discovers that
object was deleted or not yet created, it is logically more clean to
raise POSKeyError. However due to backward compatibility we still want
to raise ReadConflictError in this case - please see comments added to
MVCCAdapter for details.

Anyway, let's remove leftovers of handling regular read-conflicts from
pre-MVCC era:

Adjust docstring of ReadConflictError to explicitly describe that this
error can only happen at commit time for objects requested to be
current, or at plain access if pack is running simultaneously under
connection foot.

There were also leftover code, comment and test bits in Connection,
interfaces, testmvcc and testZODB, that are corrected/removed
correspondingly. testZODB actually had ReadConflictTests that was
completely deactivated: commit b0f992fd ("Removed the mvcc option..."; 2007)
moved read-conflict-on-access related tests out of ZODBTests, but did not
activated moved parts at all, because as that commit says when MVCC is
always on unconditionally, there is no on-access conflicts:

    Removed the mvcc option.  Everybody wants mvcc and removing us lets us
    simplify the code a little. (We'll be able to simplify more when we
    stop supporting versions.)

Today, if I try to manually activate that ReadConflictTests via

    @@ -637,6 +637,7 @@ def __init__(self, poisonedjar):
     def test_suite():
         return unittest.TestSuite((
             unittest.makeSuite(ZODBTests, 'check'),
    +        unittest.makeSuite(ReadConflictTests, 'check'),
             ))

     if __name__ == "__main__":

it fails in dumb way showing that this tests were unmaintained for ages:

    Error in test checkReadConflict (ZODB.tests.testZODB.ReadConflictTests)
    Traceback (most recent call last):
      File "/usr/lib/python2.7/unittest/case.py", line 320, in run
        self.setUp()
      File "/home/kirr/src/wendelin/z/ZODB/src/ZODB/tests/testZODB.py", line 451, in setUp
        ZODB.tests.utils.TestCase.setUp(self)
    AttributeError: 'module' object has no attribute 'utils'

Since today ZODB always uses MVCC and there is no way to get
ReadConflictError on concurrent plain read/write access, those tests
should be also gone together with old pre-MVCC way of handling
concurrency.

/cc @jimfulton
/reviewed-on https://github.com/zopefoundation/ZODB/pull/320
/reviewed-by @jamadden