| Lists: | pgsql-hackers |
|---|
| From: | Rui Zhao <zhaorui126(at)gmail(dot)com> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Separate catalog_xmin from xmin in walsender hot standby feedback |
| Date: | 2026-04-30 08:29:32 |
| Message-ID: | 3544CABD-2D09-4D7F-AFB7-2C75A09C4E22@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hi hackers,
I'd like to propose a fix for a long-standing issue where hot standby
feedback catalog_xmin incorrectly holds back vacuuming of user data
tables on the primary when no physical replication slot is used.
== Problem ==
When a standby sends hot standby feedback to a primary without a
physical replication slot, ProcessStandbyHSFeedbackMessage() takes
min(feedbackCatalogXmin, feedbackXmin) and stores it into
MyProc->xmin:
if (TransactionIdIsNormal(feedbackCatalogXmin)
&& TransactionIdPrecedes(feedbackCatalogXmin, feedbackXmin))
MyProc->xmin = feedbackCatalogXmin;
else
MyProc->xmin = feedbackXmin;
Since ComputeXidHorizons() treats proc->xmin uniformly for both data
and catalog horizons, the catalog_xmin ends up holding back
data_oldest_nonremovable, preventing vacuum from cleaning dead tuples
in regular user tables.
The existing code even acknowledges this limitation:
"We can only track the catalog xmin separately when using a slot,
so we store the least of the two provided when not using a slot."
== Why this matters ==
One might argue "just use a replication slot." However, many
production HA deployments intentionally avoid physical replication
slots because of their lifecycle management complexity:
- When a primary fails, physical slots on the old primary are lost
and cannot be automatically migrated to the promoted standby.
- Other standbys that were using slots on the old primary must
re-establish their slots on the new primary, potentially requiring
a fresh base backup.
- Dangling slots from disconnected standbys can cause unbounded WAL
accumulation until manually dropped.
These deployments use wal_keep_size or WAL archiving for WAL
retention, combined with hot_standby_feedback for visibility horizon
management. This is a legitimate production configuration -- for
example, some HA frameworks (Patroni with certain configurations,
custom HA scripts) operate this way.
The issue becomes severe when the standby also hosts a logical
replication slot (e.g., for change data capture or logical replication
to a downstream). The logical slot's catalog_xmin can be very old
(retained for logical decoding catalog access), and this old value
gets propagated to the primary's walsender via hot standby feedback,
blocking vacuum on ALL user data tables on the primary. This leads
to table bloat that is difficult to diagnose since the DBA may not
realize the connection between a standby's logical slot and the
primary's vacuum behavior.
== Fix ==
The patch adds a catalog_xmin field to PGPROC (4 bytes), so the
walsender can track catalog_xmin separately from xmin even without a
replication slot. This mirrors how replication slots already separate
slot->data.xmin from slot->data.catalog_xmin.
In ComputeXidHorizons(), the new proc_catalog_xmin is accumulated
from PGPROC entries and applied only to catalog_oldest_nonremovable
and shared_oldest_nonremovable -- exactly how slot_catalog_xmin is
already handled. It does NOT affect data_oldest_nonremovable.
GetReplicationHorizons() is updated to include proc_catalog_xmin in
the catalog_xmin sent upstream, ensuring correct behavior in
cascading standby configurations.
Changes summary:
- proc.h: add catalog_xmin to PGPROC
- proc.c: initialize catalog_xmin in InitProcess/InitAuxiliaryProcess
- procarray.c: accumulate and apply proc_catalog_xmin in
ComputeXidHorizons(); include in GetReplicationHorizons()
- walsender.c: set MyProc->xmin and MyProc->catalog_xmin separately
in the no-slot path of ProcessStandbyHSFeedbackMessage()
== Alternatives considered ==
1. Generalize the ephemeral slot concept (as suggested by the existing
XXX comment): this would automatically create a temporary slot for
slot-less walsenders. More invasive, requires slot allocation
(max_replication_slots), and adds slot lifecycle management.
2. Simply ignore catalog_xmin in the no-slot path: simpler but loses
catalog protection for the standby's logical decoding.
The proposed approach is minimal, correct, and consistent with how
slots already handle the separation.
== Testing ==
A new TAP test (053_hs_feedback_catalog_xmin.pl) verifies:
1. With hot_standby_feedback=on and no physical replication slot, when
the standby has a logical slot with an old catalog_xmin, VACUUM on
the primary can still clean dead tuples in user data tables.
2. The standby's logical slot catalog_xmin remains properly set,
confirming catalog protection is preserved.
Patch attached.
Regards,
Rui Zhao
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Separate-catalog_xmin-from-xmin-in-walsender-hs-feedback.patch | application/octet-stream | 18.2 KB |
| unknown_filename | text/plain | 2 bytes |
| From: | Rui Zhao <zhaorui126(at)gmail(dot)com> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Cc: | andres(at)anarazel(dot)de, amit(dot)kapila16(at)gmail(dot)com |
| Subject: | Re: Separate catalog_xmin from xmin in walsender hot standby feedback |
| Date: | 2026-06-08 07:57:25 |
| Message-ID: | 273DD115-F20F-4521-880C-FC08A7DE99D3@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hi,
Attaching v2, rebased on top of current master (v1 no longer applied
cleanly per cfbot). Two improvements were also made since v1:
1. Test stability
The TAP test (053_hs_feedback_catalog_xmin.pl) replaced a sleep(2)
with a poll on pg_stat_replication.reply_time, so it deterministically
waits for a fresh hot standby feedback round to reach the primary
instead of relying on wall-clock time. This removes the most obvious
source of flakiness on slower CI runners.
2. Performance impact (pgbench)
I was concerned about adding a UINT32_ACCESS_ONCE(proc->catalog_xmin)
read and an extra TransactionIdOlder() call per PGPROC iteration in
ComputeXidHorizons(), which is on a hot path. Measured on a 104-core
box, scale=30, 64 clients / 32 jobs / 60s, -M prepared, optimized
build (CFLAGS=-O2, no --enable-cassert):
workload baseline median patched median delta
---------------------- --------------- -------------- -----
pgbench -S (read-only) 1,596,196 1,617,083 +1.3%
pgbench (read-write) 129,550 132,024 +1.9%
Numbers are medians of 3 warm runs after a 30s warmup, after I
discarded the cold-start runs which were dominated by cache priming.
The patched build comes out marginally above baseline, which is of
course noise -- adding code does not make things faster -- but it
confirms the change is below the measurement floor on this hardware.
Happy to rerun on different shapes (more clients, smaller scale,
different machine) if anyone wants to see specific numbers.
Summary of the change (unchanged in substance from v1):
- proc.h: add catalog_xmin field to PGPROC (4 bytes)
- proc.c: initialize it in InitProcess / InitAuxiliaryProcess
- procarray.c: accumulate proc_catalog_xmin in ComputeXidHorizons()
and apply it only to catalog_oldest_nonremovable and
shared_oldest_nonremovable; include it in GetReplicationHorizons()
so the catalog_xmin propagates correctly in cascading setups
- walsender.c: in the no-slot path of ProcessStandbyHSFeedbackMessage(),
set MyProc->xmin and MyProc->catalog_xmin separately instead of
folding them together
Registered in the 2026-07 commitfest:
https://commitfest.postgresql.org/patch/6861/
Adding Andres (who wrote much of the ProcArray horizon code) and Amit
Kapila (logical replication / slots), in case you have any thoughts on
the approach here -- in particular whether tracking catalog_xmin in
PGPROC is preferable to generalizing the ephemeral-slot idea hinted at
by the existing XXX comment in ProcessStandbyHSFeedbackMessage().
Regards,
Rui Zhao
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Separate-catalog_xmin-from-xmin-in-walsender-hot-.patch | application/octet-stream | 19.4 KB |
| unknown_filename | text/plain | 4.7 KB |
| v1-0001-Separate-catalog_xmin-from-xmin-in-walsender-hs-feedback.patch | application/octet-stream | 18.2 KB |
| unknown_filename | text/plain | 7 bytes |
| From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
|---|---|
| To: | Rui Zhao <zhaorui126(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org, andres(at)anarazel(dot)de |
| Subject: | Re: Separate catalog_xmin from xmin in walsender hot standby feedback |
| Date: | 2026-06-08 11:29:44 |
| Message-ID: | CAA4eK1L4UMHwmtuD0ppTFvWbGQpHhBjxsuS-rSwVoiYQN=fYyw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On Mon, Jun 8, 2026 at 1:27 PM Rui Zhao <zhaorui126(at)gmail(dot)com> wrote:
>
> Attaching v2, rebased on top of current master (v1 no longer applied
> cleanly per cfbot). Two improvements were also made since v1:
>
> 1. Test stability
> The TAP test (053_hs_feedback_catalog_xmin.pl) replaced a sleep(2)
> with a poll on pg_stat_replication.reply_time, so it deterministically
> waits for a fresh hot standby feedback round to reach the primary
> instead of relying on wall-clock time. This removes the most obvious
> source of flakiness on slower CI runners.
>
> 2. Performance impact (pgbench)
> I was concerned about adding a UINT32_ACCESS_ONCE(proc->catalog_xmin)
> read and an extra TransactionIdOlder() call per PGPROC iteration in
> ComputeXidHorizons(), which is on a hot path. Measured on a 104-core
> box, scale=30, 64 clients / 32 jobs / 60s, -M prepared, optimized
> build (CFLAGS=-O2, no --enable-cassert):
>
> workload baseline median patched median delta
> ---------------------- --------------- -------------- -----
> pgbench -S (read-only) 1,596,196 1,617,083 +1.3%
> pgbench (read-write) 129,550 132,024 +1.9%
>
> Numbers are medians of 3 warm runs after a 30s warmup, after I
> discarded the cold-start runs which were dominated by cache priming.
> The patched build comes out marginally above baseline, which is of
> course noise -- adding code does not make things faster -- but it
> confirms the change is below the measurement floor on this hardware.
> Happy to rerun on different shapes (more clients, smaller scale,
> different machine) if anyone wants to see specific numbers.
>
> Summary of the change (unchanged in substance from v1):
>
> - proc.h: add catalog_xmin field to PGPROC (4 bytes)
> - proc.c: initialize it in InitProcess / InitAuxiliaryProcess
> - procarray.c: accumulate proc_catalog_xmin in ComputeXidHorizons()
> and apply it only to catalog_oldest_nonremovable and
> shared_oldest_nonremovable; include it in GetReplicationHorizons()
> so the catalog_xmin propagates correctly in cascading setups
> - walsender.c: in the no-slot path of ProcessStandbyHSFeedbackMessage(),
> set MyProc->xmin and MyProc->catalog_xmin separately instead of
> folding them together
>
I think storing catalog_xmin separately in PGPROC has few downsides
which are (a) it always needs additional four bytes in PGPROC which is
used for backend and other processes even though it is required for
walsender, (b) as both xmin and catalog_xmin are written separately
ComputeXidHorizons() could read one value as updated and other stale.
There may be something more fundamental which I may be missing but I
feel ephermal slots idea as hinted in comments is worth exploring.
--
With Regards,
Amit Kapila.
| From: | Rui Zhao <zhaorui126(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, andres(at)anarazel(dot)de |
| Subject: | Re: Separate catalog_xmin from xmin in walsender hot standby feedback |
| Date: | 2026-06-15 02:48:38 |
| Message-ID: | 57E2263F-455D-49A5-A16D-E66562DBC027@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hi Amit,
Thanks for the careful review. Following your suggestion to explore the
ephemeral-slot direction hinted at in the XXX comment in walsender.c
(ProcessStandbyHSFeedbackMessage), I built a working prototype and ran it
through the design questions you raised. Sharing what I found, since it
changes my recommendation.
== The prototype ==
When a standby connects WITHOUT a replication slot and sends hot standby
feedback, the walsender now lazily creates an *ephemeral* physical slot
(RS_EPHEMERAL, named pg_walsender_<pid>) on first feedback, and tracks the
two horizons through it:
if (MyReplicationSlot == NULL)
ReplicationSlotCreate("pg_walsender_<pid>", false, RS_EPHEMERAL,
false, false, false, false,
/* error_if_full */ false);
if (MyReplicationSlot != NULL)
PhysicalReplicationSlotNewXmin(feedbackXmin, feedbackCatalogXmin);
else
/* graceful degradation -- see below */
This directly addresses both of your concerns:
(a) Per-PGPROC memory: nothing is added to PGPROC. The 4 bytes/proc that
the previous patch spent on every backend are gone; only walsenders
that actually receive feedback consume anything, and they consume a
slot they were arguably entitled to all along.
(b) Torn read between xmin and catalog_xmin: PhysicalReplicationSlotNewXmin
updates effective_xmin and effective_catalog_xmin together under
slot->mutex, so ComputeXidHorizons never observes a half-updated pair.
The atomicity falls out of the existing slot machinery for free.
== WAL retention ==
The obvious worry with a slot is restart_lsn pinning WAL. In practice this
is bounded and, I'd argue, desirable:
- The slot is ephemeral, so it is dropped automatically when the walsender
exits (before_shmem_exit -> ReplicationSlotShmemExit ->
ReplicationSlotRelease), including on standby disconnect/crash and on
crash-restart. There is no persistent slot left behind to pile up WAL
indefinitely -- which was the historical reason this path avoided slots.
- While the standby is connected, restart_lsn keeps the WAL the standby
still needs. That is precisely the behavior we want: today the no-slot
path can let the primary recycle WAL out from under a perfectly healthy
standby. Pinning it is a feature, not a regression.
- The worst case (connected but hopelessly lagging) is bounded by
max_slot_wal_keep_size, and a dead-but-unnoticed standby is bounded by
wal_sender_timeout (default 60s) before the walsender exits and the slot
drops.
== Slot-pool exhaustion ==
The one real pitfall is that ephemeral slots consume max_replication_slots.
If the pool is exhausted, ReplicationSlotCreate would ereport(ERROR), which
in the walsender would tear down the replication connection -- a regression
for feedback-only standbys that work fine today.
I solved this by adding an error_if_full flag to ReplicationSlotCreate
(default-equivalent true for all existing callers; behavior unchanged). When
false, a full pool releases ReplicationSlotAllocationLock and returns false
instead of erroring, and the walsender degrades gracefully to the pre-existing
behavior -- holding back the older of the two horizons via its own PGPROC
xmin:
/* slot pool exhausted: never break the connection */
MyProc->xmin = min(feedbackXmin, feedbackCatalogXmin);
So the separation is best-effort: you get the catalog/data split whenever a
slot is available, and you fall back to exactly today's behavior when it is
not. No connection is ever broken by this change.
== Visibility ==
One user-facing consequence worth flagging: each feedback standby now surfaces
a pg_walsender_<pid> physical slot in pg_replication_slots while it is
connected (it disappears when the walsender exits and the ephemeral slot is
dropped). Note the temporary column reads 'f' there, since the slot is
RS_EPHEMERAL rather than RS_TEMPORARY, so to a casual reader it looks like an
ordinary persistent physical slot distinguished only by its name prefix.
This is harmless functionally, but monitoring that counts slots or alerts on
"unexpected" slots will see these appear, and the 'f' in temporary may
mislead someone into thinking they won't be cleaned up automatically (they
are). The PGPROC approach has no such surface -- the horizon is held silently
on the walsender's proc entry. So this is a genuine trade-off between the two:
visibility/observability (you can actually see what each standby is pinning)
versus invisibility (nothing new shows up anywhere). I think surfacing it is
mostly a feature, but I wanted to call it out explicitly rather than have it
surprise anyone.
== Validation ==
I tested this on a live primary/standby pair (rebased onto current master).
A standby connects with no slot and hot_standby_feedback = on, and a logical
slot on the standby drives a catalog_xmin distinct from the data xmin. On the
primary, pg_replication_slots then shows:
slot_name | pg_walsender_3908747
slot_type | physical
xmin | 696 <- data horizon
catalog_xmin | 695 <- catalog horizon
So the two horizons are tracked separately, exactly as intended. Before this
change the slotless path would have collapsed them into MyProc->xmin =
min(696, 695) = 695, needlessly holding the data-table vacuum horizon back by
one. With the patch the data horizon stays at 696 and only catalog vacuum is
held at 695.
I also confirmed the lifecycle: the ephemeral slot is created lazily on first
feedback, and it is dropped automatically when the standby disconnects (the
slot count on the primary returns to zero), so there is no WAL pile-up left
behind.
== Recommendation ==
Given the above, I lean toward the ephemeral-slot approach over the PGPROC
approach: it removes the per-proc cost, gets atomicity for free, and reuses
existing, well-understood slot machinery instead of introducing a parallel
horizon-tracking path. The new surface is small: the error_if_full flag, plus
the visibility trade-off noted above (a pg_walsender_<pid> slot per connected
feedback standby, versus the PGPROC approach holding the horizon silently).
The attached prototype compiles cleanly and passes the live test above on
current master. Before I post a formal patch I wanted your read on the
direction -- in particular whether consuming (and exposing) a
max_replication_slots entry per feedback standby is acceptable given the
graceful fallback, or whether you'd prefer the pool accounting handled
differently.
Thanks again,
Rui
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Separate-catalog_xmin-from-xmin-via-ephemeral-phy.patch | application/octet-stream | 8.9 KB |
| unknown_filename | text/plain | 2.9 KB |