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

Hi Hackers,

When hot_standby_feedback is on and wal_receiver_status_interval is 0,
the primary keeps the xmin sent at connection time and never releases
it, so VACUUM on the primary is held back. I see this on PG16, 17, 18,
19 and master. PG14 and PG15 are not affected (reason at the end).

*Cause:* XLogWalRcvSendHSFeedback() decides "feedback is disabled" in two
different ways. The wakeup time and the early return treat it as disabled
when
the interval is <= 0 or hot_standby_feedback is off, but the code that
picks the
xmin checks only hot_standby_feedback. So the first message at connect
(immed = true)
carries a real xmin, and then no more messages are sent because the wakeup
is infinity.

The same happens when the interval is changed from a positive value to 0
and the configuration is reloaded. The feedback sent on reload carries
a fresh valid xmin, and then nothing follows.

*Steps to reproduce:*

initdb -D pg20_primary
echo "port = 5525" >> pg20_primary/postgresql.conf
pg_ctl -D pg20_primary -l primary.log -w start

pg_basebackup -p 5525 -D pg20_standby -R -X stream
cat >> pg20_standby/postgresql.conf <<EOF
port = 5526
hot_standby_feedback = on
wal_receiver_status_interval = 0
EOF
pg_ctl -D pg20_standby -l standby.log -w start

primary="psql -p 5525 -d postgres -X"
$primary -c "select backend_xmin, age(backend_xmin)
from pg_stat_replication"
backend_xmin | age
--------------+-----
664 | 0

$primary -c "create table hsf_t with (autovacuum_enabled = off)
as select g a from generate_series(1,100000) g"
for i in $(seq 2000); do echo "select txid_current();"; done |
$primary -Atq > /dev/null
$primary -c "delete from hsf_t"
$primary -c "checkpoint"
sleep 6

$primary -c "select backend_xmin, age(backend_xmin) from
pg_stat_replication"
backend_xmin | age
--------------+------
664 | 2013

$primary -c "vacuum (verbose, index_cleanup off) hsf_t"
tuples: 0 removed, 100000 remain, 100000 are dead but not yet removable
removable cutoff: 664, which was 2013 XIDs old when operation ended

The xmin does not move. As a control, set the interval to 1 on the
standby and reload:

psql -p 5526 -d postgres -X -c "alter system set
wal_receiver_status_interval = 1" -c "select pg_reload_conf()"
sleep 3

Now backend_xmin is 2677 (age 0), and the next VACUUM removes all the
100000 tuples.

*Fix:* compute the xmin only when feedback is really enabled.

- if (hot_standby_feedback)
+ if (hot_standby_feedback && wal_receiver_status_interval > 0)

With interval 0, the walreceiver now sends InvalidTransactionId, so the
primary releases the xmin. This keeps "0 disables the status updates",
as the documentation says. Please let me know if people agree with it.

The patches also add a TAP test (0NN_hot_standby_feedback_status_interval.pl
in src/test/recovery). It uses a physical slot, so the xmin kept by the
primary is visible in pg_replication_slots.xmin. It checks that a positive
interval sends a valid xmin (control), and that interval 0 leaves no
xmin, both on reload and on connect. The zero-interval cases look at the
DEBUG2 feedback message in the standby log, so there is no timing race,
and then poll until the slot xmin is NULL. It fails without the fix and
passes with it.

The test makes the walreceiver reconnect before checking the connect-time
feedback. No feedback is sent while Hot Standby is not yet active, and
with feedback disabled it is not retried. So an old slot xmin may stay if
the walreceiver connects too early. That is a separate issue, and I will
start a new thread for it.

I am attaching 4 patches. The code change is the same in all. Only the
test file number and the meson.build context differ per branch:

v1-walreceiver-Do-not-leave-stale-xmin-on-primary-when-status-interval-is-zero-Master.patch

v1-walreceiver-Do-not-leave-stale-xmin-on-primary-when-status-interval-is-zero-PG19.patch

v1-walreceiver-Do-not-leave-stale-xmin-on-primary-when-status-interval-is-zero-PG18.patch

v1-walreceiver-Do-not-leave-stale-xmin-on-primary-when-status-interval-is-zero-PG16-17.patch

Why PG14 and PG15 are not affected: I ran the same steps there, and the
xmin keeps moving (733 to 2735 on PG14, 734 to 2736 on PG15). These
versions have no wakeup time array. Their "at most once per interval"
check is

if (!TimestampDifferenceExceeds(sendTime, now,
wal_receiver_status_interval * 1000))
return;

and with interval 0 it is always true, so feedback with a fresh xmin
goes out on every call. The infinite wakeup was added in PG16. (Note
that in PG14 and PG15 an interval of 0 does not really disable the
feedback, but I did not change that.)

Thanks and regards,
Vaibhav Dalvi