In HS, Startup process sets SIGALRM when waiting for buffer pin. If

woken by alarm we send SIGUSR1 to all backends requesting that they
check to see if they are blocking Startup process. If so, they throw
ERROR/FATAL as for other conflict resolutions. Deadlock stop gap
removed. max_standby_delay = -1 option removed to prevent deadlock.

Author: simonat2ndQuadrant

doc/src/sgml/backup.sgml

@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/backup.sgml,v 2.136 2010/01/15 09:18:56 heikki Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/backup.sgml,v 2.137 2010/01/23 16:37:12 sriggs Exp $ -->
 
 <chapter id="backup">
  <title>Backup and Restore</title>
@@ -2399,7 +2399,7 @@ primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass'
        </listitem>
        <listitem>
         <para>
-         Waiting to acquire buffer cleanup locks (for which there is no time out)
+         Waiting to acquire buffer cleanup locks
         </para>
        </listitem>
        <listitem>
@@ -2536,11 +2536,7 @@ primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass'
     Three-way deadlocks are possible between AccessExclusiveLocks arriving from
     the primary, cleanup WAL records that require buffer cleanup locks and
     user requests that are waiting behind replayed AccessExclusiveLocks. Deadlocks
-    are currently resolved by the cancellation of user processes that would
-    need to wait on a lock. This is heavy-handed and generates more query
-    cancellations than we need to, though does remove the possibility of deadlock.
-    This behaviour is expected to improve substantially for the main release
-    version of 8.5.
+    are resolved by time-out when we exceed <varname>max_standby_delay</>.
    </para>
 
    <para>
@@ -2630,11 +2626,7 @@ LOG:  database system is ready to accept read only connections
     <varname>max_standby_delay</> or even set it to zero, though that is a
     very aggressive setting. If the standby server is tasked as an additional
     server for decision support queries then it may be acceptable to set this
-    to a value of many hours (in seconds).  It is also possible to set
-    <varname>max_standby_delay</> to -1 which means wait forever for queries
-    to complete, if there are conflicts; this will be useful when performing
-    an archive recovery from a backup.
-   </para>
+    to a value of many hours (in seconds).
 
    <para>
     Transaction status "hint bits" written on primary are not WAL-logged,

doc/src/sgml/config.sgml

@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.244 2010/01/15 09:18:58 heikki Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.245 2010/01/23 16:37:12 sriggs Exp $ -->
 
 <chapter Id="runtime-config">
   <title>Server Configuration</title>
@@ -1825,14 +1825,15 @@ archive_command = 'copy "%p" "C:\\server\\archivedir\\%f"'  # Windows
       <listitem>
        <para>
         When server acts as a standby, this parameter specifies a wait policy
-        for queries that conflict with incoming data changes. Valid settings
-        are -1, meaning wait forever, or a wait time of 0 or more seconds.
-        If a conflict should occur the server will delay up to this
-        amount before it begins trying to resolve things less amicably, as
+        for queries that conflict with data changes being replayed by recovery.
+        If a conflict should occur the server will delay up to this number
+        of seconds before it begins trying to resolve things less amicably, as
         described in <xref linkend="hot-standby-conflict">. Typically,
         this parameter makes sense only during replication, so when
-        performing an archive recovery to recover from data loss a
-        parameter setting of 0 is recommended.  The default is 30 seconds.
+        performing an archive recovery to recover from data loss a very high
+        parameter setting is recommended.  The default is 30 seconds.
+        There is no wait-forever setting because of the potential for deadlock
+        which that setting would introduce.
         This parameter can only be set in the <filename>postgresql.conf</>
         file or on the server command line.
        </para>

src/backend/access/transam/xlog.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.359 2010/01/20 19:43:40 heikki Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.360 2010/01/23 16:37:12 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -8803,9 +8803,12 @@ StartupProcessMain(void)
 	 */
 	pqsignal(SIGHUP, StartupProcSigHupHandler); /* reload config file */
 	pqsignal(SIGINT, SIG_IGN);	/* ignore query cancel */
-	pqsignal(SIGTERM, StartupProcShutdownHandler);		/* request shutdown */
-	pqsignal(SIGQUIT, startupproc_quickdie);	/* hard crash time */
-	pqsignal(SIGALRM, SIG_IGN);
+	pqsignal(SIGTERM, StartupProcShutdownHandler);	/* request shutdown */
+	pqsignal(SIGQUIT, startupproc_quickdie);		/* hard crash time */
+	if (XLogRequestRecoveryConnections)
+		pqsignal(SIGALRM, handle_standby_sig_alarm); /* ignored unless InHotStandby */
+	else
+		pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, SIG_IGN);
 	pqsignal(SIGUSR2, SIG_IGN);

src/backend/storage/buffer/bufmgr.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.254 2010/01/02 16:57:51 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.255 2010/01/23 16:37:12 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -44,6 +44,7 @@
 #include "storage/ipc.h"
 #include "storage/proc.h"
 #include "storage/smgr.h"
+#include "storage/standby.h"
 #include "utils/rel.h"
 #include "utils/resowner.h"
 
@@ -2417,13 +2418,48 @@ LockBufferForCleanup(Buffer buffer)
 		PinCountWaitBuf = bufHdr;
 		UnlockBufHdr(bufHdr);
 		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+
 		/* Wait to be signaled by UnpinBuffer() */
-		ProcWaitForSignal();
+		if (InHotStandby)
+		{
+			/* Share the bufid that Startup process waits on */
+			SetStartupBufferPinWaitBufId(buffer - 1);
+			/* Set alarm and then wait to be signaled by UnpinBuffer() */
+			ResolveRecoveryConflictWithBufferPin();
+			SetStartupBufferPinWaitBufId(-1);
+		}
+		else
+			ProcWaitForSignal();
+
 		PinCountWaitBuf = NULL;
 		/* Loop back and try again */
 	}
 }
 
+/*
+ * Check called from RecoveryConflictInterrupt handler when Startup
+ * process requests cancelation of all pin holders that are blocking it.
+ */
+bool
+HoldingBufferPinThatDelaysRecovery(void)
+{
+	int		bufid = GetStartupBufferPinWaitBufId();
+
+	/*
+	 * If we get woken slowly then it's possible that the Startup process
+	 * was already woken by other backends before we got here. Also possible
+	 * that we get here by multiple interrupts or interrupts at inappropriate
+	 * times, so make sure we do nothing if the bufid is not set.
+	 */
+	if (bufid < 0)
+		return false;
+
+	if (PrivateRefCount[bufid] > 0)
+		return true;
+
+	return false;
+}
+
 /*
  * ConditionalLockBufferForCleanup - as above, but don't wait to get the lock
  *

src/backend/storage/ipc/procarray.c

@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.58 2010/01/21 00:53:58 sriggs Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.59 2010/01/23 16:37:12 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1680,15 +1680,13 @@ GetCurrentVirtualXIDs(TransactionId limitXmin, bool excludeXmin0,
  * latestCompletedXid since doing so would be a performance issue during
  * normal running, so we check it essentially for free on the standby.
  *
- * If dbOid is valid we skip backends attached to other databases. Some
- * callers choose to skipExistingConflicts.
+ * If dbOid is valid we skip backends attached to other databases.
  *
  * Be careful to *not* pfree the result from this function. We reuse
  * this array sufficiently often that we use malloc for the result.
  */
 VirtualTransactionId *
-GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid,
-						  bool skipExistingConflicts)
+GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid)
 {
 	static VirtualTransactionId *vxids;
 	ProcArrayStruct *arrayP = procArray;
@@ -1727,9 +1725,6 @@ GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid,
 		if (proc->pid == 0)
 			continue;
 
-		if (skipExistingConflicts && proc->recoveryConflictPending)
-			continue;
-
 		if (!OidIsValid(dbOid) ||
 			proc->databaseId == dbOid)
 		{
@@ -1886,7 +1881,7 @@ CountDBBackends(Oid databaseid)
  * CancelDBBackends --- cancel backends that are using specified database
  */
 void
-CancelDBBackends(Oid databaseid)
+CancelDBBackends(Oid databaseid, ProcSignalReason sigmode, bool conflictPending)
 {
 	ProcArrayStruct *arrayP = procArray;
 	int			index;
@@ -1899,22 +1894,21 @@ CancelDBBackends(Oid databaseid)
 	{
 		volatile PGPROC *proc = arrayP->procs[index];
 
-		if (proc->databaseId == databaseid)
+		if (databaseid == InvalidOid || proc->databaseId == databaseid)
 		{
 			VirtualTransactionId procvxid;
 
 			GET_VXID_FROM_PGPROC(procvxid, *proc);
 
-			proc->recoveryConflictPending = true;
+			proc->recoveryConflictPending = conflictPending;
 			pid = proc->pid;
 			if (pid != 0)
 			{
 				/*
 				 * Kill the pid if it's still here. If not, that's what we wanted
 				 * so ignore any errors.
 				 */
-				(void) SendProcSignal(pid, PROCSIG_RECOVERY_CONFLICT_DATABASE,
-										procvxid.backendId);
+				(void) SendProcSignal(pid, sigmode, procvxid.backendId);
 			}
 		}
 	}

src/backend/storage/ipc/procsignal.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/ipc/procsignal.c,v 1.3 2010/01/16 10:05:50 sriggs Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/ipc/procsignal.c,v 1.4 2010/01/23 16:37:12 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -272,5 +272,8 @@ procsignal_sigusr1_handler(SIGNAL_ARGS)
 	if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_SNAPSHOT))
 		RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_SNAPSHOT);
 
+	if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN))
+		RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
+
 	errno = save_errno;
 }