Re: Persist slot invalidations before publishing them

On Wed, Sep 23, 2026 at 9:35 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Wed, Sep 23, 2026 at 04:07:02PM +0530, shveta malik wrote:
> > Can we create the helper function for this logic as it is not core to
> > the sync-function. Please use the attached patch if you agree, and
> > feel free to change comments as you see apt.
>
> I was initially a bit skeptical about adding a helper with only one caller, as it
> seemed to just move the PG_TRY() block elsewhere without reducing duplication.
>
> On second thought, I agree it makes sense here, as it keeps the lock acquisition
> and required ERROR cleanup ordering together. There are also similar "one caller"
> helpers wrapping PG_TRY() blocks, such as start_table_sync() and start_sequence_sync().
>
> +static void
> +persist_slot_invalidation(ReplicationSlot *slot, ReplicationSlotInvalidationCause cause)
> +{
>
> I changed it slightly to derive slot from MyReplicationSlot, ensuring that the I/O
> lock belongs to the same slot that ReplicationSlotPersistInvalidation() and
> and ReplicationSlotRelease() are using. This is also consistent with other operations
> on the currently acquired slot.

Yes, that makes sense.

> Please find v5 attached.

Code changes looks good to me. Regarding the test, I have one trivial
comment: I think we should cover all the cases:

a) Failed sync: Memory is NOT updated.
b) Successful sync: Memory IS updated.
c) Server restart: Memory is STILL updated (because it was recovered
from disk).

I think b) is not covered. It will be good to check the memory state
before standby-stop here:

$standby->safe_psql('postgres', 'SELECT pg_sync_replication_slots()');

$standby->stop('immediate');
$standby->start;

thanks
Shveta