Hi Simon, Hi all,
I am not sure this is 9.0 material - even if I would like it to get
included... If HS wouldnt be new I wouldnt even consider suggesting it, but as
its stands its a pretty small change...
It simply allows queries involving subtransaction not to get FATALed but
canceled. This works by doing the recursive abort you erlier did in
ProcessInterrupts in the PostgresMain where there cannot be any references
higher up in the chain.
It would likely be sensible to check that errorcode in some PLs. I have code
for that but I dont think its sensible to continue on those before the
approach is agreed uppon.
I would like to get the part about a seperate error code for HS cancellations
to get commited independently. Its kinda sensible for a client to accept
specifically about such a cancellation and requiring them to play around with
the message...
Currently its called ERRCODE_QUERY_CANCELED_HS but perhaps
ERRCODE_QUERY_CANCELED_STANDBY_CONFLICT or such would be better. I dont really
know how errorcodes gets assigned, so I picked one which is likely wrong...
Additionally there is a very small cleanup removing the errno saving from
RecoveryConflictInterrupt - its somewhat incomplete (I think harmlessly
though) and more importantly the only caller in procsignal.c already does
that...
Andres
From 0dc602ca41fee1ca8bd85056add8bcb8f44bf510 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sun, 14 Feb 2010 11:24:45 +0100
Subject: [PATCH] Dont try to save the errno in RecoveryConflictInterrupt to avoid confusion.
In the current state the errno saving in there is confusing as there
are several returns ignoring to set it. I think its currently harmless
as there should be no changes to errno at those places - beside that
the only caller (procsignal_sigusr1_handler) already saves it.
---
src/backend/tcop/postgres.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8c0c8b9..3505fc4 100644
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
*************** SigHupHandler(SIGNAL_ARGS)
*** 2753,2763 ****
void
RecoveryConflictInterrupt(ProcSignalReason reason)
{
- int save_errno = errno;
-
/*
! * Don't joggle the elbow of proc_exit
! */
if (!proc_exit_inprogress)
{
RecoveryConflictReason = reason;
--- 2753,2761 ----
void
RecoveryConflictInterrupt(ProcSignalReason reason)
{
/*
! * Don't joggle the elbow of proc_exit
! */
if (!proc_exit_inprogress)
{
RecoveryConflictReason = reason;
*************** RecoveryConflictInterrupt(ProcSignalReas
*** 2856,2863 ****
ProcessInterrupts();
}
}
-
- errno = save_errno;
}
/*
--- 2854,2859 ----
--
1.6.5.12.gd65df24
From 9d421934f737f607d550d95f5e64b630c143f013 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Mon, 11 Jan 2010 17:43:01 +0100
Subject: [PATCH 1/2] Add a new error code ERRCODE_QUERY_CANCELED_HS for use with HS indicating a failure
that is more than a plain ERRCODE_QUERY_CANCELED - namely it should not be caught from
various places like savepoints and in PLs.
In want for a better name.
---
src/backend/tcop/postgres.c | 4 ++--
src/include/utils/errcodes.h | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8c0c8b9..67fa16b 100644
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
*************** ProcessInterrupts(void)
*** 2947,2959 ****
*/
silent_error_while_idle = true;
ereport(ERROR | LOG_NO_CLIENT,
! (errcode(ERRCODE_QUERY_CANCELED),
errmsg("canceling statement due to conflict with recovery"),
errdetail_recovery_conflict()));
}
else{
ereport(ERROR,
! (errcode(ERRCODE_QUERY_CANCELED),
errmsg("canceling statement due to conflict with recovery"),
errdetail_recovery_conflict()));
}
--- 2947,2959 ----
*/
silent_error_while_idle = true;
ereport(ERROR | LOG_NO_CLIENT,
! (errcode(ERRCODE_QUERY_CANCELED_HS),
errmsg("canceling statement due to conflict with recovery"),
errdetail_recovery_conflict()));
}
else{
ereport(ERROR,
! (errcode(ERRCODE_QUERY_CANCELED_HS),
errmsg("canceling statement due to conflict with recovery"),
errdetail_recovery_conflict()));
}
diff --git a/src/include/utils/errcodes.h b/src/include/utils/errcodes.h
index 52c09ca..279f0e4 100644
*** a/src/include/utils/errcodes.h
--- b/src/include/utils/errcodes.h
***************
*** 328,333 ****
--- 328,334 ----
/* Class 57 - Operator Intervention (class borrowed from DB2) */
#define ERRCODE_OPERATOR_INTERVENTION MAKE_SQLSTATE('5','7', '0','0','0')
#define ERRCODE_QUERY_CANCELED MAKE_SQLSTATE('5','7', '0','1','4')
+ #define ERRCODE_QUERY_CANCELED_HS MAKE_SQLSTATE('5','7', '0','1','5')
#define ERRCODE_ADMIN_SHUTDOWN MAKE_SQLSTATE('5','7', 'P','0','1')
#define ERRCODE_CRASH_SHUTDOWN MAKE_SQLSTATE('5','7', 'P','0','2')
#define ERRCODE_CANNOT_CONNECT_NOW MAKE_SQLSTATE('5','7', 'P','0','3')
--
1.6.5.12.gd65df24
From afb2b7c7cb198f79d593c70def859e35b7a45ef1 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sun, 14 Feb 2010 23:59:21 +0100
Subject: [PATCH 2/2] Allow HS conflict cancellations to abort subtransactions.
Nested transactions were earlier FATALed because there could be
references to some level. This patch solved this by only aborting
recursively in the MainLoop where its sure that there are no other
errors.
As the the conflict cancellation now has its own error code this is
easy to recognize. Its likely a good idea to check in the procedual
language's exception handlers whether the error is a conflict one and
not allow those to get catched.
---
src/backend/access/transam/xact.c | 46 +++++++++++++++++++++++++++++++++++++
src/backend/tcop/postgres.c | 40 +++++++++++++++++++-------------
src/include/access/xact.h | 2 +
3 files changed, 72 insertions(+), 16 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 92e4146..d31ed3e 100644
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** AbortOutOfAnyTransaction(void)
*** 3727,3732 ****
--- 3727,3778 ----
}
/*
+ * AbortTransactionAndAnySubtransactions
+ *
+ * Similar to AbortCurrentTransaction but if any subtransactions
+ * in progress we abort them *and* all of their parents. So this is
+ * used when the caller wishes to make the abort untrappable by the user.
+ * After this has run IsAbortedTransactionBlockState() will be true.
+ *
+ * May only be run if nobody else references the subtransactions.
+ */
+ void
+ AbortTransactionAndAnySubtransactions(void)
+ {
+ while(true){
+ TransactionState s = CurrentTransactionState;
+
+ switch (s->blockState)
+ {
+ case TBLOCK_DEFAULT:
+ case TBLOCK_STARTED:
+ case TBLOCK_BEGIN:
+ case TBLOCK_INPROGRESS:
+ case TBLOCK_END:
+ case TBLOCK_ABORT:
+ case TBLOCK_SUBABORT:
+ case TBLOCK_ABORT_END:
+ case TBLOCK_ABORT_PENDING:
+ case TBLOCK_PREPARE:
+ case TBLOCK_SUBABORT_END:
+ case TBLOCK_SUBABORT_RESTART:
+ AbortCurrentTransaction();
+ return;
+ break;
+
+ case TBLOCK_SUBINPROGRESS:
+ case TBLOCK_SUBBEGIN:
+ case TBLOCK_SUBEND:
+ case TBLOCK_SUBABORT_PENDING:
+ case TBLOCK_SUBRESTART:
+ AbortSubTransaction();
+ CleanupSubTransaction();
+ break;
+ }
+ }
+ }
+
+ /*
* IsTransactionBlock --- are we within a transaction block?
*/
bool
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 67fa16b..b7b0f83 100644
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
*************** RecoveryConflictInterrupt(ProcSignalReas
*** 2794,2804 ****
return;
/*
! * If we can abort just the current subtransaction then we
! * are OK to throw an ERROR to resolve the conflict. Otherwise
! * drop through to the FATAL case.
*
- * XXX other times that we can throw just an ERROR *may* be
* PROCSIG_RECOVERY_CONFLICT_LOCK
* if no locks are held in parent transactions
*
--- 2794,2808 ----
return;
/*
! * If we already aborted then we no longer need to
! * cancel. We do not want to do this if were just
! * in an aborted subtransaction because whatever
! * the conflicting object was it might be locked
! * from an a txn higher in the chain.
! *
! * XXX in some situations we *may* not need to kill the
! * whole chain:
*
* PROCSIG_RECOVERY_CONFLICT_LOCK
* if no locks are held in parent transactions
*
*************** RecoveryConflictInterrupt(ProcSignalReas
*** 2809,2824 ****
* PROCSIG_RECOVERY_CONFLICT_TABLESPACE
* if no temp files or cursors open in parent transactions
*/
! if (!IsSubTransaction())
! {
! /*
! * If we already aborted then we no longer need to cancel.
! * We do this here since we do not wish to ignore aborted
! * subtransactions, which must cause FATAL, currently.
! */
! if (IsAbortedTransactionBlockState())
! return;
!
RecoveryConflictPending = true;
QueryCancelPending = true;
InterruptPending = true;
--- 2813,2821 ----
* PROCSIG_RECOVERY_CONFLICT_TABLESPACE
* if no temp files or cursors open in parent transactions
*/
! if(!IsSubTransaction && IsAbortedTransactionBlockState())
! return;
! else{
RecoveryConflictPending = true;
QueryCancelPending = true;
InterruptPending = true;
*************** PostgresMain(int argc, char *argv[], con
*** 3730,3738 ****
debug_query_string = NULL;
/*
! * Abort the current transaction in order to recover.
*/
! AbortCurrentTransaction();
/*
* Now return to normal top-level context and clear ErrorContext for
--- 3727,3746 ----
debug_query_string = NULL;
/*
! *
! * Abort the current transaction in order to recover. If were
! * in HS this is the only safe point where we can abort not
! * only the current transaction but also all transaction above
! * the current as there exists no higher point to jump to and
! * thus were not playing around with somebodys execution context.
*/
! if(geterrcode() == ERRCODE_QUERY_CANCELED_HS)
! {
! AbortTransactionAndAnySubtransactions();
! }
! else{
! AbortCurrentTransaction();
! }
/*
* Now return to normal top-level context and clear ErrorContext for
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index 67ee39d..a0ae69e 100644
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** extern bool IsTransactionBlock(void);
*** 204,209 ****
--- 204,211 ----
extern bool IsTransactionOrTransactionBlock(void);
extern char TransactionBlockStatusCode(void);
extern void AbortOutOfAnyTransaction(void);
+ extern void AbortTransactionAndAnySubtransactions(void);
+
extern void PreventTransactionChain(bool isTopLevel, const char *stmtType);
extern void RequireTransactionChain(bool isTopLevel, const char *stmtType);
extern bool IsInTransactionChain(bool isTopLevel);
--
1.6.5.12.gd65df24
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
