Restrict copying of invalidated replication slots.

Previously, invalidated logical and physical replication slots could
be copied using the pg_copy_logical_replication_slot and
pg_copy_physical_replication_slot functions. Replication slots that
were invalidated for reasons other than WAL removal retained their
restart_lsn. This meant that a new slot copied from an invalidated
slot could have a restart_lsn pointing to a WAL segment that might
have already been removed.

This commit restricts the copying of invalidated replication slots.

Backpatch to v16, where slots could retain their restart_lsn when
invalidated for reasons other than WAL removal.

For v15 and earlier, this check is not required since slots can only
be invalidated due to WAL removal, and existing checks already handle
this issue.

Author: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CANhcyEU65aH0VYnLiu%3DOhNNxhnhNhwcXBeT-jvRe1OiJTo_Ayg%40mail.gmail.com
Backpatch-through: 16

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 2488e9ba998250a6e5e75b611f2957da40927941..9f531e23283e82acf46de5a2d72887dfb7f9bbbd 100644 (file)
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -29598,7 +29598,8 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
         The copied physical slot starts to reserve WAL from the same <acronym>LSN</acronym> as the
         source slot.
         <parameter>temporary</parameter> is optional. If <parameter>temporary</parameter>
-        is omitted, the same value as the source slot is used.
+        is omitted, the same value as the source slot is used. Copy of an
+        invalidated slot is not allowed.
        </para></entry>
       </row>
 
@@ -29623,7 +29624,8 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
         The <literal>failover</literal> option of the source logical slot
         is not copied and is set to <literal>false</literal> by default. This
         is to avoid the risk of being unable to continue logical replication
-        after failover to standby where the slot is being synchronized.
+        after failover to standby where the slot is being synchronized. Copy of
+        an invalidated slot is not allowed.
        </para></entry>
       </row>
 

diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 8a314b5ff3ba36c74fece7aa133def005c372fea..36cc2ed4e440f2a04dcd2181a5bf1cd00fa5827d 100644 (file)
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -684,6 +684,13 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                 errmsg("cannot copy a replication slot that doesn't reserve WAL")));
 
+   /* Cannot copy an invalidated replication slot */
+   if (first_slot_contents.data.invalidated != RS_INVAL_NONE)
+       ereport(ERROR,
+               errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+               errmsg("cannot copy invalidated replication slot \"%s\"",
+                      NameStr(*src_name)));
+
    /* Overwrite params from optional arguments */
    if (PG_NARGS() >= 3)
        temporary = PG_GETARG_BOOL(2);
@@ -785,6 +792,20 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
                            NameStr(*src_name)),
                     errhint("Retry when the source replication slot's confirmed_flush_lsn is valid.")));
 
+       /*
+        * Copying an invalid slot doesn't make sense. Note that the source
+        * slot can become invalid after we create the new slot and copy the
+        * data of source slot. This is possible because the operations in
+        * InvalidateObsoleteReplicationSlots() are not serialized with this
+        * function. Even though we can't detect such a case here, the copied
+        * slot will become invalid in the next checkpoint cycle.
+        */
+       if (second_slot_contents.data.invalidated != RS_INVAL_NONE)
+           ereport(ERROR,
+                   errmsg("cannot copy replication slot \"%s\"",
+                          NameStr(*src_name)),
+                   errdetail("The source replication slot was invalidated during the copy operation."));
+
        /* Install copied values again */
        SpinLockAcquire(&MyReplicationSlot->mutex);
        MyReplicationSlot->effective_xmin = copy_effective_xmin;

diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index c31cab06f1cbae6bbf58b427490b7e3989dd932c..ee066626af76cfdd9a9be0e9c1c2b1e69ae53208 100644 (file)
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -591,6 +591,15 @@ $handle =
 check_pg_recvlogical_stderr($handle,
    "can no longer access replication slot \"vacuum_full_activeslot\"");
 
+# Attempt to copy an invalidated logical replication slot
+($result, $stdout, $stderr) = $node_standby->psql(
+   'postgres',
+   qq[select pg_copy_logical_replication_slot('vacuum_full_inactiveslot', 'vacuum_full_inactiveslot_copy');],
+   replication => 'database');
+ok( $stderr =~
+     /ERROR:  cannot copy invalidated replication slot "vacuum_full_inactiveslot"/,
+   "invalidated slot cannot be copied");
+
 # Turn hot_standby_feedback back on
 change_hot_standby_feedback_and_wait_for_xmins(1, 1);