Executive DB: Eliminate SQL locking for read-only transactions

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 Postgres'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@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Fix minor error in in commit message

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 84c7d4667e67953d9df53f48bbcc29aa4f051c97..d9c698a72abdc2428bef5f4c141b48a00b3b5cc9 100644 (file)
--- 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 138f3f4e5c5bc427d677ac33180d1db52eeb1988..a1f838f51f3b47b11456cf081085973353b7d5ea 100755 (executable)
--- 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 cab32c94f3c016685a859e72fa0b30ad6e7c5ea9..fc65a5ba4d41f95c0f3292b5323065cca95daa1b 100755 (executable)
--- 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 bbe03fe5593f05105063bc0d2a67bf06cc0fbe85..db0d9220e7f07c6444758d896228e8e1dc4b84d3 100755 (executable)
--- 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 79dc519e3cef3386befa8f2984ca86f238b03def..9d39483d61409df170705643b33c08775243427e 100755 (executable)
--- 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 0e2a3f938eb8c049225a4e5d69ae1577b2171d3b..0ca441c40e2265b4a56069135026b35108b63c2e 100755 (executable)
--- 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); }