Re: [Xen-devel] [OSSTEST PATCH 1/2] Executive DB: Eliminate SQL locking for read-only transactions

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [OSSTEST PATCH 1/2] Executive DB: Eliminate SQL 
locking for read-only transactions"):
> On Fri, 2015-12-11 at 16:23 +0000, Ian Jackson wrote:
> > We retain the db_retry framing because (a) although the retry loop is
> > not necessary in these cases, the transaction framing is (b) it will
> > make it slightly easier to reverse this decision in the future if we
> > ever decide to do so (c) it is less code churn.
> > 
> > Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> 
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 
> (I haven't actually checked that everywhere you have changed is actually
> r/o

Justifying that claim, from the diffstat:


Comment only:

 Osstest/Executive.pm   |    6 ++++--


These two I have checked, but their function is indeed RO:

 cr-ensure-disk-space   |    2 +-
 ms-planner             |    3 ++-

(Note that `ms-planner' just manipulates data-plan.pl et al; it is
left to the clients to actually obtain resources in the DB.)


These are clearly RO:

 sg-report-flight       |    2 +-
 sg-report-host-history |    8 ++++----
 sg-report-job-history  |    2 +-


> nor whether you have found every r/o operation).

I may have missed some, but the obvious potential remaining
long-running lockholder would be cs-bisection-step.  I have looked at
cs-bisection step and:

 - It does not seem to take any lock, or even begin a transaction, for
   its primary archaeology function.  I think this is fine because if
   some test results or whatever show up later it ought not to break
   the algorithm.

 - The flight construction _is_ within a lock, in populateflight, but
   it basically just clones a template and merges in information from
   the archaeologist part.

 - There is the check in `checkrepetition' - the flail test.  This is
   not a trivial transaction and it would be better if it didn't take
   the lock.  It is only not read-only because it ends, in the error
   case, with an UPDATE to set the flight broken.  This could be in a
   separate transaction given that no-one else is supposed to be
   messing about with the flight that cs-bisection-step is
   constructing.  But this is not a trivial change.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel