On Thu, 2009-12-31 at 11:10 +0000, Simon Riggs wrote:
> On Thu, 2009-12-24 at 21:38 +0100, Joachim Wieland wrote:
> > On Sun, Dec 6, 2009 at 4:23 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> > > We are using NOTICE, not NOTIFY, assuming that we use anything at all
> > > (which I still regard as unnecessary).  Please stop injecting confusion
> > > into the discussion.
> > 
> > Attached is a minimal POC patch that allows to cancel an idle
> > transaction with SIGINT. The HS patch also allows this in its current
> > form but as Simon points out the client gets out of sync with it.
> > 
> > The proposal is to send an additional NOTICE to the client and abort
> > all open transactions and subtransactions (this is what I got from the
> > previous discussion).
> 
> This all works and I'm looking to post a reviewed patch soon.

Attached is the patch I intend to commit, barring objections.

This patch extends SIGINT to allow cancellation of transactions while
idle in both HS and normal mode. It also changes the standard message
reported on an idle transaction in aborted state to '<IDLE> in
transaction (aborted)', so that once aborted we keep the message even if
the user tries to issue further statements other than ROLLBACK or
COMMIT.

This also solves the bug reported by Kris Jurka.

Joachim, credit will be to you, so please re-check.

(Further changes pending on HS side, so not all issues resolved by this.
I intend to use this mechanism for HS cancellations when
CONFLICT_MODE_ERROR, and another mechanism for CONFLICT_MODE_FATAL.)

-- 
 Simon Riggs           www.2ndQuadrant.com
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***************
*** 12899,12905 **** SELECT set_config('log_statement_stats', 'off', false);
          <literal><function>pg_cancel_backend</function>(<parameter>pid</parameter> <type>int</>)</literal>
          </entry>
         <entry><type>boolean</type></entry>
!        <entry>Cancel a backend's current query</entry>
        </row>
        <row>
         <entry>
--- 12899,12905 ----
          <literal><function>pg_cancel_backend</function>(<parameter>pid</parameter> <type>int</>)</literal>
          </entry>
         <entry><type>boolean</type></entry>
!        <entry>Cancel a backend's current query or abort an idle transaction</entry>
        </row>
        <row>
         <entry>
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
***************
*** 55,60 **** postgres   965  0.0  1.1  6152 1512 pts/1    SN   13:17   0:00 postgres: stats c
--- 55,61 ----
  postgres   998  0.0  2.3  6532 2992 pts/1    SN   13:18   0:00 postgres: tgl runbug 127.0.0.1 idle
  postgres  1003  0.0  2.4  6532 3128 pts/1    SN   13:19   0:00 postgres: tgl regression [local] SELECT waiting
  postgres  1016  0.1  2.4  6532 3080 pts/1    SN   13:19   0:00 postgres: tgl regression [local] idle in transaction
+ postgres  1066  0.1  2.4  6532 3080 pts/1    SN   13:19   0:00 postgres: tgl regression [local] idle in transaction (aborted)
  </screen>
  
     (The appropriate invocation of <command>ps</> varies across different
***************
*** 77,82 **** postgres: <replaceable>user</> <replaceable>database</> <replaceable>host</> <re
--- 78,84 ----
    the life of the client connection, but the activity indicator changes.
    The activity can be <literal>idle</> (i.e., waiting for a client command),
    <literal>idle in transaction</> (waiting for client inside a <command>BEGIN</> block),
+   <literal>idle in transaction (aborted)</> (waiting for client to <command>ROLLBACK</>),
    or a command type name such as <literal>SELECT</>.  Also,
    <literal>waiting</> is attached if the server process is presently waiting
    on a lock held by another server process.  In the above example we can infer
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
***************
*** 313,320 **** IsTransactionState(void)
  /*
   *	IsAbortedTransactionBlockState
   *
!  *	This returns true if we are currently running a query
!  *	within an aborted transaction block.
   */
  bool
  IsAbortedTransactionBlockState(void)
--- 313,319 ----
  /*
   *	IsAbortedTransactionBlockState
   *
!  *	This returns true if we are within an aborted transaction block.
   */
  bool
  IsAbortedTransactionBlockState(void)
***************
*** 2692,2697 **** AbortCurrentTransaction(void)
--- 2691,2737 ----
  }
  
  /*
+  *	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.
+  */
+ void
+ AbortTransactionAndAnySubtransactions(void)
+ {
+ 	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();
+ 			break;
+ 
+ 		case TBLOCK_SUBINPROGRESS:
+ 		case TBLOCK_SUBBEGIN:
+ 		case TBLOCK_SUBEND:
+ 		case TBLOCK_SUBABORT_PENDING:
+ 		case TBLOCK_SUBRESTART:
+ 			AbortSubTransaction();
+ 			CleanupSubTransaction();
+ 			AbortTransactionAndAnySubtransactions();
+ 			break;
+ 	}
+ }
+ 
+ /*
   *	PreventTransactionChain
   *
   *	This routine is to be called by statements that must not run inside
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
***************
*** 2637,2651 **** StatementCancelHandler(SIGNAL_ARGS)
  	if (!proc_exit_inprogress)
  	{
  		InterruptPending = true;
! 		QueryCancelPending = true;
  
  		/*
! 		 * If it's safe to interrupt, and we're waiting for a lock, service
! 		 * the interrupt immediately.  No point in interrupting if we're
! 		 * waiting for input, however.
  		 */
! 		if (InterruptHoldoffCount == 0 && CritSectionCount == 0 &&
! 			(DoingCommandRead || ImmediateInterruptOK))
  		{
  			/* bump holdoff count to make ProcessInterrupts() a no-op */
  			/* until we are done getting ready for it */
--- 2637,2653 ----
  	if (!proc_exit_inprogress)
  	{
  		InterruptPending = true;
! 
! 		if (!DoingCommandRead)
! 			QueryCancelPending = true;
! 		else if (IsTransactionOrTransactionBlock())
! 			TransactionCancelPending = true;
  
  		/*
! 		 * If it's safe to interrupt, service the interrupt immediately.
  		 */
! 		if (InterruptHoldoffCount == 0 && CritSectionCount == 0 && ImmediateInterruptOK &&
! 			(TransactionCancelPending || QueryCancelPending))
  		{
  			/* bump holdoff count to make ProcessInterrupts() a no-op */
  			/* until we are done getting ready for it */
***************
*** 2789,2794 **** ProcessInterrupts(void)
--- 2791,2820 ----
  					 errmsg("canceling statement due to user request")));
  		}
  	}
+ 	if (TransactionCancelPending)
+ 	{
+ 		TransactionCancelPending = false;
+ 		ImmediateInterruptOK = false;	/* not idle anymore */
+ 
+ 		if (IsAbortedTransactionBlockState())
+ 			return;
+ 
+ 		DisableNotifyInterrupt();
+ 		DisableCatchupInterrupt();
+ 
+ 		ereport(NOTICE,
+ 				(errcode(ERRCODE_QUERY_CANCELED),
+ 				 errmsg("canceling idle transaction due to administrator request")));
+ 
+ 		AbortTransactionAndAnySubtransactions();
+ 
+ 		/*
+ 		 * Set the display correctly now, rather than wait for client wait loop
+ 		 * to cycle round and reset display at some later time.
+ 		 */
+ 		set_ps_display("idle in transaction (aborted)", false);
+ 		pgstat_report_activity("<IDLE> in transaction (aborted)");
+ 	}
  	/* If we get here, do nothing (probably, QueryCancelPending was reset) */
  }
  
***************
*** 3603,3609 **** PostgresMain(int argc, char *argv[], const char *username)
  		 */
  		if (send_ready_for_query)
  		{
! 			if (IsTransactionOrTransactionBlock())
  			{
  				set_ps_display("idle in transaction", false);
  				pgstat_report_activity("<IDLE> in transaction");
--- 3629,3640 ----
  		 */
  		if (send_ready_for_query)
  		{
! 			if (IsAbortedTransactionBlockState())
! 			{
! 				set_ps_display("idle in transaction (aborted)", false);
! 				pgstat_report_activity("<IDLE> in transaction (aborted)");
! 			}
! 			else if (IsTransactionOrTransactionBlock())
  			{
  				set_ps_display("idle in transaction", false);
  				pgstat_report_activity("<IDLE> in transaction");
*** a/src/backend/utils/init/globals.c
--- b/src/backend/utils/init/globals.c
***************
*** 27,32 **** ProtocolVersion FrontendProtocol;
--- 27,33 ----
  
  volatile bool InterruptPending = false;
  volatile bool QueryCancelPending = false;
+ volatile bool TransactionCancelPending = false;
  volatile bool ProcDiePending = false;
  volatile bool ImmediateInterruptOK = false;
  volatile uint32 InterruptHoldoffCount = 0;
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
***************
*** 204,209 **** extern bool IsTransactionBlock(void);
--- 204,210 ----
  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);
*** a/src/include/miscadmin.h
--- b/src/include/miscadmin.h
***************
*** 68,73 ****
--- 68,74 ----
  /* these are marked volatile because they are set by signal handlers: */
  extern PGDLLIMPORT volatile bool InterruptPending;
  extern volatile bool QueryCancelPending;
+ extern volatile bool TransactionCancelPending;
  extern volatile bool ProcDiePending;
  
  /* these are marked volatile because they are examined by signal handlers: */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to