Re: walreceiver: stale hot_standby_feedback xmin on primary when wal_receiver_status_interval = 0 (PG16 and later)

Hi Vaibhav,

Thank you for finding and fixing this. A stale xmin that holds back vacuum
on the primary, with nothing visible on the standby side, is hard to
diagnose in production, so I think the fix is worth having.

I reviewed
v2-walreceiver-Do-not-leave-stale-xmin-on-primary-when-status-interval-is-zero-Master.patch
(2026-10-19). I did not test the back-branch variants.

Tested against master c074737f687f604b64f6ee830fccbbe854719823 (2026-11-09),
Debian bookworm container, ./configure --enable-cassert --enable-debug
--enable-tap-tests.

Applying and building
- git apply works cleanly on current master.
- Build is clean, with no compiler warnings.

Tests
- New test 061_hot_standby_feedback_status_interval.pl, with only the test
applied (no fix): fails. The first check passes, then wait_for_log times
out after the default 180 seconds waiting for "sending hot standby
feedback xmin 0 ...". So the test does catch the bug.
- Same test with the fix: 5 tests, PASS.
- make check: all 243 tests passed.
- make -C src/test/recovery check: 60 files, 695 tests, PASS.

Manual testing
Setup: primary on port 5525 (wal_level = logical, autovacuum = off, physical
slot sb), standby created with pg_basebackup -R -S sb, hot_standby_feedback
= on.

1. Standby started with wal_receiver_status_interval = 0, walsender
terminated once so the walreceiver reconnects after Hot Standby is active.
Then on the primary: create a table with 100000 rows, burn 1000 XIDs, delete
all rows, VACUUM VERBOSE.
- master: slot xmin stays at 664. VACUUM: "0 removed, 100000 remain,
100000 are dead but not yet removable", "removable cutoff: 664, which
was 1002 XIDs old".
- patched: slot xmin is NULL. VACUUM: "100000 removed, 0 remain",
"removable cutoff: 1666, which was 1 XIDs old".
2. Reload interval 0 -> 1 (control): slot xmin advances on both builds, as
expected.
3. Reload interval 1 -> 0:
- master: slot xmin stays at the value sent on reload (1666) and does not
move after 500 more XIDs.
- patched: slot xmin becomes NULL right after the reload.
4. hot_standby_feedback = off: slot xmin NULL on both builds, as expected.

So the patch does what the commit message says, and the stale xmin is gone.

5. Logical slot on the standby (test_decoding), created with interval = 1,
then interval reloaded to 0. On the primary: 50 CREATE TABLE / DROP TABLE
pairs, then VACUUM pg_class, pg_attribute, pg_type, pg_depend.
- master: the primary slot keeps catalog_xmin 664 (stale, but it protects
the standby slot). The standby slot is not invalidated, and
pg_logical_slot_get_changes() works.
- patched: the primary slot catalog_xmin becomes NULL. On the standby:
LOG: invalidating obsolete replication slot "lsb"
DETAIL: The slot conflicted with xid horizon 675.
pg_replication_slots shows invalidation_reason = rows_removed, and
decoding fails with
ERROR: can no longer access replication slot "lsb"

See comment 2 for this case.

Comments

1. Semantics of wal_receiver_status_interval = 0. The patch makes "interval
= 0" mean "hot standby feedback is off". If I read REL_15_STABLE correctly,
the behaviour there was different. In XLogWalRcvSendHSFeedback(), with
interval = 0, TimestampDifferenceExceeds(sendTime, now, 0) is always true,
so feedback with a valid xmin was sent on every non-immediate call. So on
PG14/15 the combination hot_standby_feedback = on with interval = 0 worked
(feedback was sent often). On PG16+ it sends one message and then leaves the
xmin stale. With this patch it sends no xmin at all. A user upgrading from
15 with this setting would see a third behaviour, which may cause unexpected
query cancellations on the standby. I think the "off" semantics is a
reasonable choice, and it matches the early-return condition, but I wonder
if restoring the PG15 behaviour (keep sending feedback when interval = 0)
was considered. In any case, it would help to say in the commit message
which behaviour is intended, and why, since this is going to the back
branches.

2. catalog_xmin and logical slots on the standby. As case 5 shows, with the
fix a logical slot on the standby can be invalidated when interval = 0, even
with hot_standby_feedback = on and a physical slot, which is the setup that
logicaldecoding.sgml recommends. Before the fix, the stale catalog_xmin kept
the slot usable (as a side effect of the bug). I think this is a consequence
of comment 1 rather than a new bug, but it is user visible. A few options,
not all needed:
- Mention it in the commit message and the release notes.
- slotsync.c (the check at line 1291) requires hot_standby_feedback for
slot synchronization, but does not check wal_receiver_status_interval.
With this patch, interval = 0 means the primary gets no catalog_xmin
for the synced slots, so the same check could also reject interval = 0,
or at least warn about it.
- A WARNING at walreceiver start, or on reload, when hot_standby_feedback
= on and wal_receiver_status_interval = 0, saying that no feedback will
be sent.

3. Documentation. Neither the hot_standby_feedback nor the
wal_receiver_status_interval entry in config.sgml says that interval = 0
disables hot standby feedback. hot_standby_feedback says "Feedback messages
will not be sent more frequently than once per
wal_receiver_status_interval", which a reader could take to mean "no limit"
when the value is 0. A sentence such as "If wal_receiver_status_interval is
zero, hot standby feedback is not sent." would make the new behaviour clear.

4. Code. The condition "hot_standby_feedback && wal_receiver_status_interval
> 0" now appears in a negated form in the early return at the top of the
function, in WalRcvComputeNextWakeup(), and in the new check. A small helper
or local bool (for example "feedback_enabled") would keep the three places
consistent. Minor, and a matter of taste.

5. Known gap. As you note in the commit message, an old xmin left on the
slot by a previous connection is not released when Hot Standby is not active
yet at connect time. Since that leaves the same symptom (vacuum held back by
a slot that nothing updates), it may be worth a short XXX comment near the
HotStandbyActive() check, so the gap is visible in the code and not only in
the commit message.

6. Test.
- The test covers the reload and connect cases well, and the comments
explain why the walsender is terminated. Nice.
- wal_receiver_timeout = 0 is set without a comment. I assume it avoids a
timeout when no status messages are sent with interval = 0. A short
comment would help.
- On a build without the fix, the test fails only after the 180 second
wait_for_log timeout. That is the usual pattern, so fine.
- There is no check of catalog_xmin. If the behaviour in comment 2 is
intended, a check that catalog_xmin is also released (or a logical slot
test) would document it.

Verdict: Waiting on Author, mainly for comments 1 and 2. I think the
semantics of interval = 0, and the impact on logical slots on the standby,
should be settled (and written down in the commit message and docs) before
this goes to the back branches. The code change itself is small and correct
for the chosen semantics, and the test passes and fails as expected.

Thanks again, and I am happy to test the next version, including the
back-branch patches.

Regards,
Joao Rito