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: