From 81378de0e494c9299b29d9c1573f10c24153b255 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Wed, 29 Jan 2025 11:34:55 -0800 Subject: [PATCH v2] pg_rewind: Add dbname to primary_conninfo when using --write-recovery-conf This commit enhances pg_rewind's --write-recovery-conf option to include the dbname in the generated primary_conninfo value when specified in the --source-server option. With this modification, the rewound server can connect to the primary server without manual configuration file modifications when sync_replication_slots is enabled. Reviewed-by: Hayato Kuroda, Peter Smith Discussion: https://postgr.es/m/CAD21AoAkW=Ht0k9dVoBTCcqLiiZ2MXhVr+d=j2T_EZMerGrLWQ@mail.gmail.com --- doc/src/sgml/ref/pg_rewind.sgml | 6 ++- src/bin/pg_basebackup/pg_basebackup.c | 2 +- src/bin/pg_basebackup/streamutil.c | 69 --------------------------- src/bin/pg_basebackup/streamutil.h | 2 - src/bin/pg_rewind/pg_rewind.c | 6 ++- src/bin/pg_rewind/t/RewindTest.pm | 5 ++ src/fe_utils/recovery_gen.c | 66 +++++++++++++++++++++++++ src/include/fe_utils/recovery_gen.h | 1 + 8 files changed, 81 insertions(+), 76 deletions(-) diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml index dc039d87566..5485033ed8c 100644 --- a/doc/src/sgml/ref/pg_rewind.sgml +++ b/doc/src/sgml/ref/pg_rewind.sgml @@ -186,8 +186,10 @@ PostgreSQL documentation Create standby.signal and append connection settings to postgresql.auto.conf in the output - directory. --source-server is mandatory with - this option. + directory. The dbname will be recorded only if the dbname was + specified explicitly in the connection string or + environment variable. --source-server is + mandatory with this option. diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index dc0c805137a..d4b4e334014 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -1818,7 +1818,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail, if (writerecoveryconf) recoveryconfcontents = GenerateRecoveryConfig(conn, replication_slot, - GetDbnameFromConnectionOptions()); + GetDbnameFromConnectionOptions(connection_string)); /* * Run IDENTIFY_SYSTEM so we can get the timeline diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c index 021ab61fcb0..8e605f43ffe 100644 --- a/src/bin/pg_basebackup/streamutil.c +++ b/src/bin/pg_basebackup/streamutil.c @@ -32,7 +32,6 @@ int WalSegSz; static bool RetrieveDataDirCreatePerm(PGconn *conn); -static char *FindDbnameInConnParams(PQconninfoOption *conn_opts); /* SHOW command for replication connection was introduced in version 10 */ #define MINIMUM_VERSION_FOR_SHOW_CMD 100000 @@ -269,74 +268,6 @@ GetConnection(void) return tmpconn; } -/* - * FindDbnameInConnParams - * - * This is a helper function for GetDbnameFromConnectionOptions(). Extract - * the value of dbname from PQconninfoOption parameters, if it's present. - * Returns a strdup'd result or NULL. - */ -static char * -FindDbnameInConnParams(PQconninfoOption *conn_opts) -{ - PQconninfoOption *conn_opt; - - for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++) - { - if (strcmp(conn_opt->keyword, "dbname") == 0 && - conn_opt->val != NULL && conn_opt->val[0] != '\0') - return pg_strdup(conn_opt->val); - } - return NULL; -} - -/* - * GetDbnameFromConnectionOptions - * - * This is a special purpose function to retrieve the dbname from either the - * connection_string specified by the user or from the environment variables. - * - * We follow GetConnection() to fetch the dbname from various connection - * options. - * - * Returns NULL, if dbname is not specified by the user in the above - * mentioned connection options. - */ -char * -GetDbnameFromConnectionOptions(void) -{ - PQconninfoOption *conn_opts; - char *err_msg = NULL; - char *dbname; - - /* First try to get the dbname from connection string. */ - if (connection_string) - { - conn_opts = PQconninfoParse(connection_string, &err_msg); - if (conn_opts == NULL) - pg_fatal("%s", err_msg); - - dbname = FindDbnameInConnParams(conn_opts); - - PQconninfoFree(conn_opts); - if (dbname) - return dbname; - } - - /* - * Next try to get the dbname from default values that are available from - * the environment. - */ - conn_opts = PQconndefaults(); - if (conn_opts == NULL) - pg_fatal("out of memory"); - - dbname = FindDbnameInConnParams(conn_opts); - - PQconninfoFree(conn_opts); - return dbname; -} - /* * From version 10, explicitly set wal segment size using SHOW wal_segment_size * since ControlFile is not accessible here. diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h index 472193e239f..f917c43517f 100644 --- a/src/bin/pg_basebackup/streamutil.h +++ b/src/bin/pg_basebackup/streamutil.h @@ -31,8 +31,6 @@ extern PGconn *conn; extern PGconn *GetConnection(void); -extern char *GetDbnameFromConnectionOptions(void); - /* Replication commands */ extern bool CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin, bool is_temporary, diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index cae81cd6cb1..fbd29d81068 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -451,7 +451,8 @@ main(int argc, char **argv) pg_log_info("no rewind required"); if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, - GenerateRecoveryConfig(conn, NULL, NULL)); + GenerateRecoveryConfig(conn, NULL, + GetDbnameFromConnectionOptions(connstr_source))); exit(0); } @@ -528,7 +529,8 @@ main(int argc, char **argv) /* Also update the standby configuration, if requested. */ if (writerecoveryconf && !dry_run) WriteRecoveryConfig(conn, datadir_target, - GenerateRecoveryConfig(conn, NULL, NULL)); + GenerateRecoveryConfig(conn, NULL, + GetDbnameFromConnectionOptions(connstr_source))); /* don't need the source connection anymore */ source->destroy(source); diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm index 6115ec21eb9..ec3b4a51995 100644 --- a/src/bin/pg_rewind/t/RewindTest.pm +++ b/src/bin/pg_rewind/t/RewindTest.pm @@ -279,6 +279,11 @@ sub run_pg_rewind ], 'pg_rewind remote'); + # Check that pg_rewind with dbname and --write-recovery-conf + # wrote the dbname in the generated primary_conninfo value. + like(slurp_file("$primary_pgdata/postgresql.auto.conf"), + qr/dbname=postgres/m, 'recovery conf file sets dbname'); + # Check that standby.signal is here as recovery configuration # was requested. ok( -e "$primary_pgdata/standby.signal", diff --git a/src/fe_utils/recovery_gen.c b/src/fe_utils/recovery_gen.c index 7c172f65a10..e9023584768 100644 --- a/src/fe_utils/recovery_gen.c +++ b/src/fe_utils/recovery_gen.c @@ -14,6 +14,7 @@ #include "fe_utils/string_utils.h" static char *escape_quotes(const char *src); +static char *FindDbnameInConnOpts(PQconninfoOption *conn_opts); /* * Write recovery configuration contents into a fresh PQExpBuffer, and @@ -168,3 +169,68 @@ escape_quotes(const char *src) pg_fatal("out of memory"); return result; } + +/* + * FindDbnameInConnOpts + * + * This is a helper function for GetDbnameFromConnectionOptions(). Extract + * the value of dbname from PQconninfoOption parameters, if it's present. + * Returns a strdup'd result or NULL. + */ +static char * +FindDbnameInConnOpts(PQconninfoOption *conn_opts) +{ + for (PQconninfoOption *conn_opt = conn_opts; + conn_opt->keyword != NULL; + conn_opt++) + { + if (strcmp(conn_opt->keyword, "dbname") == 0 && + conn_opt->val != NULL && conn_opt->val[0] != '\0') + return pg_strdup(conn_opt->val); + } + return NULL; +} + +/* + * GetDbnameFromConnectionOptions + * + * This is a special purpose function to retrieve the dbname from either the + * 'connstr' specified by the caller or from the environment variables. + * + * Returns NULL, if dbname is not specified by the user in the given + * connection options. + */ +char * +GetDbnameFromConnectionOptions(const char *connstr) +{ + PQconninfoOption *conn_opts; + char *err_msg = NULL; + char *dbname; + + /* First try to get the dbname from connection string. */ + if (connstr) + { + conn_opts = PQconninfoParse(connstr, &err_msg); + if (conn_opts == NULL) + pg_fatal("%s", err_msg); + + dbname = FindDbnameInConnOpts(conn_opts); + + PQconninfoFree(conn_opts); + if (dbname) + return dbname; + } + + /* + * Next try to get the dbname from default values that are available from + * the environment. + */ + conn_opts = PQconndefaults(); + if (conn_opts == NULL) + pg_fatal("out of memory"); + + dbname = FindDbnameInConnOpts(conn_opts); + + PQconninfoFree(conn_opts); + return dbname; +} diff --git a/src/include/fe_utils/recovery_gen.h b/src/include/fe_utils/recovery_gen.h index 6412ffdaffa..c13f2263bcd 100644 --- a/src/include/fe_utils/recovery_gen.h +++ b/src/include/fe_utils/recovery_gen.h @@ -25,5 +25,6 @@ extern PQExpBuffer GenerateRecoveryConfig(PGconn *pgconn, char *dbname); extern void WriteRecoveryConfig(PGconn *pgconn, const char *target_dir, PQExpBuffer contents); +extern char *GetDbnameFromConnectionOptions(const char *connstr); #endif /* RECOVERY_GEN_H */ -- 2.43.5