diff --git a/contrib/test_decoding/expected/slot.out b/contrib/test_decoding/expected/slot.out index e30778e7e99..f7906f1e4c4 100644 --- a/contrib/test_decoding/expected/slot.out +++ b/contrib/test_decoding/expected/slot.out @@ -530,3 +530,31 @@ SELECT pg_drop_replication_slot('regress_subxact_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 8f133c1faf4..bd292c41e9b 100644 --- a/contrib/test_decoding/sql/slot.sql +++ b/contrib/test_decoding/sql/slot.sql @@ -225,3 +225,16 @@ 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..5b250f46ed2 100644 --- a/doc/src/sgml/func/func-admin.sgml +++ b/doc/src/sgml/func/func-admin.sgml @@ -1051,7 +1051,10 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset parameter, temporary, 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 on any error. An error raised and caught in a + subtransaction, for example by a + PL/pgSQL exception block, does not + drop them. This function corresponds to the replication protocol command CREATE_REPLICATION_SLOT ... PHYSICAL. @@ -1091,7 +1094,10 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset parameter, temporary, 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 on any error. An error raised and caught in a + subtransaction, for example by a + PL/pgSQL exception block, does not + drop them. The optional fourth parameter, twophase, when set to true, specifies that the decoding of prepared transactions is enabled for this slot. The optional fifth parameter, diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 2dd0bd9ce8d..6efb0c8f1ad 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -50,6 +50,7 @@ #include "replication/logicallauncher.h" #include "replication/logicalworker.h" #include "replication/origin.h" +#include "replication/slot.h" #include "replication/snapbuild.h" #include "replication/syncrep.h" #include "storage/aio_subsys.h" @@ -5203,8 +5204,7 @@ CommitSubTransaction(void) s->parent->curTransactionOwner); AtEOSubXact_LargeObject(true, s->subTransactionId, s->parent->subTransactionId); - AtEOSubXact_ReplicationSlot(true, s->subTransactionId, - s->parent->subTransactionId); + AtEOSubXact_ReplicationSlot(true, s->subTransactionId); AtSubCommit_Notify(); CallSubXactCallbacks(SUBXACT_EVENT_COMMIT_SUB, s->subTransactionId, @@ -5382,8 +5382,7 @@ AbortSubTransaction(void) s->parent->curTransactionOwner); AtEOSubXact_LargeObject(false, s->subTransactionId, s->parent->subTransactionId); - AtEOSubXact_ReplicationSlot(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 d67ec8db86d..adfa3152f71 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -40,6 +40,7 @@ #include #include "access/transam.h" +#include "access/xact.h" #include "access/xlog_internal.h" #include "access/xlogrecovery.h" #include "common/file_utils.h" @@ -864,13 +865,11 @@ ReplicationSlotRelease(void) } /* - * At subxact end, hand off or release MyReplicationSlot if it was acquired - * in this subxact. On commit, ownership passes to the parent subxact; on - * abort, the slot is released. + * At subxact end, release the replication slot if the subtransaction + * where the slot was acquired is aborted. */ void -AtEOSubXact_ReplicationSlot(bool isCommit, SubTransactionId mySubid, - SubTransactionId parentSubid) +AtEOSubXact_ReplicationSlot(bool isCommit, SubTransactionId mySubid) { /* Nothing to do unless the slot was acquired in this subxact. */ if (MyReplicationSlotSubId != mySubid) @@ -879,18 +878,12 @@ AtEOSubXact_ReplicationSlot(bool isCommit, SubTransactionId mySubid, /* * 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. We don't know of a reason to keep a slot past the - * commit of the subxact that acquired it, and the slot was most likely - * left unreleased by mistake, so warn. If a real use case turns up, the - * WARNING can be dropped. Either way, hand the slot to the parent so it - * is still released if an ancestor aborts. + * 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) { - elog(WARNING, "replication slot \"%s\" acquired in a subtransaction is still held at its commit; ownership transferred to the parent", - NameStr(MyReplicationSlot->data.name)); - - MyReplicationSlotSubId = parentSubid; + MyReplicationSlotSubId = InvalidSubTransactionId; return; } diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h index 31f8755dc39..171ba6f2318 100644 --- a/src/include/replication/slot.h +++ b/src/include/replication/slot.h @@ -343,8 +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, - SubTransactionId parentSubid); +extern void AtEOSubXact_ReplicationSlot(bool isCommit, SubTransactionId mySubid); extern void ReplicationSlotSave(void); extern void ReplicationSlotMarkDirty(void);