Fix duplicate transaction replay during pg_createsubscriber.

Previously, the tool could replay the same transaction twice, once during
recovery, then again during replication after the subscriber was set up.

This occurred because the same recovery_target_lsn was used both to
finalize recovery and to start replication. If
recovery_target_inclusive = true, the transaction at that LSN would be
applied during recovery and then sent again by the publisher leading to
duplication.

To prevent this, we now set recovery_target_inclusive = false. This
ensures the transaction at recovery_target_lsn is not reapplied during
recovery, avoiding duplication when replication begins.

Bug #18897
Reported-by: Zane Duffield <duffieldzane@gmail.com>
Author: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Backpatch-through: 17, where it was introduced
Discussion: https://postgr.es/m/18897-d3db67535860dddb@postgresql.org

diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index 025b893a41e832460291f77eb5d02d889dd6a53f..3986882f04292b0af1771abcb0ebe7b8a80c25e0 100644 (file)
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -1250,8 +1250,17 @@ setup_recovery(const struct LogicalRepInfo *dbinfo, const char *datadir, const c
    appendPQExpBufferStr(recoveryconfcontents, "recovery_target = ''\n");
    appendPQExpBufferStr(recoveryconfcontents,
                         "recovery_target_timeline = 'latest'\n");
+
+   /*
+    * Set recovery_target_inclusive = false to avoid reapplying the
+    * transaction committed at 'lsn' after subscription is enabled. This is
+    * because the provided 'lsn' is also used as the replication start point
+    * for the subscription. So, the server can send the transaction committed
+    * at that 'lsn' after replication is started which can lead to applying
+    * the same transaction twice if we keep recovery_target_inclusive = true.
+    */
    appendPQExpBufferStr(recoveryconfcontents,
-                        "recovery_target_inclusive = true\n");
+                        "recovery_target_inclusive = false\n");
    appendPQExpBufferStr(recoveryconfcontents,
                         "recovery_target_action = promote\n");
    appendPQExpBufferStr(recoveryconfcontents, "recovery_target_name = ''\n");