Relax assertion in check_tid_ordering_w_commit test

[jmuchemb]

It is pointless for lastTransaction() to block until it is allowed to return the TID of a transaction that has just been committed, because it may still not be the real last TID (e.g. for some storage implementations, invalidations are received from a shared server via the network). While invalidations are still being processed, it's fine to return immediately with the previous last TID.

This was clarified in commit 4a6b028 ("mvccadapter: check if the last TID changed without invalidation").

[jmuchemb]

Found by NEO test suite. No idea how I could miss that.

[jmuchemb]

/cc @navytux

[navytux]

/cc @navytux

@jmuchemb, thanks for heads up. After looking a bit inside I think I understand at least roughly what was going on and is changed by your patch: since 4a6b028 lastTransaction never blocks and returns local state of DB head up to transaction that was fully processed for invalidations locally. Because of that @run_in_thread lastTransaction was not waiting for commit to finish and was returning previous tid instead of what is generated by commit after delay.

However I'm not sure I understand why this test is correct at all: in my view nothing should block load to work while a commit in another thread or client is in progress. Maybe I'm missing something, but my understanding is that this particular property is true only for ZEO (which locks storage on commit), but is not true for storages that allow multi commit, e.g. NEO. In other words does the test still pass with NEO with sufficient increase of delay-during-commit-window

--- a/src/ZODB/tests/BasicStorage.py
+++ b/src/ZODB/tests/BasicStorage.py
@@ -379,7 +379,7 @@ def lastInvalidations():
             while len(attempts) < expected_attempts:
                 attempts_cond.wait()
 
-        time.sleep(.01) # for good measure :)
+        time.sleep(10) # for good measure :)
         finish.set()
 
         for t in to_join:

?

This test was added in c3091e2 by @jimfulton which talks about ordering and "some sort of locking strategy".

We should be fine with ordering. However "some sort of locking strategy" does not imply "the storage is locked for other operations while a commit is in progress", isn't it?

Kirill

[jmuchemb]

does the test still pass with NEO with sufficient increase of delay-during-commit-window [...] ?

Yes, it does. The reason is that the tpc_finish callback is called while the cache is locked, and that load wants to access the cache.

On ZODB < 5, NEO also needs lastTransaction() to be updated with the same lock as the one used to process invalidations (see #307).