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 15/33] Database locking: Tcl: for errorCode, use pg_exec, not pg_execute
Date: Fri, 8 Jul 2016 19:26:07 +0100	[thread overview]
Message-ID: <1468002385-4407-16-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1468002385-4407-1-git-send-email-ian.jackson@eu.citrix.com>

We would like to be able to retry db transactions.  To do this we need
to know why they failed (if they did).

But pg_execute does not set errorCode.  (This is clearly a bug.)  And
since it immediately discards a failed statement, any error
information has been lost by the time pg_execute returns.

So, instead, use pg_exec, and manually mess about with fishing
suitable information out of a failed statement handle, and generating
an appropriate errorCode.

There are no current consumers of this errorCode: that will come in a
moment.

A wrinkle is that as a result it is no longer possible to use
db-execute on a SELECT statement nor db-execute-array on a non-SELECT
statement.  This is because 1. the `ok' status that we have to
check for is different for statements which are commands and ones
which return tuples and 2. we need to fish a different return value out
of the statement handle (-cmdTuples vs -numTuples).  But all uses in
the codebase are now fine for this distinction.

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

diff --git a/tcl/JobDB-Executive.tcl b/tcl/JobDB-Executive.tcl
index bbce6fc..ed9abbb 100644
--- a/tcl/JobDB-Executive.tcl
+++ b/tcl/JobDB-Executive.tcl
@@ -121,13 +121,61 @@ proc db-execute-debug {stmt} {
 	puts stderr "EXECUTING >$stmt<"
     }
 }
+
+proc db--exec-check {shvar stmt expected_status body} {
+    # pg_execute does not set errorCode and it throws away the
+    # statement handle so we can't get the error out.  So
+    # use pg_exec, as wrapped up here.
+
+    # db--exec-check executes stmt and checks that the status is
+    # `expected_status'.  If OK, executes body with $shvar set to the
+    # stmt handle.   Otherwise throws with errorCode
+    #   {OSSTEST-PSQL <pg-status> <pg-sqlstate>}
+
+    global errorInfo errorCode
+    upvar 1 $shvar sh
+
+    set sh [pg_exec dbh $stmt]
+
+    set rc [catch {
+	set status [pg_result $sh -status]
+	if {[string compare $status $expected_status]} {
+	    set emsg [pg_result $sh -error]
+	    set sqlstate [pg_result $sh -error sqlstate]
+	    if {![string length $emsg]} {
+		set emsg "osstest expected status $expected_status got $status"
+	    }
+	    set context [pg_result $sh -error context]
+	    error $emsg \
+		"    while executing SQL\n$stmt\n    in SQL context\n$context" \
+		[list OSSTEST-PSQL $status $sqlstate]
+	}
+	uplevel 1 $body
+    } emsg]
+
+    set ei $errorInfo
+    set ec $errorCode
+    catch { pg_result $sh -clear }
+
+    return -code $rc -errorinfo $ei -errorcode $ec $emsg
+}
+
 proc db-execute {stmt} {
     db-execute-debug $stmt
-    uplevel 1 [list pg_execute dbh $stmt]
+    db--exec-check sh $stmt PGRES_COMMAND_OK {
+	return [pg_result $sh -cmdTuples]
+    }
 }
-proc db-execute-array {arrayvar stmt args} {
+proc db-execute-array {arrayvar stmt {body {}}} {
     db-execute-debug $stmt
-    uplevel 1 [list pg_execute -array $arrayvar dbh $stmt] $args
+    db--exec-check sh $stmt PGRES_TUPLES_OK {
+	set nrows [pg_result $sh -numTuples]
+	for {set row 0} {$row < $nrows} {incr row} {
+	    uplevel 1 [list pg_result $sh -tupleArray $row $arrayvar]
+	    uplevel 1 $body
+	}
+	return $nrows
+    }
 }
 
 proc lock-tables {tables} {
-- 
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 ` Ian Jackson [this message]
2016-07-08 18:26 ` [OSSTEST PATCH 16/33] Database locking: Tcl: Retry only on DEADLOCK DETECTED Ian Jackson
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-16-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).