From 53cb05445854f186841052c8ab58798ae1058e51 Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler@eulerto.com>
Date: Wed, 6 Mar 2024 17:50:01 -0300
Subject: [PATCH v28 2/4] Use last replication slot position as replication
 start point

Instead of using a temporary replication slot, use the last
replication slot position (LSN).
---
 doc/src/sgml/ref/pg_createsubscriber.sgml   | 17 ++----
 src/bin/pg_basebackup/pg_createsubscriber.c | 60 ++++++++++-----------
 2 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml
index 8ca9de5119..a889cea386 100644
--- a/doc/src/sgml/ref/pg_createsubscriber.sgml
+++ b/doc/src/sgml/ref/pg_createsubscriber.sgml
@@ -350,19 +350,10 @@ PostgreSQL documentation
      <quote><literal>pg_createsubscriber_%u_%d</literal></quote> (parameters:
      database <parameter>oid</parameter>, PID <parameter>int</parameter>).
      These replication slots will be used by the subscriptions in a future step.
-    </para>
-   </step>
-
-   <step>
-    <para>
-     Create a temporary replication slot to get a consistent start location.
-     This replication slot has the following name pattern:
-     <quote><literal>pg_createsubscriber_%d_startpoint</literal></quote>
-     (parameter: PID <parameter>int</parameter>). The LSN returned by
-     <link linkend="pg-create-logical-replication-slot"><function>pg_create_logical_replication_slot()</function></link>
-     is used as a stopping point in the <xref linkend="guc-recovery-target-lsn"/>
-     parameter and by the subscriptions as a replication start point. It
-     guarantees that no transaction will be lost.
+     The last replication slot LSN is used as a stopping point in the
+     <xref linkend="guc-recovery-target-lsn"/> parameter and by the
+     subscriptions as a replication start point. It guarantees that no
+     transaction will be lost.
     </para>
    </step>
 
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index 56641f583c..2a0cd47a8d 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -73,11 +73,12 @@ static void modify_subscriber_sysid(const char *pg_resetwal_path,
 									struct CreateSubscriberOptions *opt);
 static bool server_is_in_recovery(PGconn *conn);
 static void check_publisher(struct LogicalRepInfo *dbinfo);
-static void setup_publisher(struct LogicalRepInfo *dbinfo);
+static char *setup_publisher(struct LogicalRepInfo *dbinfo);
 static void check_subscriber(struct LogicalRepInfo *dbinfo);
 static void setup_subscriber(struct LogicalRepInfo *dbinfo,
 							 const char *consistent_lsn);
-static char *setup_recovery(struct LogicalRepInfo *dbinfo, char *datadir);
+static void setup_recovery(struct LogicalRepInfo *dbinfo, char *datadir,
+						   char *lsn);
 static void drop_primary_replication_slot(struct LogicalRepInfo *dbinfo,
 										  char *slotname);
 static char *create_logical_replication_slot(PGconn *conn,
@@ -570,11 +571,15 @@ modify_subscriber_sysid(const char *pg_resetwal_path, struct CreateSubscriberOpt
 
 /*
  * Create the publications and replication slots in preparation for logical
- * replication.
+ * replication. Returns the LSN from latest replication slot. It will be the
+ * replication start point that is used to adjust the subscriptions (see
+ * set_replication_progress).
  */
-static void
+static char *
 setup_publisher(struct LogicalRepInfo *dbinfo)
 {
+	char	   *lsn = NULL;
+
 	for (int i = 0; i < num_dbs; i++)
 	{
 		PGconn	   *conn;
@@ -637,8 +642,10 @@ setup_publisher(struct LogicalRepInfo *dbinfo)
 		dbinfo[i].subname = pg_strdup(replslotname);
 
 		/* Create replication slot on publisher */
-		if (create_logical_replication_slot(conn, &dbinfo[i], false) != NULL ||
-			dry_run)
+		if (lsn)
+			pg_free(lsn);
+		lsn = create_logical_replication_slot(conn, &dbinfo[i], false);
+		if (lsn != NULL || dry_run)
 			pg_log_info("create replication slot \"%s\" on publisher",
 						replslotname);
 		else
@@ -646,6 +653,8 @@ setup_publisher(struct LogicalRepInfo *dbinfo)
 
 		disconnect_database(conn, false);
 	}
+
+	return lsn;
 }
 
 /*
@@ -1012,14 +1021,11 @@ setup_subscriber(struct LogicalRepInfo *dbinfo, const char *consistent_lsn)
 }
 
 /*
- * Get a replication start location and write the required recovery parameters
- * into the configuration file. Returns the replication start location that is
- * used to adjust the subscriptions (see set_replication_progress).
+ * Write the required recovery parameters.
  */
-static char *
-setup_recovery(struct LogicalRepInfo *dbinfo, char *datadir)
+static void
+setup_recovery(struct LogicalRepInfo *dbinfo, char *datadir, char *lsn)
 {
-	char	   *consistent_lsn;
 	PGconn	   *conn;
 	PQExpBuffer recoveryconfcontents;
 
@@ -1033,15 +1039,12 @@ setup_recovery(struct LogicalRepInfo *dbinfo, char *datadir)
 	/*
 	 * Write recovery parameters.
 	 *
-	 * Despite of the recovery parameters will be written to the subscriber,
-	 * use a publisher connection for the following recovery functions. The
-	 * connection is only used to check the current server version (physical
-	 * replica, same server version). The subscriber is not running yet. In
-	 * dry run mode, the recovery parameters *won't* be written. An invalid
-	 * LSN is used for printing purposes. Additional recovery parameters are
-	 * added here. It avoids unexpected behavior such as end of recovery as
-	 * soon as a consistent state is reached (recovery_target) and failure due
-	 * to multiple recovery targets (name, time, xid, LSN).
+	 * The subscriber is not running yet. In dry run mode, the recovery
+	 * parameters *won't* be written. An invalid LSN is used for printing
+	 * purposes. Additional recovery parameters are added here. It avoids
+	 * unexpected behavior such as end of recovery as soon as a consistent
+	 * state is reached (recovery_target) and failure due to multiple recovery
+	 * targets (name, time, xid, LSN).
 	 */
 	recoveryconfcontents = GenerateRecoveryConfig(conn, NULL);
 	appendPQExpBuffer(recoveryconfcontents, "recovery_target = ''\n");
@@ -1065,14 +1068,12 @@ setup_recovery(struct LogicalRepInfo *dbinfo, char *datadir)
 	else
 	{
 		appendPQExpBuffer(recoveryconfcontents, "recovery_target_lsn = '%s'\n",
-						  consistent_lsn);
+						  lsn);
 		WriteRecoveryConfig(conn, datadir, recoveryconfcontents);
 	}
 	disconnect_database(conn, false);
 
 	pg_log_debug("recovery parameters:\n%s", recoveryconfcontents->data);
-
-	return consistent_lsn;
 }
 
 /*
@@ -1947,13 +1948,10 @@ main(int argc, char **argv)
 	 * primary slot is in use. We could use an extra connection for it but it
 	 * doesn't seem worth.
 	 */
-	setup_publisher(dbinfo);
+	consistent_lsn = setup_publisher(dbinfo);
 
-	/*
-	 * Get the replication start point and write the required recovery
-	 * parameters into the configuration file.
-	 */
-	consistent_lsn = setup_recovery(dbinfo, opt.subscriber_dir);
+	/* Write the required recovery parameters */
+	setup_recovery(dbinfo, opt.subscriber_dir, consistent_lsn);
 
 	/*
 	 * Restart subscriber so the recovery parameters will take effect. Wait
@@ -1969,7 +1967,7 @@ main(int argc, char **argv)
 	/*
 	 * Create the subscription for each database on subscriber. It does not
 	 * enable it immediately because it needs to adjust the replication start
-	 * point to the LSN reported by setup_recovery().  It also cleans up
+	 * point to the LSN reported by setup_publisher().  It also cleans up
 	 * publications created by this tool and replication to the standby.
 	 */
 	setup_subscriber(dbinfo, consistent_lsn);
-- 
2.43.0

