recovery_conflict_lock_deadlock_v2.patch

text/plain

Filename: recovery_conflict_lock_deadlock_v2.patch
Type: text/plain
Part: 0
Message: Re: Deadlock between backend and recovery may not be detected

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
Series: patch v2
File+
src/backend/storage/ipc/procarray.c 8 1
src/backend/storage/ipc/standby.c 90 12
src/backend/tcop/postgres.c 12 2
src/include/storage/procarray.h 2 0
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index ee912b9d5e..7c1a3f8b3f 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3324,6 +3324,13 @@ GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid)
  */
 pid_t
 CancelVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode)
+{
+	return SignalVirtualTransaction(vxid, sigmode, true);
+}
+
+pid_t
+SignalVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode,
+						 bool conflictPending)
 {
 	ProcArrayStruct *arrayP = procArray;
 	int			index;
@@ -3342,7 +3349,7 @@ CancelVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode)
 		if (procvxid.backendId == vxid.backendId &&
 			procvxid.localTransactionId == vxid.localTransactionId)
 		{
-			proc->recoveryConflictPending = true;
+			proc->recoveryConflictPending = conflictPending;
 			pid = proc->pid;
 			if (pid != 0)
 			{
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 4ea3cf1f5c..4b7762644e 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -42,10 +42,15 @@ int			max_standby_streaming_delay = 30 * 1000;
 
 static HTAB *RecoveryLockLists;
 
+/* Is a deadlock check pending? */
+static volatile sig_atomic_t got_standby_deadlock_timeout;
+
 static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 												   ProcSignalReason reason,
 												   uint32 wait_event_info,
 												   bool report_waiting);
+static void SendRecoveryConflictWithLock(ProcSignalReason reason,
+										 LOCKTAG locktag);
 static void SendRecoveryConflictWithBufferPin(ProcSignalReason reason);
 static XLogRecPtr LogCurrentRunningXacts(RunningTransactions CurrRunningXacts);
 static void LogAccessExclusiveLocks(int nlocks, xl_standby_lock *locks);
@@ -395,8 +400,10 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
  * lock.  As we are already queued to be granted the lock, no new lock
  * requests conflicting with ours will be granted in the meantime.
  *
- * Deadlocks involving the Startup process and an ordinary backend process
- * will be detected by the deadlock detector within the ordinary backend.
+ * We also must check for deadlocks involving the Startup process and
+ * hot-standby backend processes. If deadlock_timeout is reached in
+ * this function, the backends are requested to check themselves for
+ * deadlocks.
  */
 void
 ResolveRecoveryConflictWithLock(LOCKTAG locktag)
@@ -407,7 +414,7 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag)
 
 	ltime = GetStandbyLimitTime();
 
-	if (GetCurrentTimestamp() >= ltime)
+	if (GetCurrentTimestamp() >= ltime && ltime != 0)
 	{
 		/*
 		 * We're already behind, so clear a path as quickly as possible.
@@ -429,19 +436,47 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag)
 	else
 	{
 		/*
-		 * Wait (or wait again) until ltime
+		 * Wait (or wait again) until ltime, and check for deadlocks as well
+		 * if we will be waiting longer than deadlock_timeout
 		 */
-		EnableTimeoutParams timeouts[1];
+		EnableTimeoutParams timeouts[2];
+		int			cnt = 0;
 
-		timeouts[0].id = STANDBY_LOCK_TIMEOUT;
-		timeouts[0].type = TMPARAM_AT;
-		timeouts[0].fin_time = ltime;
-		enable_timeouts(timeouts, 1);
+		if (ltime != 0)
+		{
+			timeouts[cnt].id = STANDBY_LOCK_TIMEOUT;
+			timeouts[cnt].type = TMPARAM_AT;
+			timeouts[cnt].fin_time = ltime;
+			cnt++;
+		}
+
+		timeouts[cnt].id = STANDBY_DEADLOCK_TIMEOUT;
+		timeouts[cnt].type = TMPARAM_AFTER;
+		timeouts[cnt].delay_ms = DeadlockTimeout;
+		cnt++;
+
+		enable_timeouts(timeouts, cnt);
 	}
 
 	/* Wait to be signaled by the release of the Relation Lock */
+	got_standby_deadlock_timeout = false;
 	ProcWaitForSignal(PG_WAIT_LOCK | locktag.locktag_type);
 
+	if (got_standby_deadlock_timeout)
+	{
+		/*
+		 * Send out a request for hot-standby backends to check themselves for
+		 * deadlocks.
+		 */
+		SendRecoveryConflictWithLock(
+									 PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK,
+									 locktag);
+		got_standby_deadlock_timeout = false;
+
+		/* Wait again to be signaled by the release of the Relation Lock */
+		ProcWaitForSignal(PG_WAIT_LOCK | locktag.locktag_type);
+	}
+
 	/*
 	 * Clear any timeout requests established above.  We assume here that the
 	 * Startup process doesn't have any other outstanding timeouts than those
@@ -520,8 +555,23 @@ ResolveRecoveryConflictWithBufferPin(void)
 	}
 
 	/* Wait to be signaled by UnpinBuffer() */
+	got_standby_deadlock_timeout = false;
 	ProcWaitForSignal(PG_WAIT_BUFFER_PIN);
 
+	if (got_standby_deadlock_timeout)
+	{
+		/*
+		 * Send out a request for hot-standby backends to check themselves for
+		 * deadlocks.
+		 */
+		SendRecoveryConflictWithBufferPin(
+										  PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
+		got_standby_deadlock_timeout = false;
+
+		/* Wait again to be signaled by UnpinBuffer() */
+		ProcWaitForSignal(PG_WAIT_BUFFER_PIN);
+	}
+
 	/*
 	 * Clear any timeout requests established above.  We assume here that the
 	 * Startup process doesn't have any other timeouts than what this function
@@ -531,6 +581,30 @@ ResolveRecoveryConflictWithBufferPin(void)
 	disable_all_timeouts(false);
 }
 
+static void
+SendRecoveryConflictWithLock(ProcSignalReason reason, LOCKTAG locktag)
+{
+	VirtualTransactionId *backends;
+
+	Assert(reason == PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
+
+	backends = GetLockConflicts(&locktag, AccessExclusiveLock, NULL);
+
+	/* Quick exit if there's no work to be done */
+	if (!VirtualTransactionIdIsValid(*backends))
+		return;
+
+	/*
+	 * We send signal to all the backends holding the conflicting locks, to
+	 * ask them to check for deadlocks.
+	 */
+	while (VirtualTransactionIdIsValid(*backends))
+	{
+		SignalVirtualTransaction(*backends, reason, false);
+		backends++;
+	}
+}
+
 static void
 SendRecoveryConflictWithBufferPin(ProcSignalReason reason)
 {
@@ -588,13 +662,17 @@ CheckRecoveryConflictDeadlock(void)
 
 /*
  * StandbyDeadLockHandler() will be called if STANDBY_DEADLOCK_TIMEOUT
- * occurs before STANDBY_TIMEOUT.  Send out a request for hot-standby
- * backends to check themselves for deadlocks.
+ * occurs before STANDBY_TIMEOUT.
  */
 void
 StandbyDeadLockHandler(void)
 {
-	SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
+	int			save_errno = errno;
+
+	got_standby_deadlock_timeout = true;
+
+	SetLatch(MyLatch);
+	errno = save_errno;
 }
 
 /*
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 3679799e50..385a749e46 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2919,11 +2919,21 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
 			case PROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
 
 				/*
-				 * If we aren't blocking the Startup process there is nothing
-				 * more to do.
+				 * If PROCSIG_RECOVERY_CONFLICT_BUFFERPIN is requested but we
+				 * aren't blocking the Startup process there is nothing more
+				 * to do.
+				 *
+				 * If PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK is requested
+				 * and we're waiting for locks but don't hold buffer pins
+				 * blocking the Startup process, we set the flag so that
+				 * ProcSleep() will check for deadlocks.
 				 */
 				if (!HoldingBufferPinThatDelaysRecovery())
+				{
+					if (reason == PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK)
+						CheckDeadLockAlert();
 					return;
+				}
 
 				MyProc->recoveryConflictPending = true;
 
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index ea8a876ca4..4d272c7287 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -72,6 +72,8 @@ extern VirtualTransactionId *GetCurrentVirtualXIDs(TransactionId limitXmin,
 												   int *nvxids);
 extern VirtualTransactionId *GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid);
 extern pid_t CancelVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode);
+extern pid_t SignalVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode,
+									  bool conflictPending);
 
 extern bool MinimumActiveBackends(int min);
 extern int	CountDBBackends(Oid databaseid);