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.jack...@eu.citrix.com>
---
 Osstest/Executive.pm       | 14 ++++++++++++++
 Osstest/JobDB/Executive.pm |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index c423570..635e5dd 100644
--- 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 5f2aac9..5545849 100644
--- 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
-- 
2.1.4


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

Reply via email to