Re: Temporary slot leak when creation fails in a subtransaction

Hi,

Can you please point me to the PG19 patch on which I need to apply the
patch shared on this thread ? I am interested in reviewing this work too.

Thank you,
Narayanan

On Tue, Sep 29, 2026 at 2:40 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:

> On Tue, Sep 29, 2026 at 2:05 PM shveta malik <shveta(dot)malik(at)gmail(dot)com>
> wrote:
> >
> > On Fri, Sep 25, 2026 at 11:09 PM Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > >
> > > Hi,
> > >
> > > Moving the discussion from [1] here, and CCing the people involved
> there.
> > >
> > > A temporary slot (logical or physical) whose creation fails inside a
> > > subtransaction, with the error caught by a PL/pgSQL exception block,
> > > is left behind. Such a slot stays until the session ends, a top-level
> > > error happens in that session, or the user drops it. A temporary
> > > logical slot holds WAL and the catalog xmin, and the same session can
> > > decode partial transactions from it (data loss for logical decoding).
> > > A temporary physical slot holds WAL.
> > >
> > > Here is a reproducer for a temporary logical slot [2]. The slot is
> > > left with restart_lsn and catalog_xmin set and confirmed_flush_lsn not
> > > set. A slot in that state can still be used for decoding, and with no
> > > confirmed_flush_lsn that can lose data (see the test case in the
> > > attached patch). There is no start point, and since decoding from the
> > > slot is not slot creation, the snapshot builder can restore a
> > > serialized snapshot and become consistent in the middle of a
> > > transaction that began before restart_lsn. Without the fix, the
> > > healthy slot in the test returns the transaction with both inserts and
> > > the leftover slot returns it with only the second one.
> > >
> > > A temporary physical slot can also be left behind this way, but only
> > > when writing its state file fails after the slot is allocated, for
> > > example when the disk runs out of space.
> > >
> > > The issue exists on HEAD and all supported branches back to PG14. The
> > > fix in the other thread releases a slot that stays acquired after an
> > > error is caught in a subtransaction. That release does not drop a
> > > temporary slot whose creation failed. Even without that fix, on
> > > non-assert builds the leftover slot just stays marked active and can
> > > be decoded from, so the same data loss happens.
> > >
> > > A few ways to fix this were discussed in the other thread.
> > >
> > > 1/ Mark temporary slots as ephemeral initially and transition them to
> > > temporary once creation succeeds. Invasive. Not back-patchable.
> > >
> > > 2/ Introduce a new state to represent a temporary slot still in
> > > creation (RS_TEMPORARY_EPHEMERAL or such). Invasive. Makes the code
> > > complex. Not back-patchable.
> > >
> > > 3/ Use a boolean in the ReplicationSlot structure
> > > (is_create_in_progress or such), and in the subxact callback, when the
> > > slot is temporary and that boolean is set, drop just that slot and
> > > leave the others alone. Not ABI compatible. Not back-patchable.
> > >
> > > 4/ At subxact abort, if the slot is a temporary logical slot and its
> > > confirmed_flush is invalid, drop it. The drop needs to unset the
> > > PROC_IN_LOGICAL_DECODING flag as ReplicationSlotRelease() does.
> > >
> > > 5/ Add a check to CreateDecodingContext() that raises an error if the
> > > slot's confirmed_flush is invalid. On v17 and later, slot
> > > synchronization needs to be excluded, since update_local_synced_slot()
> > > advances a newly created synced slot via
> > > LogicalSlotAdvanceAndCheckSnapState() before its confirmed_flush is
> > > set.
> > >
> > > 6/ Track the in-progress creation with a backend-local variable
> > > (something like MyReplicationSlotCreating), set when the slot is
> > > created and reset when it is released. In the subxact callback, use it
> > > to drop just the temporary slot being created. Back-patchable. Covers
> > > both logical and physical temporary slots.
> > >
> > > I prefer the backend-local tracking approach and attached a patch for
> > > it on top of v19 from the other thread. Please have a look. I also
> > > think it is good to tighten this up with the CreateDecodingContext()
> > > check, so that a slot with an invalid confirmed_flush can never be
> > > used for decoding.
> > >
> > > Thoughts?
> > >
> > > [1]
> https://www.postgresql.org/message-id/CAHg+QDeuf9tCq3ce=kgFMJP0m=PZC+wi6B=yS+7V0vNXjLS31w@mail.gmail.com
> > >
> > > [2]
> > > DO $$
> > > BEGIN
> > > PERFORM pg_create_logical_replication_slot('tmp_bad',
> 'no_such_plugin', true);
> > > EXCEPTION WHEN OTHERS THEN
> > > RAISE NOTICE 'caught %', SQLSTATE;
> > > END $$;
> > >
> >
> > Thanks for the patch Bharath. The patch does not apply to HEAD (even
> > on an older version dated 15 days ago). Can you please rebase it?
> >
>
> Kuroda-san informed me off-list that it applies atop PG19 patch shared
> in another thread. I will review.
> But one question: are we planning to push this fix too to PG19 and
> thus the patch is this way? And then later we can do it on HEAD, is
> that the plan?
>
> thanks
> Shveta
>
>
>