From 4e06355b572ee4e094c371078d921e83c5d3796d Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Mon, 26 Jun 2017 09:00:31 +0900 Subject: [PATCH] Replace repmgr.conf item "upstream_node_id" with --upstream-node-id This is only relevant when cloning a standby and the node's upstream can change after failover/switchover etc., so no point keeping the original value around in the configuration file. --- config.c | 5 +---- config.h | 3 +-- doc/changes-in-repmgr4.md | 8 +++++++- repmgr-action-primary.c | 9 --------- repmgr-action-standby.c | 26 +++++++++++++------------- repmgr-client-global.h | 3 ++- repmgr-client.c | 4 ++++ repmgr-client.h | 2 ++ 8 files changed, 30 insertions(+), 30 deletions(-) diff --git a/config.c b/config.c index e5d49e4b..cf50e92d 100644 --- a/config.c +++ b/config.c @@ -205,7 +205,6 @@ _parse_config(t_configuration_options *options, ItemList *error_list, ItemList * /* node information */ options->node_id = UNKNOWN_NODE_ID; - options->upstream_node_id = NO_UPSTREAM_NODE; memset(options->node_name, 0, sizeof(options->node_name)); memset(options->conninfo, 0, sizeof(options->conninfo)); memset(options->pg_bindir, 0, sizeof(options->pg_bindir)); @@ -335,8 +334,6 @@ _parse_config(t_configuration_options *options, ItemList *error_list, ItemList * } else if (strcmp(name, "node_name") == 0) strncpy(options->node_name, value, MAXLEN); - else if (strcmp(name, "upstream_node_id") == 0) - options->upstream_node_id = repmgr_atoi(value, name, error_list, 1); else if (strcmp(name, "conninfo") == 0) strncpy(options->conninfo, value, MAXLEN); else if (strcmp(name, "replication_user") == 0) @@ -486,7 +483,7 @@ _parse_config(t_configuration_options *options, ItemList *error_list, ItemList * else if (strcmp(name, "upstream_node") == 0) { item_list_append(warning_list, - _("parameter \"upstream_node\" has been renamed to \"upstream_node_id\"")); + _("parameter \"upstream_node\" has been removed; use \"--upstream-node-id\" when cloning a standby")); known_parameter = false; } else if (strcmp(name, "loglevel") == 0) diff --git a/config.h b/config.h index 2b84ee89..e091d7b8 100644 --- a/config.h +++ b/config.h @@ -51,7 +51,6 @@ typedef struct { /* node information */ int node_id; - int upstream_node_id; char node_name[MAXLEN]; char conninfo[MAXLEN]; char replication_user[MAXLEN]; @@ -118,7 +117,7 @@ typedef struct #define T_CONFIGURATION_OPTIONS_INITIALIZER { \ /* node information */ \ - UNKNOWN_NODE_ID, NO_UPSTREAM_NODE, "", "", "", "", REPLICATION_TYPE_PHYSICAL, \ + UNKNOWN_NODE_ID, "", "", "", "", REPLICATION_TYPE_PHYSICAL, \ /* log settings */ \ "", "", "", DEFAULT_LOG_STATUS_INTERVAL, \ /* standby clone settings */ \ diff --git a/doc/changes-in-repmgr4.md b/doc/changes-in-repmgr4.md index b7690380..de9e59be 100644 --- a/doc/changes-in-repmgr4.md +++ b/doc/changes-in-repmgr4.md @@ -5,10 +5,16 @@ New command line options - `--dry-run`: repmgr will attempt to perform the action as far as possible without making any changes to the database +- `--upstream-node-id`: use to specify the upstream node the standby will + connect later stream from, when cloning a standby. This replaces the configuration + file parameter `upstream_node`, as the upstream node is set when the standby + is initially cloned, but can change over the lifetime of an installation (due + to failovers, switchovers etc.) so it's pointless/confusing keeping the original + value around in the config file. + Changed command line options ---------------------------- - ### repmgr - `--replication-user` is now passed when registering the master server (and diff --git a/repmgr-action-primary.c b/repmgr-action-primary.c index 4c26d4e4..671ce6ef 100644 --- a/repmgr-action-primary.c +++ b/repmgr-action-primary.c @@ -139,15 +139,6 @@ do_primary_register(void) node_info.node_id = config_file_options.node_id; } - /* if upstream_node_id set, warn that it will be ignored */ - if (config_file_options.upstream_node_id != NO_UPSTREAM_NODE) - { - log_warning(_("primary node %i is configured with \"upstream_node_id\" set to %i"), - node_info.node_id, - config_file_options.upstream_node_id); - log_detail(_("the value set for \"upstream_node_id\" will be ignored")); - } - /* set type to "primary", active to "true" and unset upstream_node_id*/ node_info.type = PRIMARY; node_info.upstream_node_id = NO_UPSTREAM_NODE; diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c index 3937adce..3cec3bac 100644 --- a/repmgr-action-standby.c +++ b/repmgr-action-standby.c @@ -680,12 +680,12 @@ do_standby_register(void) * If it doesn't exist, and --force set, create a minimal inactive record */ - if (config_file_options.upstream_node_id != NO_UPSTREAM_NODE) + if (runtime_options.upstream_node_id != NO_UPSTREAM_NODE) { RecordStatus upstream_record_status; upstream_record_status = get_node_record(primary_conn, - config_file_options.upstream_node_id, + runtime_options.upstream_node_id, &node_record); if (upstream_record_status != RECORD_FOUND) @@ -695,7 +695,7 @@ do_standby_register(void) if (!runtime_options.force) { log_error(_("no record found for upstream node %i"), - config_file_options.upstream_node_id); + runtime_options.upstream_node_id); /* footgun alert - only do this if you know what you're doing */ log_hint(_("use option -F/--force to create a dummy upstream record")); PQfinish(primary_conn); @@ -705,9 +705,9 @@ do_standby_register(void) } log_notice(_("creating placeholder record for upstream node %i"), - config_file_options.upstream_node_id); + runtime_options.upstream_node_id); - upstream_node_record.node_id = config_file_options.upstream_node_id; + upstream_node_record.node_id = runtime_options.upstream_node_id; upstream_node_record.type = STANDBY; upstream_node_record.upstream_node_id = NO_UPSTREAM_NODE; strncpy(upstream_node_record.conninfo, runtime_options.upstream_conninfo, MAXLEN); @@ -730,12 +730,12 @@ do_standby_register(void) if (record_created == false) { upstream_record_status = get_node_record(primary_conn, - config_file_options.upstream_node_id, + runtime_options.upstream_node_id, &node_record); if (upstream_record_status != RECORD_FOUND) { log_error(_("unable to create placeholder record for upstream node %i"), - config_file_options.upstream_node_id); + runtime_options.upstream_node_id); PQfinish(primary_conn); if (PQstatus(conn) == CONNECTION_OK) PQfinish(conn); @@ -743,7 +743,7 @@ do_standby_register(void) } log_info(_("a record for upstream node %i was already created"), - config_file_options.upstream_node_id); + runtime_options.upstream_node_id); } } @@ -755,7 +755,7 @@ do_standby_register(void) if (!runtime_options.force) { log_error(_("record for upstream node %i is marked as inactive"), - config_file_options.upstream_node_id); + runtime_options.upstream_node_id); log_hint(_("use option -F/--force to register a standby with an inactive upstream node")); PQfinish(primary_conn); if (PQstatus(conn) == CONNECTION_OK) @@ -768,7 +768,7 @@ do_standby_register(void) */ log_notice(_("registering node %i with inactive upstream node %i"), config_file_options.node_id, - config_file_options.upstream_node_id); + runtime_options.upstream_node_id); } } @@ -777,7 +777,7 @@ do_standby_register(void) node_record.node_id = config_file_options.node_id; node_record.type = STANDBY; - node_record.upstream_node_id = config_file_options.upstream_node_id; + node_record.upstream_node_id = runtime_options.upstream_node_id; node_record.priority = config_file_options.priority; node_record.active = true; @@ -1736,10 +1736,10 @@ check_source_server() /* * Attempt to find the upstream node record */ - if (config_file_options.upstream_node_id == NO_UPSTREAM_NODE) + if (runtime_options.upstream_node_id == NO_UPSTREAM_NODE) upstream_node_id = get_primary_node_id(source_conn); else - upstream_node_id = config_file_options.upstream_node_id; + upstream_node_id = runtime_options.upstream_node_id; record_status = get_node_record(source_conn, upstream_node_id, &node_record); diff --git a/repmgr-client-global.h b/repmgr-client-global.h index e2237c0f..8a5038b9 100644 --- a/repmgr-client-global.h +++ b/repmgr-client-global.h @@ -61,6 +61,7 @@ typedef struct char recovery_min_apply_delay[MAXLEN]; char replication_user[MAXLEN]; char upstream_conninfo[MAXLEN]; + int upstream_node_id; bool use_recovery_conninfo_password; char wal_keep_segments[MAXLEN]; bool without_barman; @@ -90,7 +91,7 @@ typedef struct /* node options */ \ UNKNOWN_NODE_ID, "", "", \ /* standby clone options */ \ - false, CONFIG_FILE_SAMEPATH, false, false, false, "", "", "", false, "", false, \ + false, CONFIG_FILE_SAMEPATH, false, false, false, "", "", "", NO_UPSTREAM_NODE, false, "", false, \ /* standby register options */ \ false, 0, \ /* event options */ \ diff --git a/repmgr-client.c b/repmgr-client.c index 459813aa..2a1ac901 100644 --- a/repmgr-client.c +++ b/repmgr-client.c @@ -354,6 +354,10 @@ main(int argc, char **argv) strncpy(runtime_options.upstream_conninfo, optarg, MAXLEN); break; + case OPT_UPSTREAM_NODE_ID: + //strncpy(runtime_options.upstream_conninfo, optarg, MAXLEN); + break; + case OPT_USE_RECOVERY_CONNINFO_PASSWORD: runtime_options.use_recovery_conninfo_password = true; break; diff --git a/repmgr-client.h b/repmgr-client.h index 868cc56d..8fa6e379 100644 --- a/repmgr-client.h +++ b/repmgr-client.h @@ -58,6 +58,7 @@ #define OPT_LIMIT 21 #define OPT_ALL 22 #define OPT_DRY_RUN 23 +#define OPT_UPSTREAM_NODE_ID 24 /* deprecated since 3.3 */ #define OPT_DATA_DIR 998 #define OPT_NO_CONNINFO_PASSWORD 999 @@ -103,6 +104,7 @@ static struct option long_options[] = {"recovery-min-apply-delay", required_argument, NULL, OPT_RECOVERY_MIN_APPLY_DELAY}, {"replication-user", required_argument, NULL, OPT_REPLICATION_USER}, {"upstream-conninfo", required_argument, NULL, OPT_UPSTREAM_CONNINFO}, + {"upstream-node-id", required_argument, NULL, OPT_UPSTREAM_NODE_ID}, {"use-recovery-conninfo-password", no_argument, NULL, OPT_USE_RECOVERY_CONNINFO_PASSWORD}, {"without-barman", no_argument, NULL, OPT_WITHOUT_BARMAN},