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>,
	Ian Campbell <ian.campbell@citrix.com>
Subject: [OSSTEST PATCH 5/7] Database locking: Tcl: for errorCode, use pg_exec, not pg_execute
Date: Thu, 7 Jan 2016 19:38:14 +0000	[thread overview]
Message-ID: <1452195496-16016-6-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1452195496-16016-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 the 1. the `ok' status that we have to
check for is different for statements which are commands and ones
which return tupes 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>
---
 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} {
-- 
1.7.10.4

  parent reply	other threads:[~2016-01-07 19:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-07 19:38 [OSSTEST PATCH 0/7] Better database handling in Tcl Ian Jackson
2016-01-07 19:38 ` [OSSTEST PATCH 1/7] Tcl database debugging: Actually work Ian Jackson
2016-01-07 19:38 ` [OSSTEST PATCH 2/7] Database locking: Tcl: Use db-execute-array Ian Jackson
2016-01-07 19:38 ` [OSSTEST PATCH 3/7] Database locking: Tcl: Use db-execute Ian Jackson
2016-01-08  9:28   ` Ian Campbell
2016-01-12 15:38     ` Ian Jackson
2016-01-07 19:38 ` [OSSTEST PATCH 4/7] Database locking: Tcl: Always use db-execute-array for SELECT Ian Jackson
2016-01-07 19:38 ` Ian Jackson [this message]
2016-01-08  9:32   ` [OSSTEST PATCH 5/7] Database locking: Tcl: for errorCode, use pg_exec, not pg_execute Ian Campbell
2016-01-12 15:39     ` Ian Jackson
2016-01-14 10:33       ` Ian Campbell
2016-01-07 19:38 ` [OSSTEST PATCH 6/7] Database locking: Tcl: Retry only on DEADLOCK DETECTED Ian Jackson
2016-01-07 19:38 ` [OSSTEST PATCH 7/7] ms-ownerdaemon: Cope with db restart. Retry recording dead tasks Ian Jackson
2016-01-08  9:40 ` [OSSTEST PATCH 0/7] Better database handling in Tcl Ian Campbell

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=1452195496-16016-6-git-send-email-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=ian.campbell@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).