On Thu, Dec 31, 2009 at 6:40 PM, Simon Riggs <si...@2ndquadrant.com> wrote:
>> Building racy infrastructure when it can be avoided with a little care
>> still seems not to be the best path to me.
>
> Doing that will add more complexity in an area that is hard to test
> effectively. I think the risk of introducing further bugs while trying
> to fix this rare condition is high. Right now the conflict processing
> needs more work and is often much less precise than this, so improving
> this aspect of it would not be a priority. I've added it to the TODO
> though. Thank you for your research.
>
> Patch implements recovery conflict signalling using SIGUSR1
> multiplexing, then uses a SessionCancelPending mode similar to Joachim's
> TransactionCancelPending.

I have reworked Simon's patch a bit and attach the result.

Quick facts:

- Hot Standby only uses SIGUSR1
- SIGINT behaves as it did before: it only cancels running statements
- pg_cancel_backend() continues to use SIGINT
- I added pg_cancel_idle_transaction() to cancel an idle transaction via
  SIGUSR1
- One central function HandleCancelAction() sets the flags before calling
  ProcessInterrupts(), it is called from the different signal handlers and
  receives parameters about what it should do
- If a SIGUSR1 reason is used that will cancel something, ProcArrayLock is
  acquired until the signal has been sent to make sure that we won't signal the
  wrong backend. Does this sufficiently cover the concerns of Andres Freund
  discussed upthread?

As there were so many boolean SomethingCancelPending variables I changed them
to be bitmasks and merged all of them into a single variable. However I am not
sure if we can change the name and type of especially InterruptPending that
easily as it was marked with PGDLLIMPORT...

@Simon: Is there a reason why you have not yet removed recoveryConflictMode
from PGPROC?


Joachim
diff -cr cvs/src/backend/access/transam/xact.c cvs.build/src/backend/access/transam/xact.c
*** cvs/src/backend/access/transam/xact.c	2009-12-24 13:55:12.000000000 +0100
--- cvs.build/src/backend/access/transam/xact.c	2010-01-05 11:25:17.000000000 +0100
***************
*** 313,320 ****
  /*
   *	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 ****
--- 2691,2738 ----
  }
  
  /*
+  *	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
diff -cr cvs/src/backend/commands/vacuum.c cvs.build/src/backend/commands/vacuum.c
*** cvs/src/backend/commands/vacuum.c	2009-12-24 13:55:13.000000000 +0100
--- cvs.build/src/backend/commands/vacuum.c	2010-01-03 20:24:26.000000000 +0100
***************
*** 3936,3942 ****
  	CHECK_FOR_INTERRUPTS();
  
  	/* Nap if appropriate */
! 	if (VacuumCostActive && !InterruptPending &&
  		VacuumCostBalance >= VacuumCostLimit)
  	{
  		int			msec;
--- 3936,3942 ----
  	CHECK_FOR_INTERRUPTS();
  
  	/* Nap if appropriate */
! 	if (VacuumCostActive && !IsInterruptEventPending &&
  		VacuumCostBalance >= VacuumCostLimit)
  	{
  		int			msec;
diff -cr cvs/src/backend/postmaster/autovacuum.c cvs.build/src/backend/postmaster/autovacuum.c
*** cvs/src/backend/postmaster/autovacuum.c	2009-12-09 11:24:41.000000000 +0100
--- cvs.build/src/backend/postmaster/autovacuum.c	2010-01-07 00:22:21.000000000 +0100
***************
*** 479,488 ****
  		/* Prevents interrupts while cleaning up */
  		HOLD_INTERRUPTS();
  
! 		/* Forget any pending QueryCancel request */
! 		QueryCancelPending = false;
  		disable_sig_alarm(true);
! 		QueryCancelPending = false;		/* again in case timeout occurred */
  
  		/* Report the error to the server log */
  		EmitErrorReport();
--- 479,489 ----
  		/* Prevents interrupts while cleaning up */
  		HOLD_INTERRUPTS();
  
! 		/* Forget any pending CancelQuery request */
! 		ClearInterruptEventPending(CancelQueryPending);
  		disable_sig_alarm(true);
! 		/* again in case timeout occurred */
! 		ClearInterruptEventPending(CancelQueryPending);
  
  		/* Report the error to the server log */
  		EmitErrorReport();
***************
*** 2233,2244 ****
  			autovacuum_do_vac_analyze(tab, bstrategy);
  
  			/*
! 			 * Clear a possible query-cancel signal, to avoid a late reaction
  			 * to an automatically-sent signal because of vacuuming the
  			 * current table (we're done with it, so it would make no sense to
  			 * cancel at this point.)
  			 */
! 			QueryCancelPending = false;
  		}
  		PG_CATCH();
  		{
--- 2234,2245 ----
  			autovacuum_do_vac_analyze(tab, bstrategy);
  
  			/*
! 			 * Clear a possible cancel-query signal, to avoid a late reaction
  			 * to an automatically-sent signal because of vacuuming the
  			 * current table (we're done with it, so it would make no sense to
  			 * cancel at this point.)
  			 */
! 			ClearInterruptEventPending(CancelQueryPending);
  		}
  		PG_CATCH();
  		{
diff -cr cvs/src/backend/storage/ipc/ipc.c cvs.build/src/backend/storage/ipc/ipc.c
*** cvs/src/backend/storage/ipc/ipc.c	2009-12-09 11:24:52.000000000 +0100
--- cvs.build/src/backend/storage/ipc/ipc.c	2010-01-03 20:33:49.000000000 +0100
***************
*** 155,163 ****
  	 * close up shop already.  Note that the signal handlers will not set
  	 * these flags again, now that proc_exit_inprogress is set.
  	 */
! 	InterruptPending = false;
! 	ProcDiePending = false;
! 	QueryCancelPending = false;
  	/* And let's just make *sure* we're not interrupted ... */
  	ImmediateInterruptOK = false;
  	InterruptHoldoffCount = 1;
--- 155,161 ----
  	 * close up shop already.  Note that the signal handlers will not set
  	 * these flags again, now that proc_exit_inprogress is set.
  	 */
! 	ZeroInterruptEventPending();
  	/* And let's just make *sure* we're not interrupted ... */
  	ImmediateInterruptOK = false;
  	InterruptHoldoffCount = 1;
diff -cr cvs/src/backend/storage/ipc/procarray.c cvs.build/src/backend/storage/ipc/procarray.c
*** cvs/src/backend/storage/ipc/procarray.c	2009-12-24 13:55:18.000000000 +0100
--- cvs.build/src/backend/storage/ipc/procarray.c	2010-01-06 20:02:31.000000000 +0100
***************
*** 52,57 ****
--- 52,58 ----
  #include "access/twophase.h"
  #include "miscadmin.h"
  #include "storage/procarray.h"
+ #include "storage/procsignal.h"
  #include "storage/standby.h"
  #include "utils/builtins.h"
  #include "utils/snapmgr.h"
***************
*** 1704,1710 ****
   * Returns pid of the process signaled, or 0 if not found.
   */
  pid_t
! CancelVirtualTransaction(VirtualTransactionId vxid, int cancel_mode)
  {
  	ProcArrayStruct *arrayP = procArray;
  	int			index;
--- 1705,1711 ----
   * Returns pid of the process signaled, or 0 if not found.
   */
  pid_t
! CancelVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode)
  {
  	ProcArrayStruct *arrayP = procArray;
  	int			index;
***************
*** 1722,1733 ****
  		if (procvxid.backendId == vxid.backendId &&
  			procvxid.localTransactionId == vxid.localTransactionId)
  		{
- 			/*
- 			 * Issue orders for the proc to read next time it receives SIGINT
- 			 */
- 			if (proc->recoveryConflictMode < cancel_mode)
- 				proc->recoveryConflictMode = cancel_mode;
- 
  			pid = proc->pid;
  			break;
  		}
--- 1723,1728 ----
***************
*** 1741,1747 ****
  		 * Kill the pid if it's still here. If not, that's what we wanted
  		 * so ignore any errors.
  		 */
! 		kill(pid, SIGINT);
  	}
  
  	return pid;
--- 1736,1742 ----
  		 * Kill the pid if it's still here. If not, that's what we wanted
  		 * so ignore any errors.
  		 */
! 		(void) SendProcSignal(pid, sigmode, vxid.backendId);
  	}
  
  	return pid;
diff -cr cvs/src/backend/storage/ipc/procsignal.c cvs.build/src/backend/storage/ipc/procsignal.c
*** cvs/src/backend/storage/ipc/procsignal.c	2009-12-09 11:24:52.000000000 +0100
--- cvs.build/src/backend/storage/ipc/procsignal.c	2010-01-07 12:00:33.000000000 +0100
***************
*** 24,29 ****
--- 24,31 ----
  #include "storage/procsignal.h"
  #include "storage/shmem.h"
  #include "storage/sinval.h"
+ #include "storage/standby.h"
+ #include "tcop/tcopprot.h"
  
  
  /*
***************
*** 170,195 ****
  SendProcSignal(pid_t pid, ProcSignalReason reason, BackendId backendId)
  {
  	volatile ProcSignalSlot *slot;
  
  	if (backendId != InvalidBackendId)
  	{
! 		slot = &ProcSignalSlots[backendId - 1];
! 
  		/*
! 		 * Note: Since there's no locking, it's possible that the target
  		 * process detaches from shared memory and exits right after this
! 		 * test, before we set the flag and send signal. And the signal slot
! 		 * might even be recycled by a new process, so it's remotely possible
! 		 * that we set a flag for a wrong process. That's OK, all the signals
! 		 * are such that no harm is done if they're mistakenly fired.
  		 */
  		if (slot->pss_pid == pid)
  		{
  			/* Atomically set the proper flag */
  			slot->pss_signalFlags[reason] = true;
  			/* Send signal */
! 			return kill(pid, SIGUSR1);
  		}
  	}
  	else
  	{
--- 172,210 ----
  SendProcSignal(pid_t pid, ProcSignalReason reason, BackendId backendId)
  {
  	volatile ProcSignalSlot *slot;
+ 	int						 ret;
  
  	if (backendId != InvalidBackendId)
  	{
! 		Assert(backendId <= MaxBackends);
  		/*
! 		 * If we did not get a lock, it would be possible that the target
  		 * process detaches from shared memory and exits right after this
! 		 * test, before we set the flag and send the signal. And the signal
! 		 * slot might even be recycled by a new process, so it's remotely
! 		 * possible that we set a flag for a wrong process. That's OK for
! 		 * harmless signal reasons. As soon as we get a cancel-something signal
! 		 * reason (which really should not happen all that often), we take the
! 		 * time to grab a shared lock on ProcArray before we start to search
! 		 * that backend and hold it until we have sent the signal.
  		 */
+ 		if (!IsHarmlessSignal(reason))
+ 			LWLockAcquire(ProcArrayLock, LW_SHARED);
+ 
+ 		slot = &ProcSignalSlots[backendId - 1];
+ 
  		if (slot->pss_pid == pid)
  		{
  			/* Atomically set the proper flag */
  			slot->pss_signalFlags[reason] = true;
  			/* Send signal */
! 			ret = kill(pid, SIGUSR1);
  		}
+ 
+ 		if (!IsHarmlessSignal(reason))
+ 			LWLockRelease(ProcArrayLock);
+ 
+ 		return ret;
  	}
  	else
  	{
***************
*** 200,205 ****
--- 215,228 ----
  		 * process, which will have a slot near the end of the array.
  		 */
  		int		i;
+ 		bool	found = false;
+ 
+ 		/*
+ 		 * If the signal is not a harmless one, grab a lock to be sure we
+ 		 * send it to the correct backend.
+ 		 */
+ 		if (!IsHarmlessSignal(reason))
+ 			LWLockAcquire(ProcArrayLock, LW_SHARED);
  
  		for (i = NumProcSignalSlots - 1; i >= 0; i--)
  		{
***************
*** 207,220 ****
  
  			if (slot->pss_pid == pid)
  			{
! 				/* the above note about race conditions applies here too */
  
  				/* Atomically set the proper flag */
  				slot->pss_signalFlags[reason] = true;
  				/* Send signal */
! 				return kill(pid, SIGUSR1);
  			}
  		}
  	}
  
  	errno = ESRCH;
--- 230,250 ----
  
  			if (slot->pss_pid == pid)
  			{
! 				found = true;
  
  				/* Atomically set the proper flag */
  				slot->pss_signalFlags[reason] = true;
  				/* Send signal */
! 				ret = kill(pid, SIGUSR1);
! 				break;
  			}
  		}
+ 
+ 		if (!IsHarmlessSignal(reason))
+ 			LWLockRelease(ProcArrayLock);
+ 
+ 		if (found)
+ 			return ret;
  	}
  
  	errno = ESRCH;
***************
*** 252,262 ****
--- 282,314 ----
  {
  	int		save_errno = errno;
  
+ 	/* signal reasons that notify a backend of an event */
  	if (CheckProcSignal(PROCSIG_CATCHUP_INTERRUPT))
  		HandleCatchupInterrupt();
  
  	if (CheckProcSignal(PROCSIG_NOTIFY_INTERRUPT))
  		HandleNotifyInterrupt();
  
+ 	/* signal reasons that request the backend to cancel some action */
+ 
+ 	/* pg_cancel_idle_transaction() */
+ 	if (CheckProcSignal(PROCSIG_CANCEL_IDLE_TRANSACTION))
+ 		HandleCancelAction(false, true, false);
+ 
+ 	/* Hot standby */
+ 	if (CheckProcSignal(PROCSIG_CONFLICT_ERROR_INTERRUPT))
+ 	{
+ 		SetInterruptEventPending(ConflictErrorPending);
+ 		HandleCancelAction(true, true, false);
+ 	}
+ 
+ 	/* Hot standby */
+ 	if (CheckProcSignal(PROCSIG_CONFLICT_FATAL_INTERRUPT))
+ 	{
+ 		SetInterruptEventPending(ConflictFatalPending);
+ 		HandleCancelAction(false, false, true);
+ 	}
+ 
  	errno = save_errno;
  }
+ 
diff -cr cvs/src/backend/storage/ipc/standby.c cvs.build/src/backend/storage/ipc/standby.c
*** cvs/src/backend/storage/ipc/standby.c	2009-12-19 02:32:35.000000000 +0100
--- cvs.build/src/backend/storage/ipc/standby.c	2010-01-06 19:35:02.000000000 +0100
***************
*** 219,229 ****
  			{
  				pid_t pid;
  
  				/*
  				 * Now find out who to throw out of the balloon.
  				 */
  				Assert(VirtualTransactionIdIsValid(*waitlist));
! 				pid = CancelVirtualTransaction(*waitlist, cancel_mode);
  
  				if (pid != 0)
  				{
--- 219,236 ----
  			{
  				pid_t pid;
  
+ 				ProcSignalReason	sigmode;
+ 
+ 				if (cancel_mode == CONFLICT_MODE_ERROR)
+ 					sigmode = PROCSIG_CONFLICT_ERROR_INTERRUPT;
+ 				else
+ 					sigmode = PROCSIG_CONFLICT_FATAL_INTERRUPT;
+ 
  				/*
  				 * Now find out who to throw out of the balloon.
  				 */
  				Assert(VirtualTransactionIdIsValid(*waitlist));
! 				pid = CancelVirtualTransaction(*waitlist, sigmode);
  
  				if (pid != 0)
  				{
diff -cr cvs/src/backend/tcop/postgres.c cvs.build/src/backend/tcop/postgres.c
*** cvs/src/backend/tcop/postgres.c	2009-12-24 13:55:18.000000000 +0100
--- cvs.build/src/backend/tcop/postgres.c	2010-01-07 12:46:26.000000000 +0100
***************
*** 479,486 ****
  		/* Allow "die" interrupt to be processed while waiting */
  		ImmediateInterruptOK = true;
  
! 		/* And don't forget to detect one that already arrived */
! 		QueryCancelPending = false;
  		CHECK_FOR_INTERRUPTS();
  	}
  }
--- 479,486 ----
  		/* Allow "die" interrupt to be processed while waiting */
  		ImmediateInterruptOK = true;
  
! 		/* And don't forget to detect those that already arrived */
! 		ZeroInterruptEventPending();
  		CHECK_FOR_INTERRUPTS();
  	}
  }
***************
*** 494,500 ****
  	if (DoingCommandRead)
  	{
  		ImmediateInterruptOK = false;
! 		QueryCancelPending = false;		/* forget any CANCEL signal */
  
  		DisableNotifyInterrupt();
  		DisableCatchupInterrupt();
--- 494,501 ----
  	if (DoingCommandRead)
  	{
  		ImmediateInterruptOK = false;
! 		/* forget any CANCEL signal */
! 		ZeroInterruptEventPending();
  
  		DisableNotifyInterrupt();
  		DisableCatchupInterrupt();
***************
*** 2593,2661 ****
  void
  die(SIGNAL_ARGS)
  {
! 	int			save_errno = errno;
! 
! 	/* Don't joggle the elbow of proc_exit */
! 	if (!proc_exit_inprogress)
! 	{
! 		InterruptPending = true;
! 		ProcDiePending = true;
! 
! 		/*
! 		 * If it's safe to interrupt, and we're waiting for input or a lock,
! 		 * service the interrupt immediately
! 		 */
! 		if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
! 			CritSectionCount == 0)
! 		{
! 			/* bump holdoff count to make ProcessInterrupts() a no-op */
! 			/* until we are done getting ready for it */
! 			InterruptHoldoffCount++;
! 			LockWaitCancel();	/* prevent CheckDeadLock from running */
! 			DisableNotifyInterrupt();
! 			DisableCatchupInterrupt();
! 			InterruptHoldoffCount--;
! 			ProcessInterrupts();
! 		}
! 	}
! 
! 	errno = save_errno;
  }
  
  /*
!  * Query-cancel signal from postmaster: abort current transaction
!  * at soonest convenient time
   */
  void
  StatementCancelHandler(SIGNAL_ARGS)
  {
  	int			save_errno = errno;
  
  	/*
! 	 * Don't joggle the elbow of proc_exit
  	 */
! 	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 */
! 			InterruptHoldoffCount++;
! 			LockWaitCancel();	/* prevent CheckDeadLock from running */
! 			DisableNotifyInterrupt();
! 			DisableCatchupInterrupt();
! 			InterruptHoldoffCount--;
! 			ProcessInterrupts();
! 		}
  	}
  
  	errno = save_errno;
--- 2594,2668 ----
  void
  die(SIGNAL_ARGS)
  {
! 	HandleCancelAction(false, false, true);
  }
  
  /*
!  * Query-cancel signal from postmaster: abort current query at soonest
!  * convenient time, do not abort an idle transaction however.
   */
  void
  StatementCancelHandler(SIGNAL_ARGS)
  {
+ 	HandleCancelAction(true, false, false);
+ }
+ 
+ void
+ HandleCancelAction(bool cancelQuery,
+ 				   bool cancelIdleTransaction,
+ 				   bool cancelSession)
+ {
  	int			save_errno = errno;
  
+ 	/* Don't joggle the elbow of proc_exit */
+ 	if (proc_exit_inprogress)
+ 		return;
+ 
  	/*
! 	 * Check cancel requests against internal states, we might want to
! 	 * ignore a request.
  	 */
! 	if (cancelQuery && DoingCommandRead)
! 		cancelQuery = false;
! 	if (cancelIdleTransaction)
! 	{
! 		if (!DoingCommandRead)
! 			cancelIdleTransaction = false;
! 		else if (!IsTransactionOrTransactionBlock())
! 			cancelIdleTransaction = false;
! 		else if (IsAbortedTransactionBlockState())
! 			cancelIdleTransaction = false;
! 	}
  
! 	if (!cancelQuery && !cancelIdleTransaction && !cancelSession)
! 		return;
! 
! 	/*
! 	 * Set the global variable InterruptPendingEvent to whatever is left set
! 	 * to true after the previous checks.
! 	 */
! 	if (cancelQuery)
! 		SetInterruptEventPending(CancelQueryPending);
! 	if (cancelIdleTransaction)
! 		SetInterruptEventPending(CancelIdleTransactionPending);
! 	if (cancelSession)
! 		SetInterruptEventPending(CancelSessionPending);
! 
! 	/*
! 	 * If it's safe to interrupt, and we're waiting for a lock, service
! 	 * the interrupt immediately.
! 	 */
! 	if (InterruptHoldoffCount == 0 && CritSectionCount == 0 &&
! 		ImmediateInterruptOK)
! 	{
! 		/* bump holdoff count to make ProcessInterrupts() a no-op */
! 		/* until we are done getting ready for it */
! 		InterruptHoldoffCount++;
! 		LockWaitCancel();	/* prevent CheckDeadLock from running */
! 		DisableNotifyInterrupt();
! 		DisableCatchupInterrupt();
! 		InterruptHoldoffCount--;
! 		ProcessInterrupts();
  	}
  
  	errno = save_errno;
***************
*** 2685,2704 ****
   * ProcessInterrupts: out-of-line portion of CHECK_FOR_INTERRUPTS() macro
   *
   * If an interrupt condition is pending, and it's safe to service it,
!  * then clear the flag and accept the interrupt.  Called only when
!  * InterruptPending is true.
   */
  void
  ProcessInterrupts(void)
  {
  	/* OK to accept interrupt now? */
  	if (InterruptHoldoffCount != 0 || CritSectionCount != 0)
  		return;
! 	InterruptPending = false;
! 	if (ProcDiePending)
  	{
- 		ProcDiePending = false;
- 		QueryCancelPending = false;		/* ProcDie trumps QueryCancel */
  		ImmediateInterruptOK = false;	/* not idle anymore */
  		DisableNotifyInterrupt();
  		DisableCatchupInterrupt();
--- 2692,2712 ----
   * ProcessInterrupts: out-of-line portion of CHECK_FOR_INTERRUPTS() macro
   *
   * If an interrupt condition is pending, and it's safe to service it,
!  * then clear the flag and accept the interrupt. Called only when
!  * InterruptPendingEvent != NothingPending.
   */
  void
  ProcessInterrupts(void)
  {
+ 	const char *msg;
+ 	int			code;
+ 
  	/* OK to accept interrupt now? */
  	if (InterruptHoldoffCount != 0 || CritSectionCount != 0)
  		return;
! 
! 	if (InterruptPendingEvent & CancelSessionPending)
  	{
  		ImmediateInterruptOK = false;	/* not idle anymore */
  		DisableNotifyInterrupt();
  		DisableCatchupInterrupt();
***************
*** 2706,2722 ****
  		if (ClientAuthInProgress && whereToSendOutput == DestRemote)
  			whereToSendOutput = DestNone;
  		if (IsAutoVacuumWorkerProcess())
! 			ereport(FATAL,
! 					(errcode(ERRCODE_ADMIN_SHUTDOWN),
! 					 errmsg("terminating autovacuum process due to administrator command")));
  		else
! 			ereport(FATAL,
! 					(errcode(ERRCODE_ADMIN_SHUTDOWN),
! 			 errmsg("terminating connection due to administrator command")));
  	}
! 	if (QueryCancelPending)
  	{
- 		QueryCancelPending = false;
  		ImmediateInterruptOK = false;	/* not idle anymore */
  		DisableNotifyInterrupt();
  		DisableCatchupInterrupt();
--- 2714,2742 ----
  		if (ClientAuthInProgress && whereToSendOutput == DestRemote)
  			whereToSendOutput = DestNone;
  		if (IsAutoVacuumWorkerProcess())
! 		{
! 			code = ERRCODE_ADMIN_SHUTDOWN;
! 			msg = "terminating autovacuum process due to administrator command";
! 		}
! 		else if (InterruptPendingEvent & ConflictFatalPending)
! 		{
! 			Assert(RecoveryInProgress());
! 			code = ERRCODE_QUERY_CANCELED;
! 			msg = "canceling session due to conflict with recovery";
! 		}
  		else
! 		{
! 			code = ERRCODE_ADMIN_SHUTDOWN;
! 			msg = "terminating connection due to administrator command";
! 		}
! 
! 		ZeroInterruptEventPending();
! 
! 		ereport(FATAL, (errcode(code),
! 						errmsg(msg)));
  	}
! 	if (InterruptPendingEvent & CancelQueryPending)
  	{
  		ImmediateInterruptOK = false;	/* not idle anymore */
  		DisableNotifyInterrupt();
  		DisableCatchupInterrupt();
***************
*** 2724,2795 ****
  		if (ClientAuthInProgress && whereToSendOutput == DestRemote)
  			whereToSendOutput = DestNone;
  		if (ClientAuthInProgress)
! 			ereport(ERROR,
! 					(errcode(ERRCODE_QUERY_CANCELED),
! 					 errmsg("canceling authentication due to timeout")));
  		else if (cancel_from_timeout)
! 			ereport(ERROR,
! 					(errcode(ERRCODE_QUERY_CANCELED),
! 					 errmsg("canceling statement due to statement timeout")));
  		else if (IsAutoVacuumWorkerProcess())
! 			ereport(ERROR,
! 					(errcode(ERRCODE_QUERY_CANCELED),
! 					 errmsg("canceling autovacuum task")));
! 		else
  		{
! 			int cancelMode = MyProc->recoveryConflictMode;
  
! 			/*
! 			 * XXXHS: We don't yet have a clean way to cancel an
! 			 * idle-in-transaction session, so make it FATAL instead.
! 			 * This isn't as bad as it looks because we don't issue a
! 			 * CONFLICT_MODE_ERROR for a session with proc->xmin == 0
! 			 * on cleanup conflicts. There's a possibility that we
! 			 * marked somebody as a conflict and then they go idle.
! 			 */
! 			if (DoingCommandRead && IsTransactionBlock() &&
! 				cancelMode == CONFLICT_MODE_ERROR)
! 			{
! 				cancelMode = CONFLICT_MODE_FATAL;
! 			}
  
! 			switch (cancelMode)
! 			{
! 				case CONFLICT_MODE_FATAL:
! 						Assert(RecoveryInProgress());
! 						ereport(FATAL,
! 							(errcode(ERRCODE_QUERY_CANCELED),
! 							 errmsg("canceling session due to conflict with recovery")));
! 
! 				case CONFLICT_MODE_ERROR:
! 						/*
! 						 * We are aborting because we need to release
! 						 * locks. So we need to abort out of all
! 						 * subtransactions to make sure we release
! 						 * all locks at whatever their level.
! 						 *
! 						 * XXX Should we try to examine the
! 						 * transaction tree and cancel just enough
! 						 * subxacts to remove locks? Doubt it.
! 						 */
! 						Assert(RecoveryInProgress());
! 						AbortOutOfAnyTransaction();
! 						ereport(ERROR,
! 							(errcode(ERRCODE_QUERY_CANCELED),
! 							 errmsg("canceling statement due to conflict with recovery")));
! 						return;
! 
! 				default:
! 						/* No conflict pending, so fall through */
! 						break;
! 			}
  
! 			ereport(ERROR,
! 					(errcode(ERRCODE_QUERY_CANCELED),
! 					 errmsg("canceling statement due to user request")));
  		}
  	}
! 	/* If we get here, do nothing (probably, QueryCancelPending was reset) */
  }
  
  
--- 2744,2794 ----
  		if (ClientAuthInProgress && whereToSendOutput == DestRemote)
  			whereToSendOutput = DestNone;
  		if (ClientAuthInProgress)
! 			msg = "canceling authentication due to timeout";
  		else if (cancel_from_timeout)
! 			msg = "canceling statement due to statement timeout";
  		else if (IsAutoVacuumWorkerProcess())
! 			msg = "canceling autovacuum task";
! 		else if (InterruptPendingEvent & ConflictErrorPending)
  		{
! 			Assert(RecoveryInProgress());
! 			msg = "canceling statement due to conflict with recovery";
! 		}
! 		else
! 			msg = "canceling statement due to user request";
  
! 		ZeroInterruptEventPending();
  
! 		ereport(ERROR,
! 				(errcode(ERRCODE_QUERY_CANCELED),
! 				 errmsg(msg)));
! 	}
! 	if (InterruptPendingEvent & CancelIdleTransactionPending)
! 	{
! 		ImmediateInterruptOK = false;	/* not idle anymore */
! 		DisableNotifyInterrupt();
! 		DisableCatchupInterrupt();
  
! 		if (InterruptPendingEvent & ConflictErrorPending)
! 		{
! 			Assert(RecoveryInProgress());
! 			msg = "canceling statement due to conflict with recovery";
  		}
+ 		else
+ 			msg = "canceling idle transaction due to administrator request";
+ 
+ 		ZeroInterruptEventPending();
+ 
+ 		ereport(NOTICE,
+ 				(errcode(ERRCODE_QUERY_CANCELED),
+ 				 errmsg(msg)));
+ 
+ 		AbortTransactionAndAnySubtransactions();
+ 
+ 		set_ps_display("idle in transaction (aborted)", false);
+ 		pgstat_report_activity("<IDLE> in transaction (aborted)");
  	}
! 	/* If we get here, do nothing (probably, CancelQueryPending was reset) */
  }
  
  
***************
*** 3507,3518 ****
  		HOLD_INTERRUPTS();
  
  		/*
! 		 * Forget any pending QueryCancel request, since we're returning to
  		 * the idle loop anyway, and cancel the statement timer if running.
  		 */
! 		QueryCancelPending = false;
  		disable_sig_alarm(true);
! 		QueryCancelPending = false;		/* again in case timeout occurred */
  
  		/*
  		 * Turn off these interrupts too.  This is only needed here and not in
--- 3506,3518 ----
  		HOLD_INTERRUPTS();
  
  		/*
! 		 * Forget any pending CancelQuery request, since we're returning to
  		 * the idle loop anyway, and cancel the statement timer if running.
  		 */
! 		ZeroInterruptEventPending();
  		disable_sig_alarm(true);
! 		/* again in case timeout occurred */
! 		ZeroInterruptEventPending();
  
  		/*
  		 * Turn off these interrupts too.  This is only needed here and not in
***************
*** 3603,3609 ****
  		 */
  		if (send_ready_for_query)
  		{
! 			if (IsTransactionOrTransactionBlock())
  			{
  				set_ps_display("idle in transaction", false);
  				pgstat_report_activity("<IDLE> in transaction");
--- 3603,3614 ----
  		 */
  		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");
***************
*** 3626,3632 ****
  		 * conditional since we don't want, say, reads on behalf of COPY FROM
  		 * STDIN doing the same thing.)
  		 */
! 		QueryCancelPending = false;		/* forget any earlier CANCEL signal */
  		DoingCommandRead = true;
  
  		/*
--- 3631,3638 ----
  		 * conditional since we don't want, say, reads on behalf of COPY FROM
  		 * STDIN doing the same thing.)
  		 */
! 		/* forget any earlier cancel signals */
! 		ZeroInterruptEventPending();
  		DoingCommandRead = true;
  
  		/*
diff -cr cvs/src/backend/utils/adt/misc.c cvs.build/src/backend/utils/adt/misc.c
*** cvs/src/backend/utils/adt/misc.c	2009-12-09 11:24:45.000000000 +0100
--- cvs.build/src/backend/utils/adt/misc.c	2010-01-07 12:38:52.000000000 +0100
***************
*** 30,35 ****
--- 30,36 ----
  #include "storage/fd.h"
  #include "storage/pmsignal.h"
  #include "storage/procarray.h"
+ #include "storage/procsignal.h"
  #include "utils/builtins.h"
  #include "tcop/tcopprot.h"
  
***************
*** 117,122 ****
--- 118,132 ----
  }
  
  Datum
+ pg_cancel_idle_transaction(PG_FUNCTION_ARGS)
+ {
+ 	int     ret;
+ 	pid_t   pid = PG_GETARG_INT32(0);
+ 	ret = SendProcSignal(pid, PROCSIG_CANCEL_IDLE_TRANSACTION, InvalidBackendId);
+ 	PG_RETURN_BOOL(ret == 0);
+ }
+ 
+ Datum
  pg_reload_conf(PG_FUNCTION_ARGS)
  {
  	if (!superuser())
diff -cr cvs/src/backend/utils/init/globals.c cvs.build/src/backend/utils/init/globals.c
*** cvs/src/backend/utils/init/globals.c	2009-12-09 11:24:42.000000000 +0100
--- cvs.build/src/backend/utils/init/globals.c	2010-01-03 20:58:09.000000000 +0100
***************
*** 25,33 ****
  
  ProtocolVersion FrontendProtocol;
  
! volatile bool InterruptPending = false;
! volatile bool QueryCancelPending = false;
! volatile bool ProcDiePending = false;
  volatile bool ImmediateInterruptOK = false;
  volatile uint32 InterruptHoldoffCount = 0;
  volatile uint32 CritSectionCount = 0;
--- 25,31 ----
  
  ProtocolVersion FrontendProtocol;
  
! volatile int InterruptPendingEvent = NothingPending;
  volatile bool ImmediateInterruptOK = false;
  volatile uint32 InterruptHoldoffCount = 0;
  volatile uint32 CritSectionCount = 0;
diff -cr cvs/src/include/access/xact.h cvs.build/src/include/access/xact.h
*** cvs/src/include/access/xact.h	2009-12-24 13:55:28.000000000 +0100
--- cvs.build/src/include/access/xact.h	2010-01-04 08:01:52.000000000 +0100
***************
*** 189,194 ****
--- 189,195 ----
  extern void StartTransactionCommand(void);
  extern void CommitTransactionCommand(void);
  extern void AbortCurrentTransaction(void);
+ extern void AbortTransactionAndAnySubtransactions(void);
  extern void BeginTransactionBlock(void);
  extern bool EndTransactionBlock(void);
  extern bool PrepareTransactionBlock(char *gid);
diff -cr cvs/src/include/catalog/pg_proc.h cvs.build/src/include/catalog/pg_proc.h
*** cvs/src/include/catalog/pg_proc.h	2009-12-24 13:55:29.000000000 +0100
--- cvs.build/src/include/catalog/pg_proc.h	2010-01-04 07:45:01.000000000 +0100
***************
*** 3268,3273 ****
--- 3268,3275 ----
  
  DATA(insert OID = 2171 ( pg_cancel_backend		PGNSP PGUID 12 1 0 0 f f f t f v 1 0 16 "23" _null_ _null_ _null_ _null_ pg_cancel_backend _null_ _null_ _null_ ));
  DESCR("cancel a server process' current query");
+ DATA(insert OID = 2179 ( pg_cancel_idle_transaction		PGNSP PGUID 12 1 0 0 f f f t f v 1 0 16 "23" _null_ _null_ _null_ _null_ pg_cancel_idle_transaction _null_ _null_ _null_ ));
+ DESCR("cancel a server process' idle transaction");
  DATA(insert OID = 2096 ( pg_terminate_backend		PGNSP PGUID 12 1 0 0 f f f t f v 1 0 16 "23" _null_ _null_ _null_ _null_ pg_terminate_backend _null_ _null_ _null_ ));
  DESCR("terminate a server process");
  DATA(insert OID = 2172 ( pg_start_backup		PGNSP PGUID 12 1 0 0 f f f t f v 2 0 25 "25 16" _null_ _null_ _null_ _null_ pg_start_backup _null_ _null_ _null_ ));
diff -cr cvs/src/include/miscadmin.h cvs.build/src/include/miscadmin.h
*** cvs/src/include/miscadmin.h	2009-12-24 13:55:28.000000000 +0100
--- cvs.build/src/include/miscadmin.h	2010-01-06 21:13:35.000000000 +0100
***************
*** 64,74 ****
   *
   *****************************************************************************/
  
  /* in globals.c */
! /* these are marked volatile because they are set by signal handlers: */
! extern PGDLLIMPORT volatile bool InterruptPending;
! extern volatile bool QueryCancelPending;
! extern volatile bool ProcDiePending;
  
  /* these are marked volatile because they are examined by signal handlers: */
  extern volatile bool ImmediateInterruptOK;
--- 64,85 ----
   *
   *****************************************************************************/
  
+ #define NothingPending						0
+ #define CancelQueryPending					1
+ #define CancelIdleTransactionPending		2
+ #define CancelQueryOrTransactionPending		4
+ #define CancelSessionPending				8
+ #define ConflictErrorPending				16
+ #define ConflictFatalPending				32
+ 
+ #define IsInterruptEventPending		   (InterruptPendingEvent != NothingPending)
+ #define SetInterruptEventPending(ev)   (InterruptPendingEvent |= ev)
+ #define ClearInterruptEventPending(ev) (InterruptPendingEvent &= ~ev)
+ #define ZeroInterruptEventPending()	   (InterruptPendingEvent = NothingPending)
+ 
  /* in globals.c */
! /* this is marked volatile because it is set by signal handlers: */
! extern PGDLLIMPORT volatile int InterruptPendingEvent;
  
  /* these are marked volatile because they are examined by signal handlers: */
  extern volatile bool ImmediateInterruptOK;
***************
*** 82,88 ****
  
  #define CHECK_FOR_INTERRUPTS() \
  do { \
! 	if (InterruptPending) \
  		ProcessInterrupts(); \
  } while(0)
  #else							/* WIN32 */
--- 93,99 ----
  
  #define CHECK_FOR_INTERRUPTS() \
  do { \
! 	if (IsInterruptEventPending) \
  		ProcessInterrupts(); \
  } while(0)
  #else							/* WIN32 */
***************
*** 91,97 ****
  do { \
  	if (UNBLOCKED_SIGNAL_QUEUE()) \
  		pgwin32_dispatch_queued_signals(); \
! 	if (InterruptPending) \
  		ProcessInterrupts(); \
  } while(0)
  #endif   /* WIN32 */
--- 102,108 ----
  do { \
  	if (UNBLOCKED_SIGNAL_QUEUE()) \
  		pgwin32_dispatch_queued_signals(); \
! 	if (IsInterruptEventPending) \
  		ProcessInterrupts(); \
  } while(0)
  #endif   /* WIN32 */
diff -cr cvs/src/include/storage/procarray.h cvs.build/src/include/storage/procarray.h
*** cvs/src/include/storage/procarray.h	2009-12-24 13:55:33.000000000 +0100
--- cvs.build/src/include/storage/procarray.h	2010-01-06 19:36:15.000000000 +0100
***************
*** 15,20 ****
--- 15,21 ----
  #define PROCARRAY_H
  
  #include "storage/lock.h"
+ #include "storage/procsignal.h"
  #include "storage/standby.h"
  #include "utils/snapshot.h"
  
***************
*** 59,65 ****
  extern VirtualTransactionId *GetConflictingVirtualXIDs(TransactionId limitXmin,
  					Oid dbOid, bool skipExistingConflicts);
  extern pid_t CancelVirtualTransaction(VirtualTransactionId vxid,
! 						 int cancel_mode);
  
  extern int	CountActiveBackends(void);
  extern int	CountDBBackends(Oid databaseid);
--- 60,66 ----
  extern VirtualTransactionId *GetConflictingVirtualXIDs(TransactionId limitXmin,
  					Oid dbOid, bool skipExistingConflicts);
  extern pid_t CancelVirtualTransaction(VirtualTransactionId vxid,
! 									  ProcSignalReason sigmode);
  
  extern int	CountActiveBackends(void);
  extern int	CountDBBackends(Oid databaseid);
diff -cr cvs/src/include/storage/procsignal.h cvs.build/src/include/storage/procsignal.h
*** cvs/src/include/storage/procsignal.h	2009-12-09 11:24:53.000000000 +0100
--- cvs.build/src/include/storage/procsignal.h	2010-01-07 00:34:26.000000000 +0100
***************
*** 32,40 ****
--- 32,51 ----
  	PROCSIG_CATCHUP_INTERRUPT,	/* sinval catchup interrupt */
  	PROCSIG_NOTIFY_INTERRUPT,	/* listen/notify interrupt */
  
+ 	IDX_PROCSIG_HARMLESS,		/* not a real identifier but an index.
+ 								 * we treat harmless signals different than
+ 								 * harmful ones. */
+ 
+ 	PROCSIG_CANCEL_IDLE_TRANSACTION, /* pg_backend_cancel_idle_transaction() */
+ 
+ 	PROCSIG_CONFLICT_ERROR_INTERRUPT, /* used by HotStandby for conflict */
+ 	PROCSIG_CONFLICT_FATAL_INTERRUPT, /* resolution.					 */
+ 
  	NUM_PROCSIGNALS				/* Must be last! */
  } ProcSignalReason;
  
+ #define IsHarmlessSignal(reason) (reason < IDX_PROCSIG_HARMLESS)
+ 
  /*
   * prototypes for functions in procsignal.c
   */
diff -cr cvs/src/include/tcop/tcopprot.h cvs.build/src/include/tcop/tcopprot.h
*** cvs/src/include/tcop/tcopprot.h	2009-12-09 11:24:53.000000000 +0100
--- cvs.build/src/include/tcop/tcopprot.h	2010-01-07 00:22:38.000000000 +0100
***************
*** 64,69 ****
--- 64,74 ----
  extern void quickdie(SIGNAL_ARGS);
  extern void StatementCancelHandler(SIGNAL_ARGS);
  extern void FloatExceptionHandler(SIGNAL_ARGS);
+ 
+ extern void HandleCancelAction(bool cancelQuery,
+ 							   bool cancelIdleTransaction,
+ 							   bool cancelSession);
+ 
  extern void prepare_for_client_read(void);
  extern void client_read_ended(void);
  extern const char *process_postgres_switches(int argc, char *argv[],
diff -cr cvs/src/include/utils/builtins.h cvs.build/src/include/utils/builtins.h
*** cvs/src/include/utils/builtins.h	2009-12-24 13:55:33.000000000 +0100
--- cvs.build/src/include/utils/builtins.h	2010-01-06 21:52:12.000000000 +0100
***************
*** 443,448 ****
--- 443,449 ----
  extern Datum current_database(PG_FUNCTION_ARGS);
  extern Datum current_query(PG_FUNCTION_ARGS);
  extern Datum pg_cancel_backend(PG_FUNCTION_ARGS);
+ extern Datum pg_cancel_idle_transaction(PG_FUNCTION_ARGS);
  extern Datum pg_terminate_backend(PG_FUNCTION_ARGS);
  extern Datum pg_reload_conf(PG_FUNCTION_ARGS);
  extern Datum pg_tablespace_databases(PG_FUNCTION_ARGS);
-- 
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