From ec8e6ed6c3325a6f9fde2d1632346e212ade9c9f Mon Sep 17 00:00:00 2001 From: Peter Eisentraut 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