From 6c3bc70ff50c98cbb876d6815af0ae32f012b81f Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Fri, 25 Sep 2026 16:58:23 +0000 Subject: [PATCH v1] Drop a temporary replication slot if its creation fails. 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. A temporary physical slot holds WAL. A leftover temporary logical slot has 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. 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. 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. A persistent logical slot does not have this problem. It is created as ephemeral and dropped on release if the creation fails. Temporary slots have no such phase and rely on the top-level error handler dropping the session's temporary slots, which an error caught in a subtransaction never reaches. Fix this by tracking the in-progress creation with a backend-local variable, set when the slot is created and reset when it is released. If the subtransaction that created a temporary slot aborts with that variable still set, drop the slot instead of releasing it. Nothing changes on disk or in shared memory. Backpatch to all supported versions. Reported-by: Chao Li Reported-by: Amit Kapila Author: Bharath Rupireddy Discussion: https://postgr.es/m/ Backpatch-through: 14 --- contrib/test_decoding/expected/slot.out | 29 +++++++++ .../expected/slot_creation_error.out | 64 +++++++++++++++++++ .../specs/slot_creation_error.spec | 49 ++++++++++++++ contrib/test_decoding/sql/slot.sql | 14 ++++ doc/src/sgml/func/func-admin.sgml | 6 +- doc/src/sgml/protocol.sgml | 2 +- doc/src/sgml/system-views.sgml | 2 +- src/backend/replication/slot.c | 48 ++++++++++---- src/backend/tcop/postgres.c | 3 +- 9 files changed, 200 insertions(+), 17 deletions(-) diff --git a/contrib/test_decoding/expected/slot.out b/contrib/test_decoding/expected/slot.out index 53d8b445b23..c36ec3a0280 100644 --- a/contrib/test_decoding/expected/slot.out +++ b/contrib/test_decoding/expected/slot.out @@ -558,3 +558,32 @@ SELECT pg_drop_replication_slot('regress_subxact_temp_slot'); (1 row) +-- A temporary slot whose creation fails inside a subtransaction, with the +-- error caught by a PL/pgSQL exception block, is dropped, so the same name can +-- be used again. +DO $$ +BEGIN + PERFORM pg_create_logical_replication_slot('regress_subxact_temp_slot', 'no_such_plugin', true); +EXCEPTION WHEN OTHERS THEN + RAISE NOTICE 'caught SQLSTATE %', SQLSTATE; +END $$; +NOTICE: caught SQLSTATE 42501 +SELECT count(*) = 0 AS temp_slot_dropped + FROM pg_replication_slots WHERE slot_name = 'regress_subxact_temp_slot'; + temp_slot_dropped +------------------- + t +(1 row) + +SELECT 'init' FROM pg_create_logical_replication_slot('regress_subxact_temp_slot', 'test_decoding', true); + ?column? +---------- + init +(1 row) + +SELECT pg_drop_replication_slot('regress_subxact_temp_slot'); + pg_drop_replication_slot +-------------------------- + +(1 row) + diff --git a/contrib/test_decoding/expected/slot_creation_error.out b/contrib/test_decoding/expected/slot_creation_error.out index 25883b508fb..192895d4b7d 100644 --- a/contrib/test_decoding/expected/slot_creation_error.out +++ b/contrib/test_decoding/expected/slot_creation_error.out @@ -76,6 +76,70 @@ pg_drop_replication_slot (1 row) +starting permutation: s1_init s1_b s1_insert1 s2_init_temp s1_cancel_s2 s1_insert2 s1_c s1_view_temp_slot s1_get_changes s2_get_changes s1_drop +step s1_init: + SELECT 'init' FROM pg_create_logical_replication_slot('slot_a', 'test_decoding'); + +?column? +-------- +init +(1 row) + +step s1_b: BEGIN; +step s1_insert1: INSERT INTO tbl VALUES (1); +step s2_init_temp: + DO $$ + BEGIN + PERFORM pg_create_logical_replication_slot('tmp_slot_b', 'test_decoding', true); + EXCEPTION WHEN query_canceled THEN + NULL; + END $$; + +step s1_cancel_s2: + SELECT pg_cancel_backend(pid) + FROM pg_stat_activity + WHERE application_name = 'isolation/slot_creation_error/s2'; + +step s2_init_temp: <... completed> +step s1_cancel_s2: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step s1_insert2: INSERT INTO tbl VALUES (2); +step s1_c: COMMIT; +step s1_view_temp_slot: + SELECT slot_name, slot_type, active FROM pg_replication_slots WHERE slot_name = 'tmp_slot_b' + +slot_name|slot_type|active +---------+---------+------ +(0 rows) + +step s1_get_changes: + SELECT data FROM pg_logical_slot_get_changes('slot_a', NULL, NULL, 'include-xids', '0'); + +data +-------------------------------------- +BEGIN +table public.tbl: INSERT: a[integer]:1 +table public.tbl: INSERT: a[integer]:2 +COMMIT +(4 rows) + +step s2_get_changes: + SELECT data FROM pg_logical_slot_get_changes('tmp_slot_b', NULL, NULL, 'include-xids', '0'); + +ERROR: replication slot "tmp_slot_b" does not exist +step s1_drop: + SELECT pg_drop_replication_slot('slot_a'); + +pg_drop_replication_slot +------------------------ + +(1 row) + + starting permutation: s1_b s1_xid s2_init s1_terminate_s2 s1_c s1_view_slot step s1_b: BEGIN; step s1_xid: SELECT 'xid' FROM txid_current(); diff --git a/contrib/test_decoding/specs/slot_creation_error.spec b/contrib/test_decoding/specs/slot_creation_error.spec index d1e35bf58b5..128319dcf22 100644 --- a/contrib/test_decoding/specs/slot_creation_error.spec +++ b/contrib/test_decoding/specs/slot_creation_error.spec @@ -1,10 +1,22 @@ # Test that erroring out during logical slot creation is handled properly +setup +{ + CREATE TABLE tbl (a int); +} + +teardown +{ + DROP TABLE tbl; +} + session "s1" setup { SET synchronous_commit=on; } step s1_b { BEGIN; } step s1_xid { SELECT 'xid' FROM txid_current(); } +step s1_insert1 { INSERT INTO tbl VALUES (1); } +step s1_insert2 { INSERT INTO tbl VALUES (2); } step s1_c { COMMIT; } step s1_cancel_s2 { SELECT pg_cancel_backend(pid) @@ -26,16 +38,53 @@ step s1_drop_slot { SELECT pg_drop_replication_slot('slot_creation_error'); } +step s1_init { + SELECT 'init' FROM pg_create_logical_replication_slot('slot_a', 'test_decoding'); +} + +step s1_get_changes { + SELECT data FROM pg_logical_slot_get_changes('slot_a', NULL, NULL, 'include-xids', '0'); +} + +step s1_drop { + SELECT pg_drop_replication_slot('slot_a'); +} + +step s1_view_temp_slot { + SELECT slot_name, slot_type, active FROM pg_replication_slots WHERE slot_name = 'tmp_slot_b' +} + session s2 setup { SET synchronous_commit=on; } step s2_init { SELECT 'init' FROM pg_create_logical_replication_slot('slot_creation_error', 'test_decoding'); } +step s2_init_temp { + DO $$ + BEGIN + PERFORM pg_create_logical_replication_slot('tmp_slot_b', 'test_decoding', true); + EXCEPTION WHEN query_canceled THEN + NULL; + END $$; +} + +step s2_get_changes { + SELECT data FROM pg_logical_slot_get_changes('tmp_slot_b', NULL, NULL, 'include-xids', '0'); +} + # The tests first start a transaction with an xid assigned in s1, then create # a slot in s2. The slot creation waits for s1's transaction to end. Instead # we cancel / terminate s2. permutation s1_b s1_xid s2_init s1_view_slot s1_cancel_s2(s2_init) s1_view_slot s1_c permutation s1_b s1_xid s2_init s1_c s1_view_slot s1_drop_slot # check slot creation still works + +# A temporary slot whose creation fails inside a subtransaction, with the error +# caught by a PL/pgSQL exception block, is dropped. Left behind, tmp_slot_b has +# restart_lsn and catalog_xmin set and confirmed_flush_lsn not set, and +# decoding from it loses data: slot_a returns s1's transaction with both +# inserts and tmp_slot_b returns it with only the second one. +permutation s1_init s1_b s1_insert1 s2_init_temp s1_cancel_s2(s2_init_temp) s1_insert2 s1_c s1_view_temp_slot s1_get_changes s2_get_changes s1_drop + permutation s1_b s1_xid s2_init s1_terminate_s2(s2_init) s1_c s1_view_slot # can't run tests after this, due to s2's connection failure diff --git a/contrib/test_decoding/sql/slot.sql b/contrib/test_decoding/sql/slot.sql index 021280ebe3e..a75d54d6b71 100644 --- a/contrib/test_decoding/sql/slot.sql +++ b/contrib/test_decoding/sql/slot.sql @@ -239,3 +239,17 @@ 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'); + +-- A temporary slot whose creation fails inside a subtransaction, with the +-- error caught by a PL/pgSQL exception block, is dropped, so the same name can +-- be used again. +DO $$ +BEGIN + PERFORM pg_create_logical_replication_slot('regress_subxact_temp_slot', 'no_such_plugin', true); +EXCEPTION WHEN OTHERS THEN + RAISE NOTICE 'caught SQLSTATE %', SQLSTATE; +END $$; +SELECT count(*) = 0 AS temp_slot_dropped + FROM pg_replication_slots WHERE slot_name = 'regress_subxact_temp_slot'; +SELECT 'init' FROM pg_create_logical_replication_slot('regress_subxact_temp_slot', 'test_decoding', true); +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 f7d943234a7..4a0db579d2a 100644 --- a/doc/src/sgml/func/func-admin.sgml +++ b/doc/src/sgml/func/func-admin.sgml @@ -1053,7 +1053,8 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset for use by the current session. Temporary slots are also dropped when an error is reported. An error caught inside a subtransaction, for example by a PL/pgSQL - exception block, does not drop them. This function corresponds + exception block, does not drop them, unless the error happened + while creating the slot. This function corresponds to the replication protocol command CREATE_REPLICATION_SLOT ... PHYSICAL. @@ -1095,7 +1096,8 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset for use by the current session. Temporary slots are also dropped when an error is reported. An error caught inside a subtransaction, for example by a PL/pgSQL - exception block, does not drop them. The optional fourth parameter, + exception block, does not drop them, unless the error happened + while creating the slot. 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/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index ae1a04b3454..4ac331f9e43 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -2374,7 +2374,7 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" or when the session has finished. An error raised and caught in a subtransaction, for example by a PL/pgSQL exception block, does not drop - them. + them, unless the error happened while creating the slot. diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml index efefc43e6fc..bb223d433bf 100644 --- a/doc/src/sgml/system-views.sgml +++ b/doc/src/sgml/system-views.sgml @@ -2897,7 +2897,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx the session has finished. An error raised and caught in a subtransaction, for example by a PL/pgSQL exception block, does not drop - them. + them, unless the error happened while creating the slot. diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index 45766541bed..124d5fa1db3 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -165,6 +165,13 @@ ReplicationSlot *MyReplicationSlot = NULL; */ static SubTransactionId MyReplicationSlotSubId = InvalidSubTransactionId; +/* + * True while MyReplicationSlot is being created, that is from + * ReplicationSlotCreate() until the creating code releases the slot. Used to + * drop a temporary slot whose creation fails in a subxact that aborts. + */ +static bool MyReplicationSlotCreating = false; + /* GUC variables */ int max_replication_slots = 10; /* the maximum number of replication * slots */ @@ -528,6 +535,7 @@ ReplicationSlotCreate(const char *name, bool db_specific, SpinLockRelease(&slot->mutex); MyReplicationSlot = slot; MyReplicationSlotSubId = GetCurrentSubTransactionId(); + MyReplicationSlotCreating = true; LWLockRelease(ReplicationSlotControlLock); @@ -859,13 +867,15 @@ ReplicationSlotRelease(void) pfree(slotname); } - /* The slot is no longer acquired in any subxact. */ + /* The slot is no longer acquired in any subxact, nor being created. */ MyReplicationSlotSubId = InvalidSubTransactionId; + MyReplicationSlotCreating = false; } /* * At subxact end, release the replication slot if the subtransaction - * where the slot was acquired is aborted. + * where the slot was acquired is aborted. A temporary slot whose creation + * fails in that subtransaction is dropped instead. */ void AtEOSubXact_ReplicationSlot(bool isCommit, SubTransactionId mySubid) @@ -883,6 +893,7 @@ AtEOSubXact_ReplicationSlot(bool isCommit, SubTransactionId mySubid) if (isCommit) { MyReplicationSlotSubId = InvalidSubTransactionId; + MyReplicationSlotCreating = false; return; } @@ -903,16 +914,28 @@ AtEOSubXact_ReplicationSlot(bool isCommit, SubTransactionId mySubid) * 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. + * A temporary slot whose creation failed is dropped, as the release does + * for an ephemeral slot. Left behind, it would hold WAL and the catalog + * xmin until the session ends, and the session could decode partial + * transactions from it, since its confirmed_flush was never set. Dropping + * a temporary slot fails softly on I/O errors, see + * ReplicationSlotDropPtr(), and PROC_IN_LOGICAL_DECODING is only set + * outside a transaction, so there is nothing to clear here. + * + * Otherwise 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(); + if (MyReplicationSlotCreating && + MyReplicationSlot->data.persistency == RS_TEMPORARY) + ReplicationSlotDropAcquired(SlotIsLogical(MyReplicationSlot)); + else + ReplicationSlotRelease(); } /* @@ -1105,9 +1128,10 @@ ReplicationSlotDropAcquired(bool try_disable) /* Can only disable logical decoding if slot is logical */ Assert(!try_disable || SlotIsLogical(slot)); - /* slot isn't acquired anymore */ + /* slot isn't acquired anymore, nor being created */ MyReplicationSlot = NULL; MyReplicationSlotSubId = InvalidSubTransactionId; + MyReplicationSlotCreating = false; ReplicationSlotDropPtr(slot); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index a2f72f9eb5b..f33553e2b62 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -4640,7 +4640,8 @@ PostgresMain(const char *dbname, const char *username) * so releasing here is fine. There also is a before_shmem_exit() * 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. + * and, unlike here, does not drop the session's temporary slots, + * except one whose creation failed. */ if (MyReplicationSlot != NULL) ReplicationSlotRelease(); -- 2.47.3