Re: Temporary slot leak when creation fails in a subtransaction - Mailing list pgsql-hackers

From shveta malik
Subject Re: Temporary slot leak when creation fails in a subtransaction
Date
Msg-id CAJpy0uCJmEv2i7F+1KrDaMCTBZ-+0zQSZd2Me+7+DNeTfU_pMw@mail.gmail.com
Whole thread
In response to Temporary slot leak when creation fails in a subtransaction  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: Temporary slot leak when creation fails in a subtransaction
List pgsql-hackers
On Fri, Sep 25, 2026 at 11:09 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.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?

thanks
Shveta



pgsql-hackers by date:

Previous
From: Bertrand Drouvot
Date:
Subject: Re: Persist slot invalidations before publishing them
Next
From: Grigorev Jurij
Date:
Subject: Re: BUG #19599: RestoreBlockImage: the decode cross-checks never bound hole_offset + hole_length against BLCKSZ