Thread

1. Separate catalog_xmin from xmin in walsender hot standby feedback

   Rui Zhao <zhaorui126@gmail.com> — 2026-04-30T08:29:32Z

   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
   
   
   

2. Re: Separate catalog_xmin from xmin in walsender hot standby feedback

   Rui Zhao <zhaorui126@gmail.com> — 2026-06-08T07:57:25Z

   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
   
   

3. Re: Separate catalog_xmin from xmin in walsender hot standby feedback

   Amit Kapila <amit.kapila16@gmail.com> — 2026-06-08T11:29:44Z

   On Mon, Jun 8, 2026 at 1:27 PM Rui Zhao <zhaorui126@gmail.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.
   
   
   
   

4. Re: Separate catalog_xmin from xmin in walsender hot standby feedback

   Rui Zhao <zhaorui126@gmail.com> — 2026-06-15T02:48:38Z

   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