From: Tom Lane <tgl@sss.pgh.pa.us>
To: 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>
Subject: Re: Intermittent buildfarm failures on wrasse
Date: Fri, 15 Apr 2022 14:14:47 -0400
Message-ID: <1737834.1650046487@sss.pgh.pa.us> (raw)
In-Reply-To: <20220415165850.rh7byj7ep5jmzuzb@alap3.anarazel.de>
References: <CAH2-WznXXkkfE_y9ieaHkEQj31HeMAqJBtJPmDY+EpyVg1QywQ@mail.gmail.com>
<1570894.1649986347@sss.pgh.pa.us>
<20220415015249.kfsqxujrf2bn2wz6@alap3.anarazel.de>
<20220415024515.GA862547@rfd.leadboat.com>
<20220415051205.6abhlilh6fnjgohr@alap3.anarazel.de>
<20220415052125.GD862547@rfd.leadboat.com>
<1637671.1650032132@sss.pgh.pa.us>
<1643686.1650036220@sss.pgh.pa.us>
<A11045E8-18D2-4BBB-8CF1-80DC6920ED73@anarazel.de>
<1650516.1650040612@sss.pgh.pa.us>
<20220415165850.rh7byj7ep5jmzuzb@alap3.anarazel.de>
Andres Freund <andres@anarazel.de> writes:
> On 2022-04-15 12:36:52 -0400, Tom Lane wrote:
>> Yeah, I was also thinking about a flag in PGPROC being a more reliable
>> way to do this. Is there anything besides walsenders that should set
>> that flag?
> Not that I can think of. It's only because of hs_feedback that we need
> to. I guess it's possible that somebody build some extension that needs
> something similar, but then they'd need to set that flag...
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.
I could use some help filling in the XXX comments, because it's far
from clear to me *why* walsenders need this to happen.
regards, tom lane
Attachments:
[text/x-diff] cleaner-walsender-handling-wip.patch (2.9K, ../1737834.1650046487@sss.pgh.pa.us/2-cleaner-walsender-handling-wip.patch)
download | inline diff:diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.cindex be40261393..a04a4a8e5d 100644--- a/src/backend/replication/walsender.c+++ b/src/backend/replication/walsender.c@@ -285,6 +285,15 @@ InitWalSender(void)
MarkPostmasterChildWalSender();
SendPostmasterSignal(PMSIGNAL_ADVANCE_STATE_MACHINE);
+ /*+ * Also, mark ourselves in PGPROC as affecting vacuum horizons in all+ * databases. XXX explain why+ */+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);+ MyProc->statusFlags |= PROC_AFFECTS_ALL_HORIZONS;+ ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;+ LWLockRelease(ProcArrayLock);+
/* Initialize empty timestamp buffer for lag tracking. */
lag_tracker = MemoryContextAllocZero(TopMemoryContext, sizeof(LagTracker));
}
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.cindex f6e98aae29..8b867bd56c 100644--- a/src/backend/storage/ipc/procarray.c+++ b/src/backend/storage/ipc/procarray.c@@ -1821,10 +1821,12 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
* to prune still needed data away). If the current backend never
* connects to a database that is harmless, because
* data_oldest_nonremovable will never be utilized.
+ *+ * XXX explain PROC_AFFECTS_ALL_HORIZONS, too
*/
if (in_recovery ||
MyDatabaseId == InvalidOid || proc->databaseId == MyDatabaseId ||
- proc->databaseId == 0) /* always include WalSender */+ (statusFlags & PROC_AFFECTS_ALL_HORIZONS))
{
/*
* We can ignore this backend if it's running CREATE INDEX
@@ -2681,10 +2683,14 @@ ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc)
/* Install xmin */
MyProc->xmin = TransactionXmin = xmin;
- /* 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;+ /* walsender cheats by passing proc == MyProc, don't check 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;+ }
result = true;
}
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.hindex 809b74db5e..e132665938 100644--- a/src/include/storage/proc.h+++ b/src/include/storage/proc.h@@ -60,6 +60,9 @@ struct XidCache
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
#define PROC_IN_LOGICAL_DECODING 0x10 /* currently doing logical
* decoding outside xact */
+#define PROC_AFFECTS_ALL_HORIZONS 0x20 /* proc's xmin must be+ * included in vacuum horizons+ * in all databases */
/* flags reset at EOXact */
#define PROC_VACUUM_STATE_MASK \
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, andres@anarazel.de, noah@leadboat.com, pg@bowt.ie, dgrowleyml@gmail.com
Subject: Re: Intermittent buildfarm failures on wrasse
In-Reply-To: <1737834.1650046487@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