From 02eb9367f7a7757c5477d5034f5d7cda45d69318 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Sun, 8 Mar 2020 13:07:41 +0900
Subject: [PATCH v3 1/2] Fix double updating ps title when recovery conflict
 resolution with lock

---
 src/backend/storage/ipc/standby.c | 92 ++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 32 deletions(-)

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 3090e57fa4..3c0d42d7b4 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -47,6 +47,7 @@ static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlis
 static void SendRecoveryConflictWithBufferPin(ProcSignalReason reason);
 static XLogRecPtr LogCurrentRunningXacts(RunningTransactions CurrRunningXacts);
 static void LogAccessExclusiveLocks(int nlocks, xl_standby_lock *locks);
+static char *set_process_title_waiting(void);
 
 /*
  * Keep track of all the locks owned by a given transaction.
@@ -221,16 +222,10 @@ static void
 ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 									   ProcSignalReason reason)
 {
-	TimestampTz waitStart;
-	char	   *new_status;
-
 	/* Fast exit, to avoid a kernel call if there's no work to be done. */
 	if (!VirtualTransactionIdIsValid(*waitlist))
 		return;
 
-	waitStart = GetCurrentTimestamp();
-	new_status = NULL;			/* we haven't changed the ps display */
-
 	while (VirtualTransactionIdIsValid(*waitlist))
 	{
 		/* reset standbyWait_us for each xact we wait for */
@@ -239,25 +234,6 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 		/* wait until the virtual xid is gone */
 		while (!VirtualXactLock(*waitlist, false))
 		{
-			/*
-			 * Report via ps if we have been waiting for more than 500 msec
-			 * (should that be configurable?)
-			 */
-			if (update_process_title && new_status == NULL &&
-				TimestampDifferenceExceeds(waitStart, GetCurrentTimestamp(),
-										   500))
-			{
-				const char *old_status;
-				int			len;
-
-				old_status = get_ps_display(&len);
-				new_status = (char *) palloc(len + 8 + 1);
-				memcpy(new_status, old_status, len);
-				strcpy(new_status + len, " waiting");
-				set_ps_display(new_status, false);
-				new_status[len] = '\0'; /* truncate off " waiting" */
-			}
-
 			/* Is it time to kill it? */
 			if (WaitExceedsMaxStandbyDelay())
 			{
@@ -281,19 +257,13 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 		/* The virtual transaction is gone now, wait for the next one */
 		waitlist++;
 	}
-
-	/* Reset ps display if we changed it */
-	if (new_status)
-	{
-		set_ps_display(new_status, false);
-		pfree(new_status);
-	}
 }
 
 void
 ResolveRecoveryConflictWithSnapshot(TransactionId latestRemovedXid, RelFileNode node)
 {
 	VirtualTransactionId *backends;
+	char		*new_status = NULL;
 
 	/*
 	 * If we get passed InvalidTransactionId then we are a little surprised,
@@ -310,14 +280,29 @@ ResolveRecoveryConflictWithSnapshot(TransactionId latestRemovedXid, RelFileNode
 	backends = GetConflictingVirtualXIDs(latestRemovedXid,
 										 node.dbNode);
 
+	/* quick exit if there is no conflict on the xid */
+	if (!VirtualTransactionIdIsValid(*backends))
+		return;
+
+	/* Report via ps we are waiting */
+	new_status = set_process_title_waiting();
+
 	ResolveRecoveryConflictWithVirtualXIDs(backends,
 										   PROCSIG_RECOVERY_CONFLICT_SNAPSHOT);
+
+	/* Reset ps display if we changed it */
+	if (new_status)
+	{
+		set_ps_display(new_status, false);
+		pfree(new_status);
+	}
 }
 
 void
 ResolveRecoveryConflictWithTablespace(Oid tsid)
 {
 	VirtualTransactionId *temp_file_users;
+	char		*new_status = NULL;
 
 	/*
 	 * Standby users may be currently using this tablespace for their
@@ -338,8 +323,23 @@ ResolveRecoveryConflictWithTablespace(Oid tsid)
 	 */
 	temp_file_users = GetConflictingVirtualXIDs(InvalidTransactionId,
 												InvalidOid);
+
+	/* quick exit if there is no conflict on the tablespace */
+	if (!VirtualTransactionIdIsValid(*temp_file_users))
+		return;
+
+	/* Report via ps we are waiting */
+	new_status = set_process_title_waiting();
+
 	ResolveRecoveryConflictWithVirtualXIDs(temp_file_users,
 										   PROCSIG_RECOVERY_CONFLICT_TABLESPACE);
+
+	/* Reset ps display if we changed it */
+	if (new_status)
+	{
+		set_ps_display(new_status, false);
+		pfree(new_status);
+	}
 }
 
 void
@@ -384,6 +384,10 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
  *
  * Deadlocks involving the Startup process and an ordinary backend process
  * will be detected by the deadlock detector within the ordinary backend.
+ *
+ * Unlike other recovery conflict resolution functions, this function
+ * doesn't update the process title since we have already updated it in
+ * WaitOnLock().
  */
 void
 ResolveRecoveryConflictWithLock(LOCKTAG locktag)
@@ -1094,3 +1098,27 @@ LogStandbyInvalidations(int nmsgs, SharedInvalidationMessage *msgs,
 					 nmsgs * sizeof(SharedInvalidationMessage));
 	XLogInsert(RM_STANDBY_ID, XLOG_INVALIDATIONS);
 }
+
+/*
+ * Append " waiting" to the process title, and return palloc'd
+ * original process title.
+ */
+static char *
+set_process_title_waiting(void)
+{
+	const char	*old_status;
+	char		*ret_status;
+	int			len;
+
+	if (!update_process_title)
+		return NULL;
+
+	old_status = get_ps_display(&len);
+	ret_status = (char *) palloc(len + 8 + 1);
+	memcpy(ret_status, old_status, len);
+	strcpy(ret_status + len, " waiting");
+	set_ps_display(ret_status, false);
+	ret_status[len] = '\0'; /* truncate off " waiting" */
+
+	return ret_status;
+}
-- 
2.23.0