Andres Freund <and...@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
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 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.c
index 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.h
index 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 \
