Date: Tue, 19 Apr 2022 18:37:13 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org,
	Noah Misch <noah@leadboat.com>, Peter Geoghegan <pg@bowt.ie>,
	David Rowley <dgrowleyml@gmail.com>,
	Amit Kapila <amit.kapila16@gmail.com>,
	Masahiko Sawada <sawada.mshk@gmail.com>
Subject: Re: Intermittent buildfarm failures on wrasse
Message-ID: <202204191637.eldwa2exvguw@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="jzwajoaj6dasettn"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1737834.1650046487@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/202204191637.eldwa2exvguw%40alvherre.pgsql>
Precedence: bulk


--jzwajoaj6dasettn
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2022-Apr-15, Tom Lane wrote:

> Here's a WIP patch for that.  The only exciting thing in it is that
> because of some undocumented cowboy programming in walsender.c, the
> 	Assert((proc->statusFlags & (~PROC_COPYABLE_FLAGS)) == 0);
> in ProcArrayInstallRestoredXmin fires unless we skip that.

Hmm, maybe a better use of that define is to use to select which flags
to copy, rather than to ensure we they are the only ones set.  What
about this?


-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"¿Qué importan los años?  Lo que realmente importa es comprobar que
a fin de cuentas la mejor edad de la vida es estar vivo"  (Mafalda)

--jzwajoaj6dasettn
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment; filename="copyflags.patch"

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 25c310f675..4347941568 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -2685,17 +2685,14 @@ ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc)
 		TransactionIdIsNormal(xid) &&
 		TransactionIdPrecedesOrEquals(xid, xmin))
 	{
-		/* Install xmin */
+		/*
+		 * Install xmin.  In addition, propagate statusFlags that affect how
+		 * the value is interpreted by vacuum.
+		 */
 		MyProc->xmin = TransactionXmin = xmin;
 
-		/* walsender cheats by passing proc == MyProc, don't check its flags */
-		if (proc != MyProc)
-		{
-			/* Flags being copied must be valid copy-able flags. */
-			Assert((proc->statusFlags & (~PROC_COPYABLE_FLAGS)) == 0);
-			MyProc->statusFlags = proc->statusFlags;
-			ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
-		}
+		MyProc->statusFlags |= (proc->statusFlags & PROC_COPYABLE_FLAGS);
+		ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
 
 		result = true;
 	}

--jzwajoaj6dasettn--