diff --git a/dbutils.c b/dbutils.c index 903837fa..31c9053b 100644 --- a/dbutils.c +++ b/dbutils.c @@ -1817,7 +1817,6 @@ _create_update_node_record(PGconn *conn, char *action, t_node_info *node_info) char upstream_node_id[MAXLEN]; char *upstream_node_id_ptr = NULL; - char slot_name[MAXLEN]; char *slot_name_ptr = NULL; int param_count = 10; @@ -1844,8 +1843,10 @@ _create_update_node_record(PGconn *conn, char *action, t_node_info *node_info) upstream_node_id_ptr = upstream_node_id; } - if (node_info->slot_name[0]) - maxlen_snprintf(slot_name, "%s", node_info->slot_name); + if (node_info->slot_name[0] != '\0') + { + slot_name_ptr = node_info->slot_name; + } param_values[0] = get_node_type_string(node_info->type); diff --git a/repmgr-action-primary.c b/repmgr-action-primary.c index 818d8f4c..d5695214 100644 --- a/repmgr-action-primary.c +++ b/repmgr-action-primary.c @@ -134,34 +134,12 @@ do_primary_register(void) exit(ERR_BAD_CONFIG); } } - else - { - node_info.node_id = config_file_options.node_id; - } - /* set type to "primary", active to "true" and unset upstream_node_id*/ + init_node_record(&node_info); + + /* set type to "primary" and unset upstream_node_id */ node_info.type = PRIMARY; node_info.upstream_node_id = NO_UPSTREAM_NODE; - node_info.active = true; - - /* update node record structure with settings from config file */ - strncpy(node_info.node_name, config_file_options.node_name, MAXLEN); - strncpy(node_info.conninfo, config_file_options.conninfo, MAXLEN); - - if (config_file_options.replication_user[0] != '\0') - { - strncpy(node_info.repluser, config_file_options.replication_user, NAMEDATALEN); - } - else - { - (void)get_conninfo_value(config_file_options.conninfo, "user", node_info.repluser); - } - - if (repmgr_slot_name_ptr != NULL) - strncpy(node_info.slot_name, repmgr_slot_name_ptr, MAXLEN); - - strncpy(node_info.location, config_file_options.location, MAXLEN); - node_info.priority = config_file_options.priority; initPQExpBuffer(&event_description); diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c index 06992334..bec09614 100644 --- a/repmgr-action-standby.c +++ b/repmgr-action-standby.c @@ -63,11 +63,12 @@ static void check_source_server_via_barman(void); static void check_primary_standby_version_match(PGconn *conn, PGconn *primary_conn); static void check_recovery_type(PGconn *conn); -static void initialise_direct_clone(void); -static void copy_configuration_files(void); -static int run_basebackup(void); -static int run_file_backup(void); +static void initialise_direct_clone(t_node_info *node_record); +static int run_basebackup(t_node_info *node_record); +static int run_file_backup(t_node_info *node_record); + +static void copy_configuration_files(void); static void drop_replication_slot_if_exists(PGconn *conn, int node_id, char *slot_name); @@ -92,6 +93,9 @@ do_standby_clone(void) PQExpBufferData event_details; int r; + /* dummy node record */ + t_node_info node_record = T_NODE_INFO_INITIALIZER; + /* * conninfo params for the actual upstream node (which might be different * to the node we're cloning from) to write to recovery.conf @@ -138,6 +142,8 @@ do_standby_clone(void) check_barman_config(); } + init_node_record(&node_record); + node_record.type = STANDBY; /* * Initialise list of conninfo parameters which will later be used @@ -274,7 +280,7 @@ do_standby_clone(void) if (mode != barman) { - initialise_direct_clone(); + initialise_direct_clone(&node_record); } switch (mode) @@ -300,11 +306,11 @@ do_standby_clone(void) if (mode == pg_basebackup) { - r = run_basebackup(); + r = run_basebackup(&node_record); } else { - r = run_file_backup(); + r = run_file_backup(&node_record); } @@ -312,9 +318,9 @@ do_standby_clone(void) if (r != SUCCESS) { /* If a replication slot was previously created, drop it */ - if (config_file_options.use_replication_slots) + if (config_file_options.use_replication_slots == true) { - drop_replication_slot(source_conn, repmgr_slot_name); + drop_replication_slot(source_conn, node_record.slot_name); } log_error(_("unable to take a base backup of the primary server")); @@ -341,7 +347,7 @@ do_standby_clone(void) /* Write the recovery.conf file */ - create_recovery_file(local_data_directory, &recovery_conninfo); + create_recovery_file(&node_record, &recovery_conninfo, local_data_directory); switch(mode) { @@ -355,26 +361,25 @@ do_standby_clone(void) } /* - * XXX It might be nice to provide an options to have repmgr start - * the PostgreSQL server automatically (e.g. with a custom pg_ctl - * command) + * TODO: It might be nice to provide an option to have repmgr start + * the PostgreSQL server automatically */ log_notice(_("you can now start your PostgreSQL server")); - if (config_file_options.service_start_command[0] != '\n') + if (config_file_options.service_start_command[0] != '\0') { - log_hint(_("for example : %s"), + log_hint(_("for example: %s"), config_file_options.service_start_command); } else if (local_data_directory_provided) { - log_hint(_("for example : pg_ctl -D %s start"), + log_hint(_("for example: pg_ctl -D %s start"), local_data_directory); } else { - log_hint(_("for example : /etc/init.d/postgresql start")); + log_hint(_("for example: /etc/init.d/postgresql start")); } /* @@ -559,10 +564,10 @@ do_standby_register(void) { if (!runtime_options.force) { - log_error(_("unable to connect to local node %i (\"%s\"):"), - config_file_options.node_id, - config_file_options.node_name); - log_detail(_("%s"), + log_error(_("unable to connect to local node \"%s\" (ID: %i)"), + config_file_options.node_name, + config_file_options.node_id); + log_detail("%s", PQerrorMessage(conn)); log_hint(_("to register a standby which is not running, provide primary connection parameters and use option -F/--force")); @@ -571,9 +576,9 @@ do_standby_register(void) if (!runtime_options.connection_param_provided) { - log_error(_("unable to connect to local node %i (\"%s\") and no primary connection parameters provided"), - config_file_options.node_id, - config_file_options.node_name); + log_error(_("unable to connect to local node \"%s\" (ID: %i) and no primary connection parameters provided"), + config_file_options.node_name, + config_file_options.node_id); exit(ERR_BAD_CONFIG); } } @@ -666,7 +671,6 @@ do_standby_register(void) * If an upstream node is defined, check if that node exists and is active * If it doesn't exist, and --force set, create a minimal inactive record */ - if (runtime_options.upstream_node_id != NO_UPSTREAM_NODE) { RecordStatus upstream_record_status; @@ -760,34 +764,13 @@ do_standby_register(void) } - /* populate node record structure */ - - node_record.node_id = config_file_options.node_id; + /* + * populate node record structure with current values (this will overwrite + * any existing values, which is what we want when updating the record + */ + init_node_record(&node_record); node_record.type = STANDBY; - node_record.upstream_node_id = runtime_options.upstream_node_id; - node_record.priority = config_file_options.priority; - node_record.active = true; - strncpy(node_record.location, config_file_options.location, MAXLEN); - - printf("XXX %s %s\n", node_record.location, config_file_options.location); - - strncpy(node_record.node_name, config_file_options.node_name, MAXLEN); - strncpy(node_record.conninfo, config_file_options.conninfo, MAXLEN); - - if (config_file_options.replication_user[0] != '\0') - { - /* Replication user explicitly provided */ - strncpy(node_record.repluser, config_file_options.replication_user, NAMEDATALEN); - } - else - { - (void)get_conninfo_value(config_file_options.conninfo, "user", node_record.repluser); - } - - - if (repmgr_slot_name_ptr != NULL) - strncpy(node_record.slot_name, repmgr_slot_name_ptr, MAXLEN); /* * node record exists - update it @@ -809,7 +792,6 @@ do_standby_register(void) if (record_created == false) { /* XXX add event description */ - create_event_notification(primary_conn, &config_file_options, config_file_options.node_id, @@ -818,6 +800,7 @@ do_standby_register(void) NULL); PQfinish(primary_conn); + primary_conn = NULL; if (PQstatus(conn) == CONNECTION_OK) PQfinish(conn); @@ -1333,8 +1316,26 @@ do_standby_follow(void) exit(ERR_BAD_CONFIG); } + /* - * If 9.4 or later, and replication slots in use, we'll need to create a + * Fetch our node record so we can write application_name, if set, + * and to get the upstream node ID, which we'll need to know if + * replication slots are in use and we want to delete the old slot. + */ + record_status = get_node_record(primary_conn, + config_file_options.node_id, + &local_node_record); + + if (record_status != RECORD_FOUND) + { + log_error(_("unable to retrieve record for node %i"), + config_file_options.node_id); + PQfinish(primary_conn); + exit(ERR_BAD_CONFIG); + } + + /* + * If replication slots are in use, we'll need to create a * slot on the new primary */ @@ -1344,16 +1345,17 @@ do_standby_follow(void) initPQExpBuffer(&event_details); - if (create_replication_slot(primary_conn, repmgr_slot_name, server_version_num, &event_details) == false) + if (create_replication_slot(primary_conn, local_node_record.slot_name, server_version_num, &event_details) == false) { log_error("%s", event_details.data); - create_event_notification(primary_conn, - &config_file_options, - config_file_options.node_id, - "standby_follow", - false, - event_details.data); + create_event_notification( + primary_conn, + &config_file_options, + config_file_options.node_id, + "standby_follow", + false, + event_details.data); PQfinish(primary_conn); exit(ERR_DB_QUERY); @@ -1377,22 +1379,7 @@ do_standby_follow(void) /* Set the replication user from the primary node record */ param_set(&recovery_conninfo, "user", primary_node_record.repluser); - /* - * Fetch our node record so we can write application_name, if set, - * and to get the upstream node ID, which we'll need to know if - * replication slots are in use and we want to delete the old slot. - */ - record_status = get_node_record(primary_conn, - config_file_options.node_id, - &local_node_record); - if (record_status != RECORD_FOUND) - { - /* this shouldn't happen, but if it does we'll plough on regardless */ - log_warning(_("unable to retrieve record for node %i"), - config_file_options.node_id); - } - else { t_conninfo_param_list local_node_conninfo; bool parse_success; @@ -1432,7 +1419,7 @@ do_standby_follow(void) log_info(_("changing node %i's primary to node %i"), config_file_options.node_id, primary_id); - if (!create_recovery_file(data_dir, &recovery_conninfo)) + if (!create_recovery_file(&local_node_record, &recovery_conninfo, data_dir)) { PQfinish(primary_conn); exit(ERR_BAD_CONFIG); @@ -2388,7 +2375,7 @@ check_source_server_via_barman() * - standby_clone */ static void -initialise_direct_clone(void) +initialise_direct_clone(t_node_info *node_record) { PGconn *superuser_conn = NULL; PGconn *privileged_conn = NULL; @@ -2516,16 +2503,17 @@ initialise_direct_clone(void) PQExpBufferData event_details; initPQExpBuffer(&event_details); - if (create_replication_slot(privileged_conn, repmgr_slot_name, server_version_num, &event_details) == false) + if (create_replication_slot(privileged_conn, node_record->slot_name, server_version_num, &event_details) == false) { log_error("%s", event_details.data); - create_event_notification(primary_conn, - &config_file_options, - config_file_options.node_id, - "standby_clone", - false, - event_details.data); + create_event_notification( + primary_conn, + &config_file_options, + config_file_options.node_id, + "standby_clone", + false, + event_details.data); PQfinish(source_conn); @@ -2538,7 +2526,7 @@ initialise_direct_clone(void) termPQExpBuffer(&event_details); log_notice(_("replication slot \"%s\" created on upstream node (node_id: %i)"), - repmgr_slot_name, + node_record->slot_name, upstream_node_id); } @@ -2550,7 +2538,7 @@ initialise_direct_clone(void) static int -run_basebackup(void) +run_basebackup(t_node_info *node_record) { char script[MAXLEN]; int r = SUCCESS; @@ -2680,7 +2668,7 @@ run_basebackup(void) if (slot_add == true) { - appendPQExpBuffer(¶ms, " -S %s", repmgr_slot_name_ptr); + appendPQExpBuffer(¶ms, " -S %s", node_record->slot_name); } } @@ -2708,7 +2696,7 @@ run_basebackup(void) static int -run_file_backup(void) +run_file_backup(t_node_info *node_record) { int r = SUCCESS, i; @@ -3266,14 +3254,13 @@ copy_configuration_files(void) if (host == NULL) host = runtime_options.host; - log_verbose(LOG_DEBUG, "fetching configuration files from host \"%s\"", host); - log_notice(_("copying external configuration files from upstream node")); + log_notice(_("copying external configuration files from upstream node, host \"%s\""), host); r = test_ssh_connection(host, runtime_options.remote_user); if (r != 0) { - log_error(_("remote host %s is not reachable via SSH - unable to copy external configuration files"), + log_error(_("remote host \"%s\" is not reachable via SSH - unable to copy external configuration files"), host); return; } diff --git a/repmgr-client-global.h b/repmgr-client-global.h index e3c0533c..d6bb2b3c 100644 --- a/repmgr-client-global.h +++ b/repmgr-client-global.h @@ -155,9 +155,6 @@ t_conninfo_param_list source_conninfo; extern bool config_file_required; extern char pg_bindir[MAXLEN]; -extern char repmgr_slot_name[MAXLEN]; -extern char *repmgr_slot_name_ptr; - extern t_node_info target_node_info; @@ -175,7 +172,7 @@ extern void print_error_list(ItemList *error_list, int log_level); extern char *make_pg_path(const char *file); -extern bool create_recovery_file(const char *data_dir, t_conninfo_param_list *recovery_conninfo); +extern bool create_recovery_file(t_node_info *node_record, t_conninfo_param_list *recovery_conninfo, const char *data_dir); extern void get_superuser_connection(PGconn **conn, PGconn **superuser_conn, PGconn **privileged_conn); @@ -187,5 +184,6 @@ extern void make_remote_repmgr_path(PQExpBufferData *outputbuf); extern void get_server_action(t_server_action action, char *script, char *data_dir); extern bool data_dir_required_for_action(t_server_action action); extern void get_node_data_directory(char *data_dir_buf); +extern void init_node_record(t_node_info *node_record); #endif /* _REPMGR_CLIENT_GLOBAL_H_ */ diff --git a/repmgr-client.c b/repmgr-client.c index 894a39ba..d8cf6df3 100644 --- a/repmgr-client.c +++ b/repmgr-client.c @@ -62,8 +62,6 @@ t_conninfo_param_list source_conninfo; bool config_file_required = true; char pg_bindir[MAXLEN] = ""; -char repmgr_slot_name[MAXLEN] = ""; -char *repmgr_slot_name_ptr = NULL; char path_buf[MAXLEN] = ""; @@ -949,20 +947,6 @@ main(int argc, char **argv) PQfinish(conn); } - /* - * Initialise slot name, if required (9.4 and later) - * - * NOTE: the slot name will be defined for each record, including - * the primary; the `slot_name` column in `repl_nodes` defines - * the name of the slot, but does not imply a slot has been created. - * The version check for 9.4 or later is done in check_upstream_config() - */ - if (config_file_options.use_replication_slots) - { - maxlen_snprintf(repmgr_slot_name, "repmgr_slot_%i", config_file_options.node_id); - repmgr_slot_name_ptr = repmgr_slot_name; - log_verbose(LOG_DEBUG, "slot name initialised as: %s", repmgr_slot_name); - } switch (action) { @@ -2359,10 +2343,10 @@ copy_remote_files(char *host, char *remote_user, char *remote_path, * Creates a recovery.conf file for a standby * * A database connection pointer is required for escaping primary_conninfo - * parameters. When cloning from Barman and --no-upstream-conne ) this might not be + * parameters. When cloning from Barman and --no-upstream-connection ) this might not be */ bool -create_recovery_file(const char *data_dir, t_conninfo_param_list *recovery_conninfo) +create_recovery_file(t_node_info *node_record, t_conninfo_param_list *recovery_conninfo, const char *data_dir) { FILE *recovery_file; char recovery_file_path[MAXPGPATH]; @@ -2443,7 +2427,7 @@ create_recovery_file(const char *data_dir, t_conninfo_param_list *recovery_conni if (config_file_options.use_replication_slots) { maxlen_snprintf(line, "primary_slot_name = %s\n", - repmgr_slot_name); + node_record->slot_name); if (write_recovery_file_line(recovery_file, recovery_file_path, line) == false) return false; @@ -2880,3 +2864,38 @@ get_node_data_directory(char *data_dir_buf) return; } + + +/* + * initialise a node record from the provided configuration + * parameters + */ +void +init_node_record(t_node_info *node_record) +{ + node_record->node_id = config_file_options.node_id; + node_record->upstream_node_id = runtime_options.upstream_node_id; + node_record->priority = config_file_options.priority; + node_record->active = true; + + strncpy(node_record->location, config_file_options.location, MAXLEN); + + strncpy(node_record->node_name, config_file_options.node_name, MAXLEN); + strncpy(node_record->conninfo, config_file_options.conninfo, MAXLEN); + + if (config_file_options.replication_user[0] != '\0') + { + /* Replication user explicitly provided */ + strncpy(node_record->repluser, config_file_options.replication_user, NAMEDATALEN); + } + else + { + /* use the "user" value from "conninfo" */ + (void)get_conninfo_value(config_file_options.conninfo, "user", node_record->repluser); + } + + if (config_file_options.use_replication_slots == true) + { + maxlen_snprintf(node_record->slot_name, "repmgr_slot_%i", config_file_options.node_id); + } +}