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

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Our transactions generally run with
  SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
(which, incidentally, does not mean that the transactions are
necessarily serialisable!)

In SQL in general it is possible for a read-only transaction to fail
and need to be retried because some writer has updated things.

However, in PostgreSQL this is not possible because Postgres uses
multi-version concurrency control: it retains the old version of the
data while the read transaction is open:
  http://www.postgresql.org/docs/8.3/static/transaction-iso.html

(And, of course, SQLite uses MVCC too, and all transactions in SQLite
are fully serialisable.)

So it is not necessary for these read-only operations to take out
locks.  When they do so they can unnecessarily block other important
work for long periods of time.

With this change, we go further from the ability to support databases
other than PostgreSQL and SQLite.  However, such support was very
distant anyway because of differences in SQL syntax and semantics, our
reliance in Executive mode on sql's command line utilities, and so on.

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>
---
 Osstest/Executive.pm   |    6 ++++--
 cr-ensure-disk-space   |    2 +-
 ms-planner             |    3 ++-
 sg-report-flight       |    2 +-
 sg-report-host-history |    8 ++++----
 sg-report-job-history  |    2 +-
 6 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 84c7d46..d9c698a 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -90,8 +90,10 @@ our (@all_lock_tables) = qw(flights resources);
 #
 # READS:
 #
-#  Nontransactional reads are also permitted
-#  Transactional reads must take out locks as if they were modifying
+#  Nontransactional and transactional reads are also permitted
+#  and do not need to take out any locks.  (We support only databases
+#  using multi-version concurrency control, which includes PostgreSQL
+#  and SQLite.)
 
 augmentconfigdefaults(
     ControlDaemonHost => 'control-daemons',
diff --git a/cr-ensure-disk-space b/cr-ensure-disk-space
index 138f3f4..a1f838f 100755
--- a/cr-ensure-disk-space
+++ b/cr-ensure-disk-space
@@ -145,7 +145,7 @@ END
     return 1;
 }
 
-db_retry($dbh_tests,[qw(flights)], sub {
+db_retry($dbh_tests,[], sub {
     @flights = ();
     for (;;) {
        iteration_continue()
diff --git a/ms-planner b/ms-planner
index cab32c9..fc65a5b 100755
--- a/ms-planner
+++ b/ms-planner
@@ -57,7 +57,8 @@ my $fn= "data-$walker.pl";
 
 sub allocations ($$) {
     my ($init, $each) = @_;
-    db_retry($dbh_tests, \@all_lock_tables, sub {
+
+    db_retry($dbh_tests, [], sub {
        $init->();
        
        our $resources_q ||= $dbh_tests->prepare(<<END);
diff --git a/sg-report-flight b/sg-report-flight
index bbe03fe..db0d922 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -1284,7 +1284,7 @@ END
     rename "$htmlout.new", $htmlout or die $!;
 }
 
-db_begin_work($dbh_tests, [qw(flights)]);
+db_begin_work($dbh_tests, []);
 findspecflight();
 my $fi= examineflight($specflight);
 my @fails= justifyfailures($fi);
diff --git a/sg-report-host-history b/sg-report-host-history
index 79dc519..9d39483 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -259,7 +259,7 @@ END
     rename "$html_file.new", "$html_file" or die "$html_file $!";
 }
 
-db_retry($dbh_tests, [qw(flights resources)], sub {
+db_retry($dbh_tests, [], sub {
     computeflightsrange();
 });
 
@@ -271,7 +271,7 @@ $dbh_tests->do("SET LOCAL enable_seqscan=false");
 foreach my $host (@ARGV) {
     if ($host =~ m/^flight:/) {
        my $flight=$'; #';
-       db_retry($dbh_tests, [qw(flights)], sub {
+       db_retry($dbh_tests, [], sub {
            our $hostsinflightq //= db_prepare(<<END);
                SELECT DISTINCT val
                  FROM runvars
@@ -292,12 +292,12 @@ END
 
 exit 0 unless %hosts;
 
-db_retry($dbh_tests, [qw(flights)], sub {
+db_retry($dbh_tests, [], sub {
     mainquery();
 });
 
 foreach my $host (sort keys %hosts) {
-    db_retry($dbh_tests, [qw(flights)], sub {
+    db_retry($dbh_tests, [], sub {
        reporthost $host;
     });
 }
diff --git a/sg-report-job-history b/sg-report-job-history
index 0e2a3f9..0ca441c 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -298,5 +298,5 @@ sub processjob ($) {
     processjobbranch($j,$_) foreach @branches;
 }
 
-db_begin_work($dbh_tests, [qw(flights)]);
+db_begin_work($dbh_tests, []);
 foreach my $j (@jobs) { processjob($j); }
-- 
1.7.10.4


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