Hi all.
Here is a proposed patch which enables cancellation of $subject. The
problematic point about doing so is that the client is not expecting any
messages from the server when its in an idle state during an transaction and
that simply suppressing that message is not enough as ready for query messages
would get sent out at unexpected places.
Thus the first patch adds the possibility to add flags to ereport()s error
level
to suppress notifying the client.
It also switches the earlier "COMERROR" log level over to this flag
(LOG_NO_CLIENT) with a typedef to support the old method.
The second patch sets up a variable "silent_error_while_idle" when its
cancelling an idle txn which suppresses sending out messages at wrong places
and resets its after having read a command in the simple protocol and after
having read a 'sync' message in the extended protocol.
Currently it does *not* report any special error message to the client if it
starts sending commands in an (unbekownst to it) failed transaction, but just
the normal "25P02: current transaction is aborted..." message.
It shouldn't be hard to add that and I will propose a patch if people would
like it (I personally am not very interested, but I can see people validly
wanting it), but I would like to have some feedback on the patch first.
Greetings,
Andres
From 150b0bc845f7aab9629cc169a3a73f3338d6ab7f Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sat, 13 Feb 2010 22:21:15 +0100
Subject: [PATCH 1/2] Support transporting flags in the 'elevel' argument of ereport(). The
reason is the wish to support avoiding the error to the client which
was only possible using the COMERROR level - which is not a error but
just a log message.
The only supported flag till now is LOG_NO_CLIENT which does what
COMERROR did for all error levels
---
src/backend/utils/error/elog.c | 6 ++++--
src/include/utils/elog.h | 29 ++++++++++++++++++++---------
2 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index e321b99..5fc21b4 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -214,7 +214,8 @@ errstart(int elevel, const char *filename, int lineno,
bool output_to_server;
bool output_to_client = false;
int i;
-
+ int eflags = elevel & LOG_FLAG_MASK;
+ elevel = elevel & ~LOG_FLAG_MASK;
/*
* Check some cases in which we want to promote an error into a more
* severe error. None of this logic applies for non-error messages.
@@ -274,7 +275,7 @@ errstart(int elevel, const char *filename, int lineno,
output_to_server = (elevel >= log_min_messages);
/* Determine whether message is enabled for client output */
- if (whereToSendOutput == DestRemote && elevel != COMMERROR)
+ if (whereToSendOutput == DestRemote && !(eflags & LOG_NO_CLIENT))
{
/*
* client_min_messages is honored only after we complete the
@@ -333,6 +334,7 @@ errstart(int elevel, const char *filename, int lineno,
edata = &errordata[errordata_stack_depth];
MemSet(edata, 0, sizeof(ErrorData));
edata->elevel = elevel;
+ edata->eflags = eflags;
edata->output_to_server = output_to_server;
edata->output_to_client = output_to_client;
edata->filename = filename;
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 92641ba..1853e1d 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -16,6 +16,13 @@
#include <setjmp.h>
+#define LOG_FLAG_MASK ((~0)<<20)
+
+/* logging flags */
+#define LOG_NO_CLIENT (2<<30) /* Don't send to the client. Helpfull
+ * if it would confuse the client at
+ * that moment */
+
/* Error level codes */
#define DEBUG5 10 /* Debugging messages, in categories of
* decreasing detail. */
@@ -25,30 +32,33 @@
#define DEBUG1 14 /* used by GUC debug_* variables */
#define LOG 15 /* Server operational messages; sent only to
* server log by default. */
-#define COMMERROR 16 /* Client communication problems; same as LOG
+#define COMMERROR (LOG|LOG_NO_CLIENT)/* Client communication problems; same as LOG
* for server reporting, but never sent to
- * client. */
-#define INFO 17 /* Messages specifically requested by user (eg
+ * client. For backward compatibility this is
+ * is available without explicitly specifying
+ * LOG_NO_CLIENT.*/
+#define INFO 16 /* Messages specifically requested by user (eg
* VACUUM VERBOSE output); always sent to
* client regardless of client_min_messages,
* but by default not sent to server log. */
-#define NOTICE 18 /* Helpful messages to users about query
+#define NOTICE 17 /* Helpful messages to users about query
* operation; sent to client and server log by
* default. */
-#define WARNING 19 /* Warnings. NOTICE is for expected messages
+#define WARNING 18 /* Warnings. NOTICE is for expected messages
* like implicit sequence creation by SERIAL.
* WARNING is for unexpected messages. */
-#define ERROR 20 /* user error - abort transaction; return to
+#define ERROR 19 /* user error - abort transaction; return to
* known state */
/* Save ERROR value in PGERROR so it can be restored when Win32 includes
* modify it. We have to use a constant rather than ERROR because macros
* are expanded only when referenced outside macros.
*/
#ifdef WIN32
-#define PGERROR 20
+#define PGERROR 19
#endif
-#define FATAL 21 /* fatal error - abort process */
-#define PANIC 22 /* take down the other backends with me */
+#define FATAL 20 /* fatal error - abort process */
+#define PANIC 21 /* take down the other backends with me */
+
/* #define DEBUG DEBUG1 */ /* Backward compatibility with pre-7.3 */
@@ -291,6 +301,7 @@ extern PGDLLIMPORT sigjmp_buf *PG_exception_stack;
typedef struct ErrorData
{
int elevel; /* error level */
+ int eflags; /* error flags */
bool output_to_server; /* will report to server log? */
bool output_to_client; /* will report to client? */
bool show_funcname; /* true to force funcname inclusion */
--
1.7.3.rc1.5.g73aa2
From 6a27a19395df053112616b83c966f4b20869ebc3 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Fri, 21 May 2010 20:17:11 +0200
Subject: [PATCH 2/2] Idle transaction cancellation.
---
src/backend/tcop/postgres.c | 75 +++++++++++++++++++++++++++++++-----------
1 files changed, 55 insertions(+), 20 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index cba90a9..d0c8696 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -177,6 +177,12 @@ static bool RecoveryConflictPending = false;
static bool RecoveryConflictRetryable = true;
static ProcSignalReason RecoveryConflictReason;
+/*
+ * Are we disallowed from sending a "ready for query" message right
+ * now because it would confuse the frontend?
+ */
+bool silent_error_while_idle = false;
+
/* ----------------------------------------------------------------
* decls for routines only used in this file
* ----------------------------------------------------------------
@@ -2877,6 +2883,8 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
void
ProcessInterrupts(void)
{
+ int error = ERROR;
+
/* OK to accept interrupt now? */
if (InterruptHoldoffCount != 0 || CritSectionCount != 0)
return;
@@ -2949,18 +2957,24 @@ ProcessInterrupts(void)
RecoveryConflictPending = false;
DisableNotifyInterrupt();
DisableCatchupInterrupt();
- if (DoingCommandRead)
- ereport(FATAL,
- (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
- errmsg("terminating connection due to conflict with recovery"),
- errdetail_recovery_conflict(),
- errhint("In a moment you should be able to reconnect to the"
- " database and repeat your command.")));
- else
- ereport(ERROR,
- (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
- errmsg("canceling statement due to conflict with recovery"),
- errdetail_recovery_conflict()));
+
+ if (DoingCommandRead){
+ /*
+ * We cant issue a normal ERROR here because the
+ * client doesnt expect the server to send an error at
+ * that point.
+ * We also may not send a "ready for query"/Z message
+ * because that would be unexpected as well.
+ */
+ silent_error_while_idle = true;
+ error |= LOG_NO_CLIENT;
+
+ }
+
+ ereport(error,
+ (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+ errmsg("canceling statement due to conflict with recovery"),
+ errdetail_recovery_conflict()));
}
/*
@@ -2968,15 +2982,18 @@ ProcessInterrupts(void)
* request --- sending an extra error message won't accomplish
* anything. Otherwise, go ahead and throw the error.
*/
- if (!DoingCommandRead)
+ if (DoingCommandRead)
{
- ImmediateInterruptOK = false; /* not idle anymore */
- DisableNotifyInterrupt();
- DisableCatchupInterrupt();
- ereport(ERROR,
- (errcode(ERRCODE_QUERY_CANCELED),
- errmsg("canceling statement due to user request")));
+ silent_error_while_idle = true;
+ error |= LOG_NO_CLIENT;
}
+
+ ImmediateInterruptOK = false; /* not idle anymore */
+ DisableNotifyInterrupt();
+ DisableCatchupInterrupt();
+ ereport(error,
+ (errcode(ERRCODE_QUERY_CANCELED),
+ errmsg("canceling statement due to user request")));
}
/* If we get here, do nothing (probably, QueryCancelPending was reset) */
}
@@ -3794,7 +3811,7 @@ PostgresMain(int argc, char *argv[], const char *username)
* uncommitted updates (that confuses autovacuum). The notification
* processor wants a call too, if we are not in a transaction block.
*/
- if (send_ready_for_query)
+ if (send_ready_for_query && !silent_error_while_idle)
{
if (IsAbortedTransactionBlockState())
{
@@ -3819,6 +3836,7 @@ PostgresMain(int argc, char *argv[], const char *username)
send_ready_for_query = false;
}
+
/*
* (2) Allow asynchronous signals to be executed immediately if they
* come in while we are waiting for client input. (This must be
@@ -3854,6 +3872,23 @@ PostgresMain(int argc, char *argv[], const char *username)
if (ignore_till_sync && firstchar != EOF)
continue;
+ if(silent_error_while_idle && !doing_extended_query_message)
+ {
+ /*
+ * When using the simple protocol:
+ * At this point we have processed a ReadCommand which means
+ * that we got some input from the frontend and thus can start
+ * spewing errors again
+
+ * When using the extended protocol:
+ * doing_extended_query_message is false after a 'sync'
+ * message - which is exactly the point after which we can
+ * start sending errors again.
+ */
+ silent_error_while_idle = false;
+ }
+
+
switch (firstchar)
{
case 'Q': /* simple query */
--
1.7.3.rc1.5.g73aa2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
