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 16/33] Database locking: Tcl: Retry only on DEADLOCK DETECTED
Date: Fri, 8 Jul 2016 19:26:08 +0100	[thread overview]
Message-ID: <1468002385-4407-17-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1468002385-4407-1-git-send-email-ian.jackson@eu.citrix.com>

Use the new errorCode coming out of db-execute* to tell when the error
is that we got a database deadlock, which is the situation in which we
should retry.

This involves combining the two catch blocks, so that there is only
one error handling strategy.  Previously errors on COMMIT would be
retried and others would not.  Now errors anywhere might be retried
but only if the DB indicated deadlock.

We now unconditionally execute ROLLBACK.  This is more correct, since
we always previously executed BEGIN.

And, we pass the errorInfo and errorCode from the $body to the caller.

I have tested this with a test db instance, using contrived means to
generate a database deadlock, and it does actually retry.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tcl/JobDB-Executive.tcl | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/tcl/JobDB-Executive.tcl b/tcl/JobDB-Executive.tcl
index ed9abbb..63db4f0 100644
--- a/tcl/JobDB-Executive.tcl
+++ b/tcl/JobDB-Executive.tcl
@@ -283,25 +283,27 @@ proc transaction {tables script} {
 	    db-execute "SET TRANSACTION ISOLATION LEVEL SERIALIZABLE"
 	    lock-tables $tables
 	    uplevel 1 $script
+	    db-execute COMMIT
 	} result]
-	if {!$rc} {
-	    if {[catch {
-		db-execute COMMIT
-	    } emsg]} {
-		puts "commit failed: $emsg; retrying ..."
-		db-execute ROLLBACK
-		if {[incr retries -1] <= 0} {
-		    error \
- "commit failed, too many retries: $emsg\n$errorInfo\n$errorCode\n"
+	set ei $errorInfo
+	set ec $errorCode
+	if {$rc} {
+	    db-execute ROLLBACK
+	    switch -glob $errorCode {
+		{OSSTEST-PSQL * 40P01} {
+		    # DEADLOCK DETECTED
+		    puts "transaction deadlock ($result) retrying ..."
+		    if {[incr retries -1] <= 0} {
+			error \
+ "transaction failed, too many retries: $result\n$errorInfo\n$errorCode\n"
+		    }
+		    after 500
+		    continue
 		}
-		after 500
-		continue
 	    }
-	} else {
-	    db-execute ROLLBACK
 	}
         db-close
-	return -code $rc $result
+	return -code $rc -errorinfo $ei -errorcode $ec $result
     }
 }
 
-- 
2.1.4


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

  parent reply	other threads:[~2016-07-08 18:26 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-08 18:25 [OSSTEST PATCH 00/33] Database locking and retry Ian Jackson
2016-07-08 18:25 ` [OSSTEST PATCH 01/33] mg-allocate: Fix "issteallable" call Ian Jackson
2016-07-08 18:25 ` [OSSTEST PATCH 02/33] mg-allocate: Do not treat already-allocated resources as satisfactory Ian Jackson
2016-07-08 18:25 ` [OSSTEST PATCH 03/33] mg-schema-test-database: Direct logs to local directory Ian Jackson
2016-07-08 18:25 ` [OSSTEST PATCH 04/33] mg-schema-test-database: Prepare for `daemons' to be cleverer Ian Jackson
2016-07-08 18:25 ` [OSSTEST PATCH 05/33] mg-schema-test-database: Make `daemons' " Ian Jackson
2016-07-08 18:25 ` [OSSTEST PATCH 06/33] mg-schema-test-database: Change default minflight to -100 Ian Jackson
2016-07-08 18:25 ` [OSSTEST PATCH 07/33] invoke-daemon: Honour OSSTEST_DAEMON_TCLSH Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 08/33] Tcl: Use tclsh8.5 Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 09/33] ms-flights-summary: Remove spurious \ in keys \%{ something } Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 10/33] ms-planner: Support ClientNotes Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 11/33] Tcl database debugging: Actually work Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 12/33] Database locking: Tcl: Use db-execute-array Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 13/33] Database locking: Tcl: Use db-execute Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 14/33] Database locking: Tcl: Always use db-execute-array for SELECT Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 15/33] Database locking: Tcl: for errorCode, use pg_exec, not pg_execute Ian Jackson
2016-07-08 18:26 ` Ian Jackson [this message]
2016-07-08 18:26 ` [OSSTEST PATCH 17/33] ms-ownerdaemon: Cope with db restart. Retry recording dead tasks Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 18/33] ms-ownerdaemon: Break out db-reopen, and move it to JobDB-Executive Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 19/33] tcl daemons: Move BEGIN within scope of transaction error trapping Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 20/33] tcl daemons: jobdb::transaction: Improve two message generation sites Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 21/33] tcl daemons: Remove obsolete `global g' Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 22/33] tcl daemons: Break out db-ensure-open and db-ensure-closed Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 23/33] tcl daemons: db-ensure-open, -close: Make idempotent Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 24/33] tcl daemons: make db-reopen actually work Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 25/33] tcl daemons: More info in db--exec-check error Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 26/33] tcl daemons: Recognise `SSL SYSCALL' errors with their own errorCode Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 27/33] tcl daemons: transaction: Properly match db-open and db-close Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 28/33] tcl daemons: if error occurs, ensure db is closed afterwards Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 29/33] tcl daemons: transaction: Only try ROLLBACK when necessary Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 30/33] tcl daemons: transaction: Support db autoreconnect Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 31/33] tcl-daemons: ms-ownerdaemon: Use autoreconnect Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 32/33] tcl daemons: Provide with-db Ian Jackson
2016-07-08 18:26 ` [OSSTEST PATCH 33/33] tcl daemons: Use with-db 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=1468002385-4407-17-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).