On Fri, Sep 18, 2026 at 9:49 AM Bharath Rupireddy
<[email protected]> wrote:
>
> Hi,
>
> On Fri, Sep 18, 2026 at 1:31 AM kedar anavardekar
> <[email protected]> wrote:
> >
> > Two minor naming suggestions: (please take the suggestions if you
> > think the points are valid)
>
> Thanks for taking a look.
>
> > 1. Could MyReplicationSlotSubId be renamed to
> > MyReplicationSlotSubXactId (or MyReplicationSlotSubTransactionId)
> >
> > SubId may be read as a subscription ID, whereas this variable stores
> > the SubTransactionId of the subtransaction that acquired
> > MyReplicationSlot. The more explicit name would make its purpose
> > clearer and avoid confusion with logical replication subscriptions.
>
> Subscription and its related replication slot on the publisher are on
> two different database instances, and one has the context when reading
> the code around this. Also, "SubId" is used across the code base and I
> want to keep it consistent and short, so MyReplicationSlotSubId looks
> fine to me.
+1
> > 2., could the comment above AtEOSubXact_ReplicationSlot() be revised from:
> > /*
> > * At subxact end, release the replication slot if the subtransaction
> > * where the slot was acquired is aborted.
> > */
> > to:
> > /*
> > * At subxact end, release the replication slot if the subtransaction
> > * in which the slot was acquired is aborted.
> > */
> > “In which” is more precise here because the slot is acquired during
> > that subtransaction.
>
> I believe "where the slot was acquired" is grammatically correct as
> well, so I'm fine with the existing wording.
I reviewed the v17 patch and here are some comments:
for use by the current session. Temporary slots are also
- released upon any error. This function corresponds
+ dropped on any error. An error raised and caught in a
+ subtransaction, for example by a
+ <application>PL/pgSQL</application> exception block, does not
+ drop them. This function corresponds
ISTM what the following sentence says seems to contradict with what
the first sentence says. How about rephrasing it to:
for use by the current session. Temporary slots are also
- released upon any error. This function corresponds
+ dropped when an error is reported to the client. An error
caught inside a
+ subtransaction, for example by a <application>PL/pgSQL</application>
+ exception block, does not drop them. This function corresponds
---
+-- A slot function that errors out must still release the slot, otherwise the
+-- next slot operation in the session fails an assertion or leaks the slot.
+-- Advancing a freshly created slot to a low LSN always errors.
+SELECT 'init' FROM
pg_create_logical_replication_slot('regress_subxact_slot',
'test_decoding');
The comment seems not to be in the right place; it's in right before
the pg_create_logical_replication_slot() call but not related. Given
that we have the comments for subsequent tests, we can remove it.
I've attached the updated patch that incorporated the above comments.
I'm going to push it early next week, barring any objections.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
From 8b4cf7b61d0eaaef60c36ec8407fc5836d6875a3 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Thu, 17 Sep 2026 20:09:55 +0000
Subject: [PATCH v18] Fix replication slot leak on error caught in a
subtransaction.
The SQL-callable replication slot functions, such as
pg_replication_slot_advance(), acquire a slot and release it before
returning. When one of them throws an error, the slot is normally
released by the top-level error handler (the sigsetjmp block in
PostgresMain()). But PL/pgSQL, PL/Perl, PL/Python and PL/Tcl run the
statements they protect in an internal subtransaction, and when a
matching handler catches the error there, it is never re-thrown and
the top-level handler is never reached. Nothing on
the (sub)transaction abort path releases the slot either, so it stays
acquired.
The next slot operation in the session then hits an assertion failure,
or in a non-assertion build silently overwrites MyReplicationSlot. The
leaked slot keeps holding back WAL removal and the catalog xmin, and
can no longer be acquired.
Fix this by recording the subtransaction that acquires the slot and
releasing the slot when that subtransaction aborts. We cannot simply
release the slot on every subtransaction abort, because logical
decoding starts and aborts an internal (sub)transaction for each
decoded transaction while holding the slot.
Unlike the top-level handler, this path only releases the slot and
doesn't drop the session's temporary slots. An error caught in a
subtransaction is normally meant to be handled so that the session
carries on, so we leave them in place, which is also what happened
before this fix. Update the documentation accordingly. Note that a
slot whose creation fails partway is still left behind holding
resources, but that is a pre-existing problem not specific to
subtransactions.
Reported-by: SATYANARAYANA NARLAPURAM <[email protected]>
Author: Bharath Rupireddy <[email protected]>
Suggeted-by: Masahiko Sawada <[email protected]>
Reviewed-by: Fujii Masao <[email protected]>
Reviewed-by: shveta malik <[email protected]>
Reviewed-by: Hou Zhijie <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Reviewed-by: Ashutosh Sharma <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-by: Chao Li <[email protected]>
Reviewed-by: kedar anavardekar <[email protected]>
Discussion: https://postgr.es/m/CAHg+QDeuf9tCq3ce=kgFMJP0m=PZC+wi6B=ys+7v0vnxjls...@mail.gmail.com
Backpatch-through: 14
---
contrib/test_decoding/expected/slot.out | 86 +++++++++++++++++++++++++
contrib/test_decoding/sql/slot.sql | 42 ++++++++++++
doc/src/sgml/func/func-admin.sgml | 8 ++-
doc/src/sgml/protocol.sgml | 5 +-
doc/src/sgml/system-views.sgml | 5 +-
src/backend/access/transam/xact.c | 2 +
src/backend/replication/slot.c | 66 +++++++++++++++++++
src/backend/tcop/postgres.c | 4 +-
src/include/replication/slot.h | 1 +
9 files changed, 214 insertions(+), 5 deletions(-)
diff --git a/contrib/test_decoding/expected/slot.out b/contrib/test_decoding/expected/slot.out
index cf0136455b0..f722ebb0b9a 100644
--- a/contrib/test_decoding/expected/slot.out
+++ b/contrib/test_decoding/expected/slot.out
@@ -469,3 +469,89 @@ SELECT pg_drop_replication_slot('physical_slot');
(1 row)
+SELECT 'init' FROM pg_create_logical_replication_slot('regress_subxact_slot', 'test_decoding');
+ ?column?
+----------
+ init
+(1 row)
+
+-- Error raised inside a PL/pgSQL block with an EXCEPTION clause is caught in a
+-- subtransaction; the slot must still be released.
+DO $$
+BEGIN
+ PERFORM pg_replication_slot_advance('regress_subxact_slot', '0/1');
+EXCEPTION WHEN OTHERS THEN
+ RAISE NOTICE 'caught SQLSTATE %', SQLSTATE;
+END $$;
+NOTICE: caught SQLSTATE 55000
+SELECT active FROM pg_replication_slots WHERE slot_name = 'regress_subxact_slot';
+ active
+--------
+ f
+(1 row)
+
+SELECT count(*) >= 0 AS peek_ok
+ FROM pg_logical_slot_peek_changes('regress_subxact_slot', NULL, NULL);
+ peek_ok
+---------
+ t
+(1 row)
+
+-- The EXCEPTION clause does not match, so the error is not caught, but the
+-- slot is still released. Show only the SQLSTATE for stable output.
+\set VERBOSITY sqlstate
+DO $$
+BEGIN
+ PERFORM pg_replication_slot_advance('regress_subxact_slot', '0/1');
+EXCEPTION WHEN division_by_zero THEN
+ RAISE NOTICE 'unreachable';
+END $$;
+ERROR: 55000
+\set VERBOSITY default
+SELECT active FROM pg_replication_slots WHERE slot_name = 'regress_subxact_slot';
+ active
+--------
+ f
+(1 row)
+
+SELECT count(*) >= 0 AS peek_ok
+ FROM pg_logical_slot_peek_changes('regress_subxact_slot', NULL, NULL);
+ peek_ok
+---------
+ t
+(1 row)
+
+SELECT pg_drop_replication_slot('regress_subxact_slot');
+ pg_drop_replication_slot
+--------------------------
+
+(1 row)
+
+-- Unlike a top-level error, an error caught in a subtransaction releases the
+-- slot but leaves the session's temporary slots in place.
+SELECT 'init' FROM pg_create_logical_replication_slot('regress_subxact_temp_slot', 'test_decoding', true);
+ ?column?
+----------
+ init
+(1 row)
+
+DO $$
+BEGIN
+ PERFORM pg_replication_slot_advance('regress_subxact_temp_slot', '0/1');
+EXCEPTION WHEN OTHERS THEN
+ RAISE NOTICE 'caught SQLSTATE %', SQLSTATE;
+END $$;
+NOTICE: caught SQLSTATE 55000
+SELECT count(*) = 1 AS temp_slot_kept
+ FROM pg_replication_slots WHERE slot_name = 'regress_subxact_temp_slot';
+ temp_slot_kept
+----------------
+ t
+(1 row)
+
+SELECT pg_drop_replication_slot('regress_subxact_temp_slot');
+ pg_drop_replication_slot
+--------------------------
+
+(1 row)
+
diff --git a/contrib/test_decoding/sql/slot.sql b/contrib/test_decoding/sql/slot.sql
index 50a950341fc..ddbf4fa9995 100644
--- a/contrib/test_decoding/sql/slot.sql
+++ b/contrib/test_decoding/sql/slot.sql
@@ -193,3 +193,45 @@ SELECT pg_drop_replication_slot('failover_true_slot');
SELECT pg_drop_replication_slot('failover_false_slot');
SELECT pg_drop_replication_slot('failover_default_slot');
SELECT pg_drop_replication_slot('physical_slot');
+
+SELECT 'init' FROM pg_create_logical_replication_slot('regress_subxact_slot', 'test_decoding');
+
+-- Error raised inside a PL/pgSQL block with an EXCEPTION clause is caught in a
+-- subtransaction; the slot must still be released.
+DO $$
+BEGIN
+ PERFORM pg_replication_slot_advance('regress_subxact_slot', '0/1');
+EXCEPTION WHEN OTHERS THEN
+ RAISE NOTICE 'caught SQLSTATE %', SQLSTATE;
+END $$;
+SELECT active FROM pg_replication_slots WHERE slot_name = 'regress_subxact_slot';
+SELECT count(*) >= 0 AS peek_ok
+ FROM pg_logical_slot_peek_changes('regress_subxact_slot', NULL, NULL);
+
+-- The EXCEPTION clause does not match, so the error is not caught, but the
+-- slot is still released. Show only the SQLSTATE for stable output.
+\set VERBOSITY sqlstate
+DO $$
+BEGIN
+ PERFORM pg_replication_slot_advance('regress_subxact_slot', '0/1');
+EXCEPTION WHEN division_by_zero THEN
+ RAISE NOTICE 'unreachable';
+END $$;
+\set VERBOSITY default
+SELECT active FROM pg_replication_slots WHERE slot_name = 'regress_subxact_slot';
+SELECT count(*) >= 0 AS peek_ok
+ FROM pg_logical_slot_peek_changes('regress_subxact_slot', NULL, NULL);
+SELECT pg_drop_replication_slot('regress_subxact_slot');
+
+-- Unlike a top-level error, an error caught in a subtransaction releases the
+-- slot but leaves the session's temporary slots in place.
+SELECT 'init' FROM pg_create_logical_replication_slot('regress_subxact_temp_slot', 'test_decoding', true);
+DO $$
+BEGIN
+ PERFORM pg_replication_slot_advance('regress_subxact_temp_slot', '0/1');
+EXCEPTION WHEN OTHERS THEN
+ RAISE NOTICE 'caught SQLSTATE %', SQLSTATE;
+END $$;
+SELECT count(*) = 1 AS temp_slot_kept
+ FROM pg_replication_slots WHERE slot_name = 'regress_subxact_temp_slot';
+SELECT pg_drop_replication_slot('regress_subxact_temp_slot');
diff --git a/doc/src/sgml/func/func-admin.sgml b/doc/src/sgml/func/func-admin.sgml
index 64b0e7bb972..b6a23f65ece 100644
--- a/doc/src/sgml/func/func-admin.sgml
+++ b/doc/src/sgml/func/func-admin.sgml
@@ -1051,7 +1051,9 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
parameter, <parameter>temporary</parameter>, when set to true, specifies that
the slot should not be permanently stored to disk and is only meant
for use by the current session. Temporary slots are also
- released upon any error. This function corresponds
+ dropped when an error is reported to the client. An error caught inside a
+ subtransaction, for example by a <application>PL/pgSQL</application>
+ exception block, does not drop them. This function corresponds
to the replication protocol command <literal>CREATE_REPLICATION_SLOT
... PHYSICAL</literal>.
</para></entry>
@@ -1091,7 +1093,9 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
parameter, <parameter>temporary</parameter>, when set to true, specifies that
the slot should not be permanently stored to disk and is only meant
for use by the current session. Temporary slots are also
- released upon any error. The optional fourth parameter,
+ dropped when an error is reported to the client. An error caught inside a
+ subtransaction, for example by a <application>PL/pgSQL</application>
+ exception block, does not drop them. The optional fourth parameter,
<parameter>twophase</parameter>, when set to true, specifies
that the decoding of prepared transactions is enabled for this
slot. The optional fifth parameter,
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 8ced69f283b..ae1a04b3454 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2371,7 +2371,10 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
<para>
Specify that this replication slot is a temporary one. Temporary
slots are not saved to disk and are automatically dropped on error
- or when the session has finished.
+ or when the session has finished. An error raised and caught in a
+ subtransaction, for example by a
+ <application>PL/pgSQL</application> exception block, does not drop
+ them.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 77202e2c765..efefc43e6fc 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2894,7 +2894,10 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
<para>
True if this is a temporary replication slot. Temporary slots are
not saved to disk and are automatically dropped on error or when
- the session has finished.
+ the session has finished. An error raised and caught in a
+ subtransaction, for example by a
+ <application>PL/pgSQL</application> exception block, does not drop
+ them.
</para></entry>
</row>
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index ebb010853cf..4f1a91c1965 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -5205,6 +5205,7 @@ CommitSubTransaction(void)
s->parent->curTransactionOwner);
AtEOSubXact_LargeObject(true, s->subTransactionId,
s->parent->subTransactionId);
+ AtEOSubXact_ReplicationSlot(true, s->subTransactionId);
AtSubCommit_Notify();
CallSubXactCallbacks(SUBXACT_EVENT_COMMIT_SUB, s->subTransactionId,
@@ -5382,6 +5383,7 @@ AbortSubTransaction(void)
s->parent->curTransactionOwner);
AtEOSubXact_LargeObject(false, s->subTransactionId,
s->parent->subTransactionId);
+ AtEOSubXact_ReplicationSlot(false, s->subTransactionId);
AtSubAbort_Notify();
/* Advertise the fact that we aborted in pg_xact. */
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 63ce6d27885..45766541bed 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -59,6 +59,7 @@
#include "utils/builtins.h"
#include "utils/guc_hooks.h"
#include "utils/injection_point.h"
+#include "utils/snapmgr.h"
#include "utils/varlena.h"
#include "utils/wait_event.h"
@@ -157,6 +158,13 @@ const ShmemCallbacks ReplicationSlotsShmemCallbacks = {
/* My backend's replication slot in the shared memory array */
ReplicationSlot *MyReplicationSlot = NULL;
+/*
+ * Subxact that acquired MyReplicationSlot, or invalid if none is held
+ * or it was acquired with no transaction in progress (as a walsender does).
+ * Used to release the slot when that subxact aborts.
+ */
+static SubTransactionId MyReplicationSlotSubId = InvalidSubTransactionId;
+
/* GUC variables */
int max_replication_slots = 10; /* the maximum number of replication
* slots */
@@ -519,6 +527,7 @@ ReplicationSlotCreate(const char *name, bool db_specific,
slot->active_proc = MyProcNumber;
SpinLockRelease(&slot->mutex);
MyReplicationSlot = slot;
+ MyReplicationSlotSubId = GetCurrentSubTransactionId();
LWLockRelease(ReplicationSlotControlLock);
@@ -724,6 +733,7 @@ retry:
/* We made this slot active, so it's ours now. */
MyReplicationSlot = s;
+ MyReplicationSlotSubId = GetCurrentSubTransactionId();
/*
* We need to check for invalidation after making the slot ours to avoid
@@ -848,6 +858,61 @@ ReplicationSlotRelease(void)
pfree(slotname);
}
+
+ /* The slot is no longer acquired in any subxact. */
+ MyReplicationSlotSubId = InvalidSubTransactionId;
+}
+
+/*
+ * At subxact end, release the replication slot if the subtransaction
+ * where the slot was acquired is aborted.
+ */
+void
+AtEOSubXact_ReplicationSlot(bool isCommit, SubTransactionId mySubid)
+{
+ /* Nothing to do unless the slot was acquired in this subxact. */
+ if (MyReplicationSlotSubId != mySubid)
+ return;
+
+ /*
+ * The subxact that acquired the slot is committing with the slot still
+ * held. No slot function in the core code does that today, though an
+ * extension might. A slot left unreleased by mistake cannot be told apart
+ * from one the caller means to release later. Leave it to the caller.
+ */
+ if (isCommit)
+ {
+ MyReplicationSlotSubId = InvalidSubTransactionId;
+ return;
+ }
+
+ /*
+ * We must not get here while decoding is running. Decoding starts and
+ * aborts an internal (sub)transaction while holding the slot, for each
+ * decoded transaction (ReorderBufferProcessTXN()) and when executing
+ * invalidations (ReorderBufferImmediateInvalidation()). However, those
+ * subtransactions are always nested below the one that acquired the slot,
+ * so their subtransaction ids are deeper and do not match here. Decoding
+ * also runs with a historic snapshot set up, so assert that it is not.
+ */
+ Assert(!HistoricSnapshotActive());
+
+ /*
+ * The aborting subxact is the one that acquired the slot, so the slot is
+ * still held and must be released. MyReplicationSlotSubId is set only
+ * when a slot is held and cleared when it is released, so a matching
+ * subxact id means the slot is ours.
+ *
+ * We only release the slot here and do not drop the session's temporary
+ * slots, unlike the top-level error handler in PostgresMain(). An error
+ * caught within a subtransaction, for example by a PL/pgSQL exception
+ * block, is normally meant to be handled so the session carries on,
+ * unlike a top-level error, so a temporary slot is left in place. That
+ * matches the temporary slot behavior that predates this callback and is
+ * simpler to reason about; the slot lives on until the session ends or a
+ * top-level error occurs, as documented.
+ */
+ ReplicationSlotRelease();
}
/*
@@ -1042,6 +1107,7 @@ ReplicationSlotDropAcquired(bool try_disable)
/* slot isn't acquired anymore */
MyReplicationSlot = NULL;
+ MyReplicationSlotSubId = InvalidSubTransactionId;
ReplicationSlotDropPtr(slot);
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index b6bdfe213fe..a2f72f9eb5b 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4638,7 +4638,9 @@ PostgresMain(const char *dbname, const char *username)
* need to be able to start and abort transactions while having a slot
* acquired. But we never need to hold them across top level errors,
* so releasing here is fine. There also is a before_shmem_exit()
- * callback ensuring correct cleanup on FATAL errors.
+ * callback for FATAL errors, and AtEOSubXact_ReplicationSlot() for an
+ * error caught in a subtransaction. The latter only releases the slot
+ * and, unlike here, does not drop the session's temporary slots.
*/
if (MyReplicationSlot != NULL)
ReplicationSlotRelease();
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index 9b29444cbca..171ba6f2318 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -343,6 +343,7 @@ extern void ReplicationSlotAcquire(const char *name, bool nowait,
bool error_if_invalid);
extern void ReplicationSlotRelease(void);
extern void ReplicationSlotCleanup(bool synced_only);
+extern void AtEOSubXact_ReplicationSlot(bool isCommit, SubTransactionId mySubid);
extern void ReplicationSlotSave(void);
extern void ReplicationSlotMarkDirty(void);
--
2.55.0
