db retry: Use HandleError and exceptions to detect when to retry

It appears that sometimes, $dbh->state could be overwritten before
$mjobdb->need_retry got to run.  $dbh->err and $@ would still be
right.  I have not been able to explain this; I suspect that there is
something exciting going on in the eval exception trapping.

To try to isolate the problem, I developed this different approach: we
use the HandleError database handle feature, to cause all the
retriable errors to be thrown as a dedicated exception class.
We can then simply test ref $@.  (We don't care about subclassing
here.  And we don't want isa because $@ might just be a string.)
This is, in general, more reliable, as it cannot treat any other
failures as db retry failures.

Osstest::Executive and Osstest::JobDB::Executive become slightly more
intertwined with this patch.

Sadly this does not seem to completely eliminate the problem.  It does
allow us to present clearer evidence of problems in the underlying
DBI, DBD or PostgreSQL layers...

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index c4235704906c3a9b1a379e18caa7163fc58d73b7..635e5dde9f9338e76a26c5f4f7abaefaaea3992c 100644 (file)
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -245,6 +245,8 @@ sub db_pg_dsn ($) {
     return $pg;
 }
 
+use Exception::Class ( 'Osstest::JobDB::Executive::RetryException' );
+
 sub opendb ($) {
     my ($dbname) = @_;
 
@@ -252,6 +254,18 @@ sub opendb ($) {
 
     my $dbh= DBI->connect("dbi:Pg:$pg", '','', {
         AutoCommit => 1,
+        HandleError => sub {
+            my ($emsg,$edbh,$firstval) = @_;
+            if (Osstest::JobDB::Executive::_need_retry($edbh)) {
+                chomp $emsg;
+                printf STDERR "DB confict (err=%s state=%s): %s\n",
+                    ($edbh->err // 'undef'),
+                    $edbh->state,
+                    $emsg;
+                die new Osstest::JobDB::Executive::RetryException $emsg;
+            }
+            undef;
+        },
         RaiseError => 1,
         ShowErrorStatement => 1,
         })

diff --git a/Osstest/JobDB/Executive.pm b/Osstest/JobDB/Executive.pm
index 5f2aac9e015b18d40e66b47bec4b81d3d495762a..5545849bcf47fbaca3f75cafbb5fbb8892baa37b 100644 (file)
--- a/Osstest/JobDB/Executive.pm
+++ b/Osstest/JobDB/Executive.pm
@@ -111,7 +111,7 @@ sub _need_retry ($) {
 
 sub need_retry ($$$) {
     my ($jd, $dbh,$committing) = @_; # implicitly, $@ is an argument too
-    return _need_retry($dbh_tests);
+    return ref($@) eq 'Osstest::JobDB::Executive::RetryException';
 }
 
 sub readonly_report ($$) { #method