From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org, Noah Misch <noah@leadboat.com>
Cc: Peter Geoghegan <pg@bowt.ie>
Cc: David Rowley <dgrowleyml@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Subject: Re: Intermittent buildfarm failures on wrasse
Date: Tue, 19 Apr 2022 14:29:06 -0400
Message-ID: <2571201.1650392946@sss.pgh.pa.us> (raw)
In-Reply-To: <202204191637.eldwa2exvguw@alvherre.pgsql>
References: <202204191637.eldwa2exvguw@alvherre.pgsql>
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> 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?
Yeah, I thought about that too, but figured that the author probably
had a reason for writing the assertion the way it was. If we want
to redefine PROC_COPYABLE_FLAGS as "flags associated with xmin",
that's fine by me. But I'd suggest that both the name of the macro
and the comment for it in proc.h should be revised to match that
definition.
Another point is that as you've coded it, the code doesn't so much
copy those flags as union them with whatever the recipient had,
which seems wrong. I could go with either
Assert(!(MyProc->statusFlags & PROC_XMIN_FLAGS));
MyProc->statusFlags |= (proc->statusFlags & PROC_XMIN_FLAGS);
or
MyProc->statusFlags = (MyProc->statusFlags & ~PROC_XMIN_FLAGS) |
(proc->statusFlags & PROC_XMIN_FLAGS);
Perhaps the latter is more future-proof.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, alvherre@alvh.no-ip.org, andres@anarazel.de, noah@leadboat.com, pg@bowt.ie, dgrowleyml@gmail.com, amit.kapila16@gmail.com, sawada.mshk@gmail.com
Subject: Re: Intermittent buildfarm failures on wrasse
In-Reply-To: <2571201.1650392946@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox