On Sat, Aug 29, 2026 at 4:19 AM Masahiko Sawada <[email protected]> wrote: > > 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. >
Okay, fair enough. I agree. thanks Shveta
