xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xenproject.org
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Subject: [OSSTEST PATCH 8/9] db retry: Use HandleError and exceptions to detect when to retry
Date: Tue, 20 Dec 2016 18:38:01 +0000	[thread overview]
Message-ID: <1482259082-30767-9-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1482259082-30767-1-git-send-email-ian.jackson@eu.citrix.com>

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>
---
 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

  parent reply	other threads:[~2016-12-20 18:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-20 18:37 [OSSTEST PATCH 0/9] db retry: fixes and workarounds Ian Jackson
2016-12-20 18:37 ` [OSSTEST PATCH 1/9] mg-schema-test-database: Revamp sequence handling Ian Jackson
2016-12-20 18:37 ` [OSSTEST PATCH 2/9] mg-schema-test-database: Wrap some withtest psql_do in subshells Ian Jackson
2016-12-20 18:37 ` [OSSTEST PATCH 3/9] cs-bisection-step: Do not acquire the repo lock Ian Jackson
2016-12-20 18:37 ` [OSSTEST PATCH 4/9] db retry, bisection: Reset %jobs_created on db retry Ian Jackson
2016-12-20 18:37 ` [OSSTEST PATCH 5/9] db retry, bisect: Cache build reuse investigations Ian Jackson
2016-12-20 18:37 ` [OSSTEST PATCH 6/9] db retry: Document $@ as an implicit parameter to need_retry Ian Jackson
2016-12-20 18:38 ` [OSSTEST PATCH 7/9] db retry: Break out Osstest::Executive::JobDB::_need_retry Ian Jackson
2016-12-20 18:38 ` Ian Jackson [this message]
2016-12-20 18:38 ` [OSSTEST PATCH 9/9] db retry: Retry on $dbh->state eq '' Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1482259082-30767-9-git-send-email-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).