On Thu, Aug 27, 2026 at 9:19 PM shveta malik <[email protected]> wrote: > > On Thu, Aug 27, 2026 at 6:00 PM Amit Kapila <[email protected]> wrote: > > > > On Thu, Aug 27, 2026 at 11:58 AM Masahiko Sawada <[email protected]> > > wrote: > > > > > > On Mon, Aug 24, 2026 at 3:29 PM Bharath Rupireddy > > > <[email protected]> wrote: > > > > > > > > > > > > In short, having just the slot release in the subxact path gives the > > > > same error behavior, is simple to reason about, and fixes the crash > > > > reported in this thread. > > > > > > One thing I'm a bit concerned about is that this would be the first > > > caller to invoke ReplicationSlotRelease() from inside the transaction > > > machinery. > > > > > > > True, but OTOH, won't we already clean up resources not directly > > associated with subxact in AtEOSubXact_LargeObject() or > > AtEOSubXact_Files()? I don't see any problem as far as the current > > pattern of usage for slots. > > I agree. > > > The new restriction this patch will add is > > "a slot acquired in a subxact does not survive that subxact being > > unwound." which should be okay because of its similarity with > > top-level xact behavior. I feel if possible we should restrict such > > usage explicitly in code in some way rather than one finding out this > > as a surprise. > > > > * > > An error raised and caught in a > > + subtransaction, for example by a > > + <application>PL/pgSQL</application> exception block, does not > > drop > > + them. > > > > Based on above, something like below won't clean up temp slots and end > > up holding xmin. > > DO $$ BEGIN > > PERFORM pg_create_logical_replication_slot('s', 'nonexistent_plugin', > > true); > > EXCEPTION WHEN OTHERS THEN RAISE NOTICE '%', SQLERRM; > > END $$; > > Well, on rethinking, I feel that if we encounter an error while > creating a slot, whether persistent or temporary, the slot should be > dropped right there. > > This already works correctly for persistent slots: by the time the > slot reaches ReplicationSlotRelease, it is still in RS_EPHEMERAL state > and is therefore dropped by release. OTIOH, a temporary slot is left > behind. I think the temporary slot should also be dropped because the > caller never received a reference to it. I don't see a legitimate use > case where a temp slot should survive specifically because its > creation call failed.
While I agree that it would be an ideal behavior and the analysis holds for logical slots, I want to note that persistent physical replication slots are created with RS_PERSISTENT so if an error happens during the slot creation the slot is left behind. Also, logical persistent slots actually have the same gap: if ReplicationSlotPersist() raises an error it leaves a persistent slot behind as well. Given that slot creation and drop are not transactional operations, and that leaving a slot behind on a failure is not a new behavior, I'm inclined toward only releasing the slot at the subxact abort. We can discuss the better behavior on HEAD separately. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
