commit 66ea2ed46e7d2ce7202c1cec1e0bd0377d19b0c9
author Dave Borowitz <dborowitz@google.com> Thu Apr 20 10:48:29 2017 -0400
committer Dave Borowitz <dborowitz@google.com> Tue May 02 16:25:20 2017 -0400
tree 67734907751141f81bc2870e80c9319abaaa224e
parent 521f78f700e2924c30763175ee91445a30bb7aca

Add a helper for retrying BatchUpdates under NoteDb

With a SQL or SQL-like backend for ReviewDb, two transactions on the
same Change entity initiated around the same time will generally both
succeed, due to the low-level implementation waiting for a lock or
retrying. NoteDb, being Git-backed, has no notion of locking, and the
only atomic operation is a compare-and-swap. This means that concurrent
writes carry a higher risk of exceptions in the Gerrit level when
compared with ReviewDb, and it will be worth it to implement a retrying
mechanism in Gerrit.

The question becomes: what is the appropriate unit of work to retry? The
implementation in this change encourages retrying at the highest level
of an entire end-user operation, like a REST API handler.

The main reason not to limit retrying to a lower level, like a single
BatchUpdate or its Ops, is that the op implementations may depend on
repository state that was read prior to entering the retry loop. This
potentially includes pretty much any caller of
BatchUpdate#setRepository, but the most notable is MergeOp: the initial
branch tips, which are ultimately used as old IDs in the final ref
updates, are read outside of the BatchUpdate. If we retried the
BatchUpdate on LOCK_FAILURE but not the outer code, retrying would be
guaranteed to fail.

The next question is: under what conditions should we retry? The safest
approach, implemented here, is to look specifically for LOCK_FAILUREs
only in the disabled-ReviewDb case, and only when the underlying ref
backend performs atomic multi-ref transactions. If transactions are not
atomic, then it is infeasible to find out which portions of the code
would need to be retried; if they are atomic, then we can assume that a
failed transaction means the operation had no side effects, so retrying
is safe.

There is certainly an argument to be made that it may be worth retrying
even after non-atomic partially-successful operations, under the
assumption that if an error propagates back to the user, probably the
next thing they were going to anyway is just retry manually. But
decisions about when to loosen up our initially tight safety assumptions
can be deferred.

Change-Id: Ic7a9df9ba1bfdb01784cd1fce2b2ce82511e1068