0001-Various-improvements.patch

text/plain

Filename: 0001-Various-improvements.patch
Type: text/plain
Part: 0
Message: Re: speed up a logical replica setup

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch 0001
Subject: Various improvements
File+
src/bin/pg_basebackup/Makefile 1 1
src/bin/pg_basebackup/nls.mk 1 0
src/bin/pg_basebackup/pg_createsubscriber.c 21 28
From ec8e6ed6c3325a6f9fde2d1632346e212ade9c9f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 18 Mar 2024 14:48:55 +0100
Subject: [PATCH] Various improvements

---
 src/bin/pg_basebackup/Makefile              |  2 +-
 src/bin/pg_basebackup/nls.mk                |  1 +
 src/bin/pg_basebackup/pg_createsubscriber.c | 49 +++++++++------------
 3 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile
index e9a920dbcda..26c53e473f5 100644
--- a/src/bin/pg_basebackup/Makefile
+++ b/src/bin/pg_basebackup/Makefile
@@ -49,7 +49,7 @@ all: pg_basebackup pg_createsubscriber pg_receivewal pg_recvlogical
 pg_basebackup: $(BBOBJS) $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
 	$(CC) $(CFLAGS) $(BBOBJS) $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
 
-pg_createsubscriber: $(WIN32RES) pg_createsubscriber.o | submake-libpq submake-libpgport submake-libpgfeutils
+pg_createsubscriber: pg_createsubscriber.o $(WIN32RES) | submake-libpq submake-libpgport submake-libpgfeutils
 	$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
 
 pg_receivewal: pg_receivewal.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
diff --git a/src/bin/pg_basebackup/nls.mk b/src/bin/pg_basebackup/nls.mk
index fc475003e8e..7870cea71ce 100644
--- a/src/bin/pg_basebackup/nls.mk
+++ b/src/bin/pg_basebackup/nls.mk
@@ -8,6 +8,7 @@ GETTEXT_FILES    = $(FRONTEND_COMMON_GETTEXT_FILES) \
                    bbstreamer_tar.c \
                    bbstreamer_zstd.c \
                    pg_basebackup.c \
+                   pg_createsubscriber.c \
                    pg_receivewal.c \
                    pg_recvlogical.c \
                    receivelog.c \
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index e24ed7ef506..91c3a2f0036 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -43,7 +43,7 @@ struct CreateSubscriberOptions
 	SimpleStringList sub_names; /* list of subscription names */
 	SimpleStringList replslot_names;	/* list of replication slot names */
 	int			recovery_timeout;	/* stop recovery after this time */
-}			CreateSubscriberOptions;
+};
 
 struct LogicalRepInfo
 {
@@ -57,7 +57,7 @@ struct LogicalRepInfo
 
 	bool		made_replslot;	/* replication slot was created */
 	bool		made_publication;	/* publication was created */
-}			LogicalRepInfo;
+};
 
 static void cleanup_objects_atexit(void);
 static void usage();
@@ -155,9 +155,9 @@ cleanup_objects_atexit(void)
 	 */
 	if (recovery_ended)
 	{
-		pg_log_warning("pg_createsubscriber failed after the end of recovery");
-		pg_log_warning_hint("The target server cannot be used as a physical replica anymore.");
-		pg_log_warning_hint("You must recreate the physical replica before continuing.");
+		pg_log_warning("failed after the end of recovery");
+		pg_log_warning_hint("The target server cannot be used as a physical replica anymore.  "
+							"You must recreate the physical replica before continuing.");
 	}
 
 	for (int i = 0; i < num_dbs; i++)
@@ -184,13 +184,13 @@ cleanup_objects_atexit(void)
 				 */
 				if (dbinfo[i].made_publication)
 				{
-					pg_log_warning("There might be a publication \"%s\" in database \"%s\" on primary",
+					pg_log_warning("publication \"%s\" in database \"%s\" on primary might be left behind",
 								   dbinfo[i].pubname, dbinfo[i].dbname);
 					pg_log_warning_hint("Consider dropping this publication before trying again.");
 				}
 				if (dbinfo[i].made_replslot)
 				{
-					pg_log_warning("There might be a replication slot \"%s\" in database \"%s\" on primary",
+					pg_log_warning("replication slot \"%s\" in database \"%s\" on primary might be left behind",
 								   dbinfo[i].replslotname, dbinfo[i].dbname);
 					pg_log_warning_hint("Drop this replication slot soon to avoid retention of WAL files.");
 				}
@@ -420,7 +420,7 @@ store_pub_sub_info(const struct CreateSubscriberOptions *opt, const char *pub_ba
 	SimpleStringListCell *replslotcell = NULL;
 	int			i = 0;
 
-	dbinfo = (struct LogicalRepInfo *) pg_malloc(num_dbs * sizeof(struct LogicalRepInfo));
+	dbinfo = pg_malloc_array(struct LogicalRepInfo, num_dbs);
 
 	if (num_pubs > 0)
 		pubcell = opt->pub_names.head;
@@ -611,7 +611,7 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt)
 
 	char	   *cmd_str;
 
-	pg_log_info("modifying system identifier from subscriber");
+	pg_log_info("modifying system identifier of subscriber");
 
 	cf = get_controlfile(subscriber_dir, &crc_ok);
 	if (!crc_ok)
@@ -965,7 +965,7 @@ check_subscriber(const struct LogicalRepInfo *dbinfo)
 	/* The target server must be a standby */
 	if (!server_is_in_recovery(conn))
 	{
-		pg_log_error("The target server must be a standby");
+		pg_log_error("target server must be a standby");
 		disconnect_database(conn, true);
 	}
 
@@ -1217,13 +1217,11 @@ create_logical_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo)
 {
 	PQExpBuffer str = createPQExpBuffer();
 	PGresult   *res = NULL;
-	char		slot_name[NAMEDATALEN];
+	const char *slot_name = dbinfo->replslotname;
 	char	   *lsn = NULL;
 
 	Assert(conn != NULL);
 
-	snprintf(slot_name, NAMEDATALEN, "%s", dbinfo->replslotname);
-
 	pg_log_info("creating the replication slot \"%s\" on database \"%s\"",
 				slot_name, dbinfo->dbname);
 
@@ -1451,8 +1449,7 @@ wait_for_end_recovery(const char *conninfo, const struct CreateSubscriberOptions
 		pg_fatal("server did not end recovery");
 
 	pg_log_info("target server reached the consistent state");
-	pg_log_info_hint("If pg_createsubscriber fails after this point, "
-					 "you must recreate the physical replica before continuing.");
+	pg_log_info_hint("If pg_createsubscriber fails after this point, you must recreate the physical replica before continuing.");
 }
 
 /*
@@ -1625,8 +1622,8 @@ set_replication_progress(PGconn *conn, const struct LogicalRepInfo *dbinfo, cons
 	PQExpBuffer str = createPQExpBuffer();
 	PGresult   *res;
 	Oid			suboid;
-	char		originname[NAMEDATALEN];
-	char		lsnstr[17 + 1]; /* MAXPG_LSNLEN = 17 */
+	char	   *originname;
+	char	   *lsnstr;
 
 	Assert(conn != NULL);
 
@@ -1653,13 +1650,12 @@ set_replication_progress(PGconn *conn, const struct LogicalRepInfo *dbinfo, cons
 	if (dry_run)
 	{
 		suboid = InvalidOid;
-		snprintf(lsnstr, sizeof(lsnstr), "%X/%X",
-				 LSN_FORMAT_ARGS((XLogRecPtr) InvalidXLogRecPtr));
+		lsnstr = psprintf("%X/%X", LSN_FORMAT_ARGS((XLogRecPtr) InvalidXLogRecPtr));
 	}
 	else
 	{
 		suboid = strtoul(PQgetvalue(res, 0, 0), NULL, 10);
-		snprintf(lsnstr, sizeof(lsnstr), "%s", lsn);
+		lsnstr = psprintf("%s", lsn);
 	}
 
 	PQclear(res);
@@ -1668,7 +1664,7 @@ set_replication_progress(PGconn *conn, const struct LogicalRepInfo *dbinfo, cons
 	 * The origin name is defined as pg_%u. %u is the subscription OID. See
 	 * ApplyWorkerMain().
 	 */
-	snprintf(originname, sizeof(originname), "pg_%u", suboid);
+	originname = psprintf("pg_%u", suboid);
 
 	pg_log_info("setting the replication progress (node name \"%s\" ; LSN %s) on database \"%s\"",
 				originname, lsnstr, dbinfo->dbname);
@@ -1692,6 +1688,8 @@ set_replication_progress(PGconn *conn, const struct LogicalRepInfo *dbinfo, cons
 		PQclear(res);
 	}
 
+	pg_free(originname);
+	pg_free(lsnstr);
 	destroyPQExpBuffer(str);
 }
 
@@ -1797,13 +1795,9 @@ main(int argc, char **argv)
 	opt.config_file = NULL;
 	opt.pub_conninfo_str = NULL;
 	opt.socket_dir = NULL;
-	opt.sub_port = palloc(16);
-	strcpy(opt.sub_port, DEFAULT_SUB_PORT);
+	opt.sub_port = DEFAULT_SUB_PORT;
 	opt.sub_username = NULL;
-	opt.database_names = (SimpleStringList)
-	{
-		NULL, NULL
-	};
+	opt.database_names = (SimpleStringList){0};
 	opt.recovery_timeout = 0;
 
 	/*
@@ -1847,7 +1841,6 @@ main(int argc, char **argv)
 				dry_run = true;
 				break;
 			case 'p':
-				pg_free(opt.sub_port);
 				opt.sub_port = pg_strdup(optarg);
 				break;
 			case 'P':

base-commit: 48018f1d8c12d42b53c2a855626ee1ceb7f4ca71
prerequisite-patch-id: e4c7223061ca1e12dfa84f4a5ccdc3a9ab0c268a
prerequisite-patch-id: 6cc7583799a18d1119c8ecbb400b4471c6873768
-- 
2.44.0