Re: [BUG] [PATCH] Allow physical replication slots to recover from archive after invalidation

I have done a detailed review of the patch and its functionality.
A slot is revalidated (if auto_revalidate=true) when it's recoverable,
and when it's not, the eventual behavior is the same as pre-fix,
i.e. if the slot is not recoverable, the walreceiver fatal's and
subsequently the walsender exits. The standby relaunches and attempts
to start replication again, leading to postmaster launching the
walsender again. This continues to happen, which is the same
behavior as pre-fix.

Also for a revalidatable and recoverable slot, during the window
between slot acquire and revalidation post the first flush
acknowledgement, there is a possibility for a checkpoint to occur
resulting in the removal of the required WAL. However, this
information is just for knowledge of the scenario rather than
the indication of any side effect.

For a slot that is revalidatable but not recoverable, during the
window between slot acquire and the first flush acknowledgement,
the slot will show up as active while invalid for a slightly
longer period, and continues to happen on every retry. To report
correctly during such windows, we can check for WAL availability
before slot acquire (before going active).

The fix produces an additional discrepancy in the slot status,
which is very unlikely to occur, through a phantom false clear.
A buffered 'r' reply from downstream can revalidate a slot for
a short duration before its invalidation again when the WAL is
not found. The log also produces a line that the slot has been
revalidated, leading to confusion. The slot is subsequently
invalidated again. It may be worth looking at how we can avoid
this, possibly by clearing the flag on a successful
restart_lsn advancement past the invalidation point, or after a
successful WAL send.

Can we consider allowing auto_revalidate via a GUC - to provide
the option across a fleet of slots, for convenience?

I have looked at what this means for pg_receivewal and
pg_basebackup. pg_basebackup can be ignored conveniently due to
its transient usage of a temporary slot. A slot that
pg_receivewal uses gets invalidated on idle time out, so as to
allow removing WALs that it blocks. Allowing auto_revalidate in
this case seems counter intuitive. So it seems there's nothing
to do in this context.

The feature does not affect upgrade as well since it does not
carry slots across. The slots need to be recreated anyway and
can therefore use the auto_revalidate flag accordingly.

f41d8468 centralized the "can no longer access replication slot"
error into ReplicationSlotAcquire() via error_if_invalid, for a
consistent message and to avoid missing the check as new
invalidation causes are added. This patch re-introduces a copy
of that message in StartReplication() for the non-revalidatable
branch, which is worth de-duplicating.

But I'd like to flag a design question rather than just propose
"move it back." Revalidation isn't a new invalidation cause;
it's a new policy — "error unless revalidatable, else warn and
attempt." A single boolean can't express that, and adding another
boolean (allow_revalidation) to ReplicationSlotAcquire() would
(a) start turning a slot-acquisition mechanism into a policy
switchboard that grows a flag per future scenario, and
(b) couple that general function (also used by logical decoding,
slotsync, the launcher) to a physical-slot concept
(SlotCanBeRevalidated).

So: is the intended contract of ReplicationSlotAcquire() to be the
central invalidation gatekeeper — in which case an extensible
policy argument (e.g. an enum) would age better than accreting
booleans — or an acquisition mechanism, in which case the
invalidation policy (error/warn/attempt) could stay with each caller
and only the error message be single-sourced via a small shared
helper? The latter keeps the walsender's revalidation-specific
behavior in the walsender and still honors the message-consistency
goal of f41d8468. Either direction removes the duplication;
I lean toward the shared-helper approach, but I think the contract
is the authors' call.

--
Thanks & Regards,
Sravan Velagandula
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Jun 9, 2026 at 9:54 PM Joao Foltran <joao(at)foltrandba(dot)com> wrote:

> Fixed the TAP test because a timing condition affected its execution in
> the CI.
>
> JOÃO FOLTRAN
>