From 135fa2e1b97cc7e230574a06f2bba2f0066cb47d Mon Sep 17 00:00:00 2001 From: Gianni Ciolli Date: Thu, 18 Aug 2016 13:10:26 +0200 Subject: [PATCH 01/17] Fixing primary_conninfo generation (#232) After introducing Barman mode, it is no longer true that STANDBY CLONE can derive primary_conninfo from the connection to the master. Now we ask Barman how to connect to a valid cluster node, and then we fetch the conninfo for the current master from repmgr metadata. --- repmgr.c | 120 +++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 86 insertions(+), 34 deletions(-) diff --git a/repmgr.c b/repmgr.c index d6c97504..6a8bea8d 100644 --- a/repmgr.c +++ b/repmgr.c @@ -96,7 +96,7 @@ static int copy_remote_files(char *host, char *remote_user, char *remote_path, static int run_basebackup(const char *data_dir, int server_version); static void check_parameters_for_action(const int action); static bool create_schema(PGconn *conn); -static bool create_recovery_file(const char *data_dir, PGconn *primary_conn); +static bool create_recovery_file(const char *data_dir, PGconn *primary_conn, const char *barmans_conninfo); static void write_primary_conninfo(char *line, PGconn *primary_conn); static bool write_recovery_file_line(FILE *recovery_file, char *recovery_file_path, char *line); static void check_master_standby_version_match(PGconn *conn, PGconn *master_conn); @@ -106,6 +106,7 @@ static bool update_node_record_set_master(PGconn *conn, int this_node_id); static void tablespace_data_append(TablespaceDataList *list, const char *name, const char *oid, const char *location); static int get_tablespace_data(PGconn *upstream_conn, TablespaceDataList *list); static int get_tablespace_data_barman(char *, TablespaceDataList *); +static void get_barman_property(char *dst, char *name, char *local_repmgr_directory); static char *string_skip_prefix(const char *prefix, char *string); static char *string_remove_trailing_newlines(char *string); @@ -1686,6 +1687,36 @@ get_tablespace_data_barman return SUCCESS; } +void +get_barman_property(char *dst, char *name, char *local_repmgr_directory) +{ + PQExpBufferData command_output; + char buf[MAXLEN]; + char command[MAXLEN]; + char *p; + + initPQExpBuffer(&command_output); + + maxlen_snprintf(command, + "grep \"^\t%s:\" %s/show-server.txt", + name, local_repmgr_directory); + (void)local_command(command, &command_output); + + maxlen_snprintf(buf, "\t%s: ", name); + p = string_skip_prefix(buf, command_output.data); + if (p == NULL) + { + log_err("Unexpected output from Barman: %s\n", + command_output.data); + exit(ERR_INTERNAL); + } + + strncpy(dst, p, MAXLEN); + string_remove_trailing_newlines(dst); + + termPQExpBuffer(&command_output); +} + static void do_standby_clone(void) { @@ -2071,32 +2102,6 @@ do_standby_clone(void) exit(ERR_INTERNAL); } - /* - * Locate Barman's backup directory - */ - - maxlen_snprintf(command, "%s show-server %s | grep 'backup_directory'", - make_barman_ssh_command(), - options.cluster_name); - - initPQExpBuffer(&command_output); - (void)local_command( - command, - &command_output); - - p = string_skip_prefix("\tbackup_directory: ", command_output.data); - if (p == NULL) - { - log_err("Unexpected output from Barman: %s\n", - command_output.data); - exit(ERR_INTERNAL); - } - - strncpy(backup_directory, p, MAXLEN); - string_remove_trailing_newlines(backup_directory); - - termPQExpBuffer(&command_output); - /* * Create the local repmgr subdirectory */ @@ -2114,6 +2119,22 @@ do_standby_clone(void) goto stop_backup; } + /* + * Fetch server parameters from Barman + */ + + maxlen_snprintf(command, "%s show-server %s > %s/show-server.txt", + make_barman_ssh_command(), + options.cluster_name, + local_repmgr_directory); + (void)local_command(command, NULL); + + /* + * Locate Barman's backup directory + */ + + get_barman_property(backup_directory, "backup_directory", local_repmgr_directory); + /* * Read the list of backup files into a local file. In the * process: @@ -2791,11 +2812,21 @@ stop_backup: } /* Finally, write the recovery.conf file */ - create_recovery_file(local_data_directory, upstream_conn); - - /* In Barman mode, remove local_repmgr_directory */ if (mode == barman) + { + char conninfo_on_barman[MAXLEN]; + + get_barman_property(conninfo_on_barman, "conninfo", local_repmgr_directory); + + create_recovery_file(local_data_directory, upstream_conn, conninfo_on_barman); + + /* In Barman mode, remove local_repmgr_directory */ rmdir(local_repmgr_directory); + } + else + { + create_recovery_file(local_data_directory, upstream_conn, NULL); + } switch(mode) { @@ -3347,7 +3378,7 @@ do_standby_follow(void) log_info(_("changing standby's master\n")); /* write the recovery.conf file */ - if (!create_recovery_file(data_dir, master_conn)) + if (!create_recovery_file(data_dir, master_conn, NULL)) exit(ERR_BAD_CONFIG); /* Finally, restart the service */ @@ -5113,7 +5144,7 @@ do_help(void) * Creates a recovery file for a standby. */ static bool -create_recovery_file(const char *data_dir, PGconn *primary_conn) +create_recovery_file(const char *data_dir, PGconn *primary_conn, const char *conninfo_on_barman) { FILE *recovery_file; char recovery_file_path[MAXLEN]; @@ -5138,8 +5169,29 @@ create_recovery_file(const char *data_dir, PGconn *primary_conn) log_debug(_("recovery.conf: %s"), line); - /* primary_conninfo = '...' */ - write_primary_conninfo(line, primary_conn); + if (primary_conn == NULL) + { + char buf[MAXLEN]; + PQExpBufferData command_output; + + initPQExpBuffer(&command_output); + maxlen_snprintf(buf, + "ssh %s \"psql -Aqt \\\"%s\\\" -c \\\"" + " SELECT conninfo" + " FROM repmgr_%s.repl_nodes" + " WHERE type='master'" + " AND active" + "\\\"\"", options.barman_server, conninfo_on_barman, options.cluster_name); + (void)local_command(buf, &command_output); + maxlen_snprintf(buf, "%s", command_output.data); + string_remove_trailing_newlines(buf); + maxlen_snprintf(line, "primary_conninfo = '%s'\n", buf); + } + else + { + /* primary_conninfo = '...' */ + write_primary_conninfo(line, primary_conn); + } if (write_recovery_file_line(recovery_file, recovery_file_path, line) == false) return false; From c0eea90402d4951311ed418fd661bb4a4c89c6c5 Mon Sep 17 00:00:00 2001 From: Gianni Ciolli Date: Thu, 18 Aug 2016 13:29:20 +0200 Subject: [PATCH 02/17] Cascading replication support for Barman mode If upstream_node is specified, we point the standby to that node; otherwise we point it to the current primary node. --- repmgr.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/repmgr.c b/repmgr.c index 6a8bea8d..a77ad511 100644 --- a/repmgr.c +++ b/repmgr.c @@ -5172,16 +5172,27 @@ create_recovery_file(const char *data_dir, PGconn *primary_conn, const char *con if (primary_conn == NULL) { char buf[MAXLEN]; + char where_condition[MAXLEN]; PQExpBufferData command_output; + switch(options.upstream_node) + { + case NO_UPSTREAM_NODE: + maxlen_snprintf(where_condition, "type='master'"); + default: + maxlen_snprintf(where_condition, "id=%d", options.upstream_node); + } + initPQExpBuffer(&command_output); maxlen_snprintf(buf, "ssh %s \"psql -Aqt \\\"%s\\\" -c \\\"" " SELECT conninfo" " FROM repmgr_%s.repl_nodes" - " WHERE type='master'" + " WHERE %s" " AND active" - "\\\"\"", options.barman_server, conninfo_on_barman, options.cluster_name); + "\\\"\"", + options.barman_server, conninfo_on_barman, + options.cluster_name, where_condition); (void)local_command(buf, &command_output); maxlen_snprintf(buf, "%s", command_output.data); string_remove_trailing_newlines(buf); From 95de5ef976cdf5b165f4bde3534c79080bbb323b Mon Sep 17 00:00:00 2001 From: Gianni Ciolli Date: Thu, 18 Aug 2016 15:48:10 +0200 Subject: [PATCH 03/17] Bug fix --- repmgr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/repmgr.c b/repmgr.c index a77ad511..a360a233 100644 --- a/repmgr.c +++ b/repmgr.c @@ -5179,8 +5179,10 @@ create_recovery_file(const char *data_dir, PGconn *primary_conn, const char *con { case NO_UPSTREAM_NODE: maxlen_snprintf(where_condition, "type='master'"); + break; default: maxlen_snprintf(where_condition, "id=%d", options.upstream_node); + break; } initPQExpBuffer(&command_output); From 1778eeab9c08f7f0121874434d212a378aca12da Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Tue, 30 Aug 2016 12:35:36 +0900 Subject: [PATCH 04/17] Initial change for differentiating between host we're cloning from and the defined upstream --- repmgr.c | 245 ++++++++++++++++++++++++++++++++++--------------------- repmgr.h | 7 ++ 2 files changed, 159 insertions(+), 93 deletions(-) diff --git a/repmgr.c b/repmgr.c index a360a233..c0c0d71c 100644 --- a/repmgr.c +++ b/repmgr.c @@ -145,16 +145,20 @@ static bool copy_file(const char *old_filename, const char *new_filename); static bool read_backup_label(const char *local_data_directory, struct BackupLabel *out_backup_label); -static void param_set(const char *param, const char *value); +static void initialize_conninfo_params(t_conninfo_param_list *param_list); +static void param_set_new(t_conninfo_param_list *param_list, const char *param, const char *value); static void parse_pg_basebackup_options(const char *pg_basebackup_options, t_basebackup_options *backup_options); /* Global variables */ static PQconninfoOption *opts = NULL; -static int param_count = 0; -static char **param_keywords; -static char **param_values; +/* conninfo params for the node we're cloning from */ +t_conninfo_param_list source_conninfo; + +/* conninfo params for the actual upstream node (which might be different) */ +t_conninfo_param_list upstream_conninfo; + static bool config_file_required = true; @@ -234,9 +238,6 @@ main(int argc, char **argv) bool config_file_parsed = false; char *ptr = NULL; - PQconninfoOption *defs = NULL; - PQconninfoOption *def; - set_progname(argv[0]); /* Disallow running as root to prevent directory ownership problems */ @@ -252,64 +253,42 @@ main(int argc, char **argv) exit(1); } - param_count = 0; - defs = PQconndefaults(); - - /* Count maximum number of parameters */ - for (def = defs; def->keyword; def++) - param_count ++; - - /* Initialize our internal parameter list */ - param_keywords = pg_malloc0(sizeof(char *) * (param_count + 1)); - param_values = pg_malloc0(sizeof(char *) * (param_count + 1)); - - for (c = 0; c <= param_count; c++) - { - param_keywords[c] = NULL; - param_values[c] = NULL; - } + initialize_conninfo_params(&source_conninfo); /* * Pre-set any defaults, which can be overwritten if matching * command line parameters are provided */ - for (def = defs; def->keyword; def++) + for (c = 0; c < source_conninfo.size && source_conninfo.keywords[c]; c++) { - if (def->val != NULL && def->val[0] != '\0') + if (strcmp(source_conninfo.keywords[c], "host") == 0 && + (source_conninfo.values[c] != NULL)) { - param_set(def->keyword, def->val); + strncpy(runtime_options.host, source_conninfo.values[c], MAXLEN); } - - if (strcmp(def->keyword, "host") == 0 && - (def->val != NULL && def->val[0] != '\0')) + else if (strcmp(source_conninfo.keywords[c], "hostaddr") == 0 && + (source_conninfo.values[c] != NULL)) { - strncpy(runtime_options.host, def->val, MAXLEN); + strncpy(runtime_options.host, source_conninfo.values[c], MAXLEN); } - else if (strcmp(def->keyword, "hostaddr") == 0 && - (def->val != NULL && def->val[0] != '\0')) + else if (strcmp(source_conninfo.keywords[c], "port") == 0 && + (source_conninfo.values[c] != NULL)) { - strncpy(runtime_options.host, def->val, MAXLEN); + strncpy(runtime_options.masterport, source_conninfo.values[c], MAXLEN); } - else if (strcmp(def->keyword, "port") == 0 && - (def->val != NULL && def->val[0] != '\0')) + else if (strcmp(source_conninfo.keywords[c], "dbname") == 0 && + (source_conninfo.values[c] != NULL)) { - strncpy(runtime_options.masterport, def->val, MAXLEN); + strncpy(runtime_options.dbname, source_conninfo.values[c], MAXLEN); } - else if (strcmp(def->keyword, "dbname") == 0 && - (def->val != NULL && def->val[0] != '\0')) + else if (strcmp(source_conninfo.keywords[c], "user") == 0 && + (source_conninfo.values[c] != NULL)) { - strncpy(runtime_options.dbname, def->val, MAXLEN); - } - else if (strcmp(def->keyword, "user") == 0 && - (def->val != NULL && def->val[0] != '\0')) - { - strncpy(runtime_options.username, def->val, MAXLEN); + strncpy(runtime_options.username, source_conninfo.values[c], MAXLEN); } } - PQconninfoFree(defs); - /* set default user for -R/--remote-user */ { @@ -372,13 +351,13 @@ main(int argc, char **argv) break; case 'h': strncpy(runtime_options.host, optarg, MAXLEN); - param_set("host", optarg); + param_set_new(&source_conninfo, "host", optarg); connection_param_provided = true; host_param_provided = true; break; case 'p': repmgr_atoi(optarg, "-p/--port", &cli_errors, false); - param_set("port", optarg); + param_set_new(&source_conninfo, "port", optarg); strncpy(runtime_options.masterport, optarg, MAXLEN); @@ -386,7 +365,7 @@ main(int argc, char **argv) break; case 'U': strncpy(runtime_options.username, optarg, MAXLEN); - param_set("user", optarg); + param_set_new(&source_conninfo, "user", optarg); connection_param_provided = true; break; case 'S': @@ -585,7 +564,7 @@ main(int argc, char **argv) { if (opt->val != NULL && opt->val[0] != '\0') { - param_set(opt->keyword, opt->val); + param_set_new(&source_conninfo, opt->keyword, opt->val); } if (strcmp(opt->keyword, "host") == 0 && @@ -615,7 +594,7 @@ main(int argc, char **argv) } else { - param_set("dbname", runtime_options.dbname); + param_set_new(&source_conninfo, "dbname", runtime_options.dbname); } } @@ -736,7 +715,7 @@ main(int argc, char **argv) else { strncpy(runtime_options.host, argv[optind++], MAXLEN); - param_set("host", runtime_options.host); + param_set_new(&source_conninfo, "host", runtime_options.host); } } } @@ -1791,15 +1770,29 @@ do_standby_clone(void) runtime_options.dest_dir); } - if (mode != barman) + + param_set_new(&source_conninfo, "application_name", options.node_name); + + /* Attempt to connect to the upstream server to verify its configuration */ + log_info(_("connecting to upstream node\n")); + upstream_conn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, false); + + /* + * Unless in barman mode, exit with an error; + * establish_db_connection_by_params() will have already logged an error message + */ + if (mode != barman && PQstatus(upstream_conn) != CONNECTION_OK) { + PQfinish(upstream_conn); + exit(ERR_DB_CON); + } - param_set("application_name", options.node_name); - - /* Connect to check configuration */ - log_info(_("connecting to upstream node\n")); - upstream_conn = establish_db_connection_by_params((const char**)param_keywords, (const char**)param_values, true); - + /* + * If a connection was established, perform some sanity checks on the + * provided upstream connection + */ + if (PQstatus(upstream_conn) == CONNECTION_OK) + { /* Verify that upstream node is a supported server version */ log_verbose(LOG_INFO, _("connected to upstream node, checking its state\n")); server_version_num = check_server_version(upstream_conn, "master", true, NULL); @@ -1812,6 +1805,20 @@ do_standby_clone(void) log_info(_("Successfully connected to upstream node. Current installation size is %s\n"), cluster_size); + /* + * If --recovery-min-apply-delay was passed, check that + * we're connected to PostgreSQL 9.4 or later + */ + if (*runtime_options.recovery_min_apply_delay) + { + if (server_version_num < 90400) + { + log_err(_("PostgreSQL 9.4 or greater required for --recovery-min-apply-delay\n")); + PQfinish(upstream_conn); + exit(ERR_BAD_CONFIG); + } + } + /* * If the upstream node is a standby, try to connect to the primary too so we * can write an event record @@ -1828,22 +1835,29 @@ do_standby_clone(void) { primary_conn = upstream_conn; } + } - /* - * If --recovery-min-apply-delay was passed, check that - * we're connected to PostgreSQL 9.4 or later - */ + // now set up conninfo params for the actual upstream, as we may be cloning from a different node + // if upstream conn + // get upstream node rec directly + // else + // attempt to get upstream node rec via Barman - if (*runtime_options.recovery_min_apply_delay) - { - if (server_version_num < 90400) - { - log_err(_("PostgreSQL 9.4 or greater required for --recovery-min-apply-delay\n")); - PQfinish(upstream_conn); - exit(ERR_BAD_CONFIG); - } - } + // if no upstream node rec + // if not --force + // exit with error + // else + // copy upstream_conn params to recovery primary_conninfo params + // else + // parse returned conninfo to recovery primary_conninfo params + // + // finally, set application name + // !! make write_primary_conninfo() use recovery primary_conninfo params, + // !! don't pass the connection + + if (mode != barman) + { /* * Check that tablespaces named in any `tablespace_mapping` configuration * file parameters exist. @@ -2812,10 +2826,14 @@ stop_backup: } /* Finally, write the recovery.conf file */ - if (mode == barman) + if (mode == barman && upstream_conn == NULL) { char conninfo_on_barman[MAXLEN]; + /* + * We don't have an upstream connection - attempt to connect + * to the upstream via the barman server to fetch the upstream's conninfo + */ get_barman_property(conninfo_on_barman, "conninfo", local_repmgr_directory); create_recovery_file(local_data_directory, upstream_conn, conninfo_on_barman); @@ -3322,7 +3340,7 @@ do_standby_follow(void) /* primary server info explictly provided - attempt to connect to that */ else { - master_conn = establish_db_connection_by_params((const char**)param_keywords, (const char**)param_values, true); + master_conn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, true); master_id = get_master_node_id(master_conn, options.cluster_name); @@ -4465,11 +4483,11 @@ do_witness_create(void) get_conninfo_value(options.conninfo, "user", repmgr_user); get_conninfo_value(options.conninfo, "dbname", repmgr_db); - param_set("user", repmgr_user); - param_set("dbname", repmgr_db); + param_set_new(&source_conninfo, "user", repmgr_user); + param_set_new(&source_conninfo, "dbname", repmgr_db); /* We need to connect to check configuration and copy it */ - masterconn = establish_db_connection_by_params((const char**)param_keywords, (const char**)param_values, false); + masterconn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, false); if (PQstatus(masterconn) != CONNECTION_OK) { @@ -4805,15 +4823,15 @@ do_witness_register(PGconn *masterconn) get_conninfo_value(options.conninfo, "user", repmgr_user); get_conninfo_value(options.conninfo, "dbname", repmgr_db); - param_set("user", repmgr_user); - param_set("dbname", repmgr_db); + param_set_new(&source_conninfo, "user", repmgr_user); + param_set_new(&source_conninfo, "dbname", repmgr_db); /* masterconn will only be set when called from do_witness_create() */ if (PQstatus(masterconn) != CONNECTION_OK) { event_is_register = true; - masterconn = establish_db_connection_by_params((const char**)param_keywords, (const char**)param_values, false); + masterconn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, false); if (PQstatus(masterconn) != CONNECTION_OK) { @@ -6480,7 +6498,7 @@ do_check_upstream_config(void) /* We need to connect to check configuration and start a backup */ log_info(_("connecting to upstream server\n")); - conn = establish_db_connection_by_params((const char**)param_keywords, (const char**)param_values, true); + conn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, true); /* Verify that upstream server is a supported server version */ @@ -6763,8 +6781,48 @@ copy_file(const char *old_filename, const char *new_filename) return true; } + + + static void -param_set(const char *param, const char *value) +initialize_conninfo_params(t_conninfo_param_list *param_list) +{ + PQconninfoOption *defs = NULL; + PQconninfoOption *def; + int c; + + defs = PQconndefaults(); + param_list->size = 0; + + /* Count maximum number of parameters */ + for (def = defs; def->keyword; def++) + param_list->size ++; + + /* Initialize our internal parameter list */ + param_list->keywords = pg_malloc0(sizeof(char *) * (param_list->size + 1)); + param_list->values = pg_malloc0(sizeof(char *) * (param_list->size + 1)); + + for (c = 0; c <= param_list->size; c++) + { + param_list->keywords[c] = NULL; + param_list->values[c] = NULL; + } + + /* Pre-set any defaults */ + + for (def = defs; def->keyword; def++) + { + if (def->val != NULL && def->val[0] != '\0') + { + param_set_new(param_list, def->keyword, def->val); + } + } + +} + + +static void +param_set_new(t_conninfo_param_list *param_list, const char *param, const char *value) { int c; int value_len = strlen(value) + 1; @@ -6772,15 +6830,15 @@ param_set(const char *param, const char *value) /* * Scan array to see if the parameter is already set - if not, replace it */ - for (c = 0; c <= param_count && param_keywords[c] != NULL; c++) + for (c = 0; c <= param_list->size && param_list->keywords[c] != NULL; c++) { - if (strcmp(param_keywords[c], param) == 0) + if (strcmp(param_list->keywords[c], param) == 0) { - if (param_values[c] != NULL) - pfree(param_values[c]); + if (param_list->values[c] != NULL) + pfree(param_list->values[c]); - param_values[c] = pg_malloc0(value_len); - strncpy(param_values[c], value, value_len); + param_list->values[c] = pg_malloc0(value_len); + strncpy(param_list->values[c], value, value_len); return; } @@ -6789,14 +6847,14 @@ param_set(const char *param, const char *value) /* * Parameter not in array - add it and its associated value */ - if (c < param_count) + if (c < param_list->size) { int param_len = strlen(param) + 1; - param_keywords[c] = pg_malloc0(param_len); - param_values[c] = pg_malloc0(value_len); + param_list->keywords[c] = pg_malloc0(param_len); + param_list->values[c] = pg_malloc0(value_len); - strncpy(param_keywords[c], param, param_len); - strncpy(param_values[c], value, value_len); + strncpy(param_list->keywords[c], param, param_len); + strncpy(param_list->values[c], value, value_len); } /* @@ -6807,6 +6865,7 @@ param_set(const char *param, const char *value) } + static void parse_pg_basebackup_options(const char *pg_basebackup_options, t_basebackup_options *backup_options) { diff --git a/repmgr.h b/repmgr.h index 0bac2e90..f719951f 100644 --- a/repmgr.h +++ b/repmgr.h @@ -131,4 +131,11 @@ typedef struct #define T_BASEBACKUP_OPTIONS_INITIALIZER { "", "" } +typedef struct +{ + int size; + char **keywords; + char **values; +} t_conninfo_param_list; + #endif From afa5c1469bff2172c9fb5c56aaea27bd6fd3729e Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Tue, 30 Aug 2016 14:57:51 +0900 Subject: [PATCH 05/17] Actually differentiate between clone source node and defined upstream node --- repmgr.c | 369 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 241 insertions(+), 128 deletions(-) diff --git a/repmgr.c b/repmgr.c index c0c0d71c..e976bdc7 100644 --- a/repmgr.c +++ b/repmgr.c @@ -96,8 +96,8 @@ static int copy_remote_files(char *host, char *remote_user, char *remote_path, static int run_basebackup(const char *data_dir, int server_version); static void check_parameters_for_action(const int action); static bool create_schema(PGconn *conn); -static bool create_recovery_file(const char *data_dir, PGconn *primary_conn, const char *barmans_conninfo); -static void write_primary_conninfo(char *line, PGconn *primary_conn); +static bool create_recovery_file(const char *data_dir, t_conninfo_param_list *upstream_conninfo, const char *barmans_conninfo); +static void write_primary_conninfo(char *line, t_conninfo_param_list *param_list); static bool write_recovery_file_line(FILE *recovery_file, char *recovery_file_path, char *line); static void check_master_standby_version_match(PGconn *conn, PGconn *master_conn); static int check_server_version(PGconn *conn, char *server_type, bool exit_on_error, char *server_version_string); @@ -145,8 +145,8 @@ static bool copy_file(const char *old_filename, const char *new_filename); static bool read_backup_label(const char *local_data_directory, struct BackupLabel *out_backup_label); -static void initialize_conninfo_params(t_conninfo_param_list *param_list); -static void param_set_new(t_conninfo_param_list *param_list, const char *param, const char *value); +static void initialize_conninfo_params(t_conninfo_param_list *param_list, bool set_defaults); +static void param_set(t_conninfo_param_list *param_list, const char *param, const char *value); static void parse_pg_basebackup_options(const char *pg_basebackup_options, t_basebackup_options *backup_options); @@ -156,8 +156,7 @@ static PQconninfoOption *opts = NULL; /* conninfo params for the node we're cloning from */ t_conninfo_param_list source_conninfo; -/* conninfo params for the actual upstream node (which might be different) */ -t_conninfo_param_list upstream_conninfo; + static bool config_file_required = true; @@ -253,10 +252,10 @@ main(int argc, char **argv) exit(1); } - initialize_conninfo_params(&source_conninfo); + initialize_conninfo_params(&source_conninfo, true); /* - * Pre-set any defaults, which can be overwritten if matching + * Pre-set any defaults , which can be overwritten if matching * command line parameters are provided */ @@ -351,13 +350,13 @@ main(int argc, char **argv) break; case 'h': strncpy(runtime_options.host, optarg, MAXLEN); - param_set_new(&source_conninfo, "host", optarg); + param_set(&source_conninfo, "host", optarg); connection_param_provided = true; host_param_provided = true; break; case 'p': repmgr_atoi(optarg, "-p/--port", &cli_errors, false); - param_set_new(&source_conninfo, "port", optarg); + param_set(&source_conninfo, "port", optarg); strncpy(runtime_options.masterport, optarg, MAXLEN); @@ -365,7 +364,7 @@ main(int argc, char **argv) break; case 'U': strncpy(runtime_options.username, optarg, MAXLEN); - param_set_new(&source_conninfo, "user", optarg); + param_set(&source_conninfo, "user", optarg); connection_param_provided = true; break; case 'S': @@ -564,7 +563,7 @@ main(int argc, char **argv) { if (opt->val != NULL && opt->val[0] != '\0') { - param_set_new(&source_conninfo, opt->keyword, opt->val); + param_set(&source_conninfo, opt->keyword, opt->val); } if (strcmp(opt->keyword, "host") == 0 && @@ -594,7 +593,7 @@ main(int argc, char **argv) } else { - param_set_new(&source_conninfo, "dbname", runtime_options.dbname); + param_set(&source_conninfo, "dbname", runtime_options.dbname); } } @@ -715,7 +714,7 @@ main(int argc, char **argv) else { strncpy(runtime_options.host, argv[optind++], MAXLEN); - param_set_new(&source_conninfo, "host", runtime_options.host); + param_set(&source_conninfo, "host", runtime_options.host); } } } @@ -1700,9 +1699,19 @@ static void do_standby_clone(void) { PGconn *primary_conn = NULL; - PGconn *upstream_conn = NULL; + PGconn *source_conn = NULL; PGresult *res; + char upstream_conninfo_str[MAXLEN]; + bool upstream_record_found = false; + int upstream_node_id; + + /* + * conninfo params for the actual upstream node (which might be different + * to the node we're cloning from) + */ + t_conninfo_param_list upstream_conninfo; + enum { barman, rsync, @@ -1748,6 +1757,7 @@ do_standby_clone(void) PQExpBufferData event_details; + /* * Detecting the appropriate mode */ @@ -1770,20 +1780,20 @@ do_standby_clone(void) runtime_options.dest_dir); } - - param_set_new(&source_conninfo, "application_name", options.node_name); + // XXX set this to "repmgr ? + param_set(&source_conninfo, "application_name", options.node_name); /* Attempt to connect to the upstream server to verify its configuration */ log_info(_("connecting to upstream node\n")); - upstream_conn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, false); + source_conn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, false); /* * Unless in barman mode, exit with an error; * establish_db_connection_by_params() will have already logged an error message */ - if (mode != barman && PQstatus(upstream_conn) != CONNECTION_OK) + if (mode != barman && PQstatus(source_conn) != CONNECTION_OK) { - PQfinish(upstream_conn); + PQfinish(source_conn); exit(ERR_DB_CON); } @@ -1791,15 +1801,15 @@ do_standby_clone(void) * If a connection was established, perform some sanity checks on the * provided upstream connection */ - if (PQstatus(upstream_conn) == CONNECTION_OK) + if (PQstatus(source_conn) == CONNECTION_OK) { /* Verify that upstream node is a supported server version */ log_verbose(LOG_INFO, _("connected to upstream node, checking its state\n")); - server_version_num = check_server_version(upstream_conn, "master", true, NULL); + server_version_num = check_server_version(source_conn, "master", true, NULL); - check_upstream_config(upstream_conn, server_version_num, true); + check_upstream_config(source_conn, server_version_num, true); - if (get_cluster_size(upstream_conn, cluster_size) == false) + if (get_cluster_size(source_conn, cluster_size) == false) exit(ERR_DB_QUERY); log_info(_("Successfully connected to upstream node. Current installation size is %s\n"), @@ -1814,7 +1824,7 @@ do_standby_clone(void) if (server_version_num < 90400) { log_err(_("PostgreSQL 9.4 or greater required for --recovery-min-apply-delay\n")); - PQfinish(upstream_conn); + PQfinish(source_conn); exit(ERR_BAD_CONFIG); } } @@ -1823,38 +1833,164 @@ do_standby_clone(void) * If the upstream node is a standby, try to connect to the primary too so we * can write an event record */ - if (is_standby(upstream_conn)) + if (is_standby(source_conn)) { if (strlen(options.cluster_name)) { - primary_conn = get_master_connection(upstream_conn, options.cluster_name, + primary_conn = get_master_connection(source_conn, options.cluster_name, NULL, NULL); } } else { - primary_conn = upstream_conn; + primary_conn = source_conn; } } + // XXX + --no-upstream-connection // now set up conninfo params for the actual upstream, as we may be cloning from a different node // if upstream conn // get upstream node rec directly - // else - // attempt to get upstream node rec via Barman - // if no upstream node rec - // if not --force - // exit with error - // else - // copy upstream_conn params to recovery primary_conninfo params - // else - // parse returned conninfo to recovery primary_conninfo params - // - // finally, set application name + if (options.upstream_node == NO_UPSTREAM_NODE) + { + upstream_node_id = get_master_node_id(source_conn, options.cluster_name); + } + else + { + upstream_node_id = options.upstream_node; + } + + + + // XXX merge with previous if() + if (PQstatus(source_conn) == CONNECTION_OK) + { + t_node_info upstream_node_record = T_NODE_INFO_INITIALIZER; + int query_result; + + query_result = get_node_record(source_conn, options.cluster_name, upstream_node_id, &upstream_node_record); + + if (query_result) + { + upstream_record_found = true; + strncpy(upstream_conninfo_str, upstream_node_record.conninfo_str, MAXLEN); + } + } + else + { + // attempt to get upstream node rec via Barman: + // - get barman connstr + // - parse to list (no set defaults) + // - set dbname to the one provided by the user + /*char buf[MAXLEN]; + char where_condition[MAXLEN]; + PQExpBufferData command_output; + + switch(options.upstream_node) + { + case NO_UPSTREAM_NODE: + maxlen_snprintf(where_condition, "type='master'"); + break; + default: + maxlen_snprintf(where_condition, "id=%d", options.upstream_node); + break; + } + + initPQExpBuffer(&command_output); + maxlen_snprintf(buf, + "ssh %s \"psql -Aqt \\\"%s\\\" -c \\\"" + " SELECT conninfo" + " FROM repmgr_%s.repl_nodes" + " WHERE %s" + " AND active" + "\\\"\"", options.barman_server, conninfo_on_barman, + options.cluster_name, where_condition); + (void)local_command(buf, &command_output); + maxlen_snprintf(buf, "%s", command_output.data); + string_remove_trailing_newlines(buf); + maxlen_snprintf(line, "primary_conninfo = '%s'\n", buf);*/ + } + + printf("upstream found? %c\n", upstream_record_found == true ? 'y' : 'n'); + initialize_conninfo_params(&upstream_conninfo, true); + + /* + * Copy the source connection so that we have some default values, + * particularly stuff like passwords extracted from PGPASSFILE; + * these will be overridden from the upstream conninfo, if provided + */ + // XXX merge with previous if() + if (PQstatus(source_conn) == CONNECTION_OK) + { + PQconninfoOption *connOptions; + PQconninfoOption *option; + + connOptions = PQconninfo(source_conn); + for (option = connOptions; option && option->keyword; option++) + { + /* Ignore non-set or blank parameter values*/ + if((option->val == NULL) || + (option->val != NULL && option->val[0] == '\0')) + continue; + + param_set(&upstream_conninfo, option->keyword, option->val); + } + } + + /* + * If no upstream node record found, we'll abort with an error here, unless + * --force is used, in which case we'll use the conninfo parameters + * from source_conn, and if that's not available, using those provided + * on the command line (and assume the user knows what they're doing). + */ + if (upstream_record_found == false) + { + // if not --force + // exit with error + if (!runtime_options.force) + { + log_err(_("No record found for upstream node with id %i\n"), upstream_node_id); + PQfinish(source_conn); + exit(ERR_BAD_CONFIG); + } + // copy upstream_conn params to recovery primary_conninfo params + if (PQstatus(source_conn) != CONNECTION_OK) + { + // copy command line options ??? + } + + } + else + { + /* parse returned upstream conninfo string to recovery primary_conninfo params*/ + + PQconninfoOption *connOptions; + PQconninfoOption *option; + char *errmsg = NULL; + + connOptions = PQconninfoParse(upstream_conninfo_str, &errmsg); + + printf("upstr conn: %s\n", upstream_conninfo_str); + if (connOptions == NULL) + { + log_err(_("Unable to parse conninfo string \"%s\" for upstream node %i\n"), + upstream_conninfo_str, upstream_node_id); + PQfinish(source_conn); + exit(ERR_BAD_CONFIG); + } + + for (option = connOptions; option && option->keyword; option++) + { + /* Ignore non-set or blank parameter values*/ + if((option->val == NULL) || + (option->val != NULL && option->val[0] == '\0')) + continue; + + param_set(&upstream_conninfo, option->keyword, option->val); + } + } - // !! make write_primary_conninfo() use recovery primary_conninfo params, - // !! don't pass the connection if (mode != barman) { @@ -1877,7 +2013,7 @@ do_standby_clone(void) if (server_version_num < 90400 && !runtime_options.rsync_only) { log_err(_("in PostgreSQL 9.3, tablespace mapping can only be used in conjunction with --rsync-only\n")); - PQfinish(upstream_conn); + PQfinish(source_conn); exit(ERR_BAD_CONFIG); } @@ -1888,12 +2024,12 @@ do_standby_clone(void) " FROM pg_tablespace " " WHERE pg_tablespace_location(oid) = '%s'", cell->old_dir); - res = PQexec(upstream_conn, sqlquery); + res = PQexec(source_conn, sqlquery); if (PQresultStatus(res) != PGRES_TUPLES_OK) { - log_err(_("unable to execute tablespace query: %s\n"), PQerrorMessage(upstream_conn)); + log_err(_("unable to execute tablespace query: %s\n"), PQerrorMessage(source_conn)); PQclear(res); - PQfinish(upstream_conn); + PQfinish(source_conn); exit(ERR_BAD_CONFIG); } @@ -1901,7 +2037,7 @@ do_standby_clone(void) { log_err(_("no tablespace matching path '%s' found\n"), cell->old_dir); PQclear(res); - PQfinish(upstream_conn); + PQfinish(source_conn); exit(ERR_BAD_CONFIG); } } @@ -1930,13 +2066,13 @@ do_standby_clone(void) " ORDER BY 1 "); log_debug(_("standby clone: %s\n"), sqlquery); - res = PQexec(upstream_conn, sqlquery); + res = PQexec(source_conn, sqlquery); if (PQresultStatus(res) != PGRES_TUPLES_OK) { log_err(_("can't get info about data directory and configuration files: %s\n"), - PQerrorMessage(upstream_conn)); + PQerrorMessage(source_conn)); PQclear(res); - PQfinish(upstream_conn); + PQfinish(source_conn); exit(ERR_BAD_CONFIG); } @@ -1945,7 +2081,7 @@ do_standby_clone(void) { log_err("STANDBY CLONE should be run by a SUPERUSER\n"); PQclear(res); - PQfinish(upstream_conn); + PQfinish(source_conn); exit(ERR_BAD_CONFIG); } @@ -2059,9 +2195,9 @@ do_standby_clone(void) */ if (mode != barman && options.use_replication_slots) { - if (create_replication_slot(upstream_conn, repmgr_slot_name, server_version_num) == false) + if (create_replication_slot(source_conn, repmgr_slot_name, server_version_num) == false) { - PQfinish(upstream_conn); + PQfinish(source_conn); exit(ERR_DB_QUERY); } } @@ -2384,14 +2520,14 @@ do_standby_clone(void) * From 9.1 default is to wait for a sync standby to ack, avoid that by * turning off sync rep for this session */ - if (set_config_bool(upstream_conn, "synchronous_commit", false) == false) + if (set_config_bool(source_conn, "synchronous_commit", false) == false) { r = ERR_BAD_CONFIG; retval = ERR_BAD_CONFIG; goto stop_backup; } - if (start_backup(upstream_conn, first_wal_segment, runtime_options.fast_checkpoint) == false) + if (start_backup(source_conn, first_wal_segment, runtime_options.fast_checkpoint) == false) { r = ERR_BAD_BASEBACKUP; retval = ERR_BAD_BASEBACKUP; @@ -2440,7 +2576,7 @@ do_standby_clone(void) } /* Copy tablespaces and, if required, remap to a new location */ - retval = get_tablespace_data(upstream_conn, &tablespace_list); + retval = get_tablespace_data(source_conn, &tablespace_list); if(retval != SUCCESS) goto stop_backup; } @@ -2735,7 +2871,7 @@ stop_backup: if (mode == rsync && pg_start_backup_executed) { log_notice(_("notifying master about backup completion...\n")); - if (stop_backup(upstream_conn, last_wal_segment) == false) + if (stop_backup(source_conn, last_wal_segment) == false) { r = ERR_BAD_BASEBACKUP; retval = ERR_BAD_BASEBACKUP; @@ -2748,14 +2884,14 @@ stop_backup: /* If a replication slot was previously created, drop it */ if (options.use_replication_slots) { - drop_replication_slot(upstream_conn, repmgr_slot_name); + drop_replication_slot(source_conn, repmgr_slot_name); } log_err(_("unable to take a base backup of the master server\n")); log_warning(_("destination directory (%s) may need to be cleaned up manually\n"), local_data_directory); - PQfinish(upstream_conn); + PQfinish(source_conn); exit(retval); } @@ -2826,7 +2962,7 @@ stop_backup: } /* Finally, write the recovery.conf file */ - if (mode == barman && upstream_conn == NULL) + if (mode == barman && source_conn == NULL) { char conninfo_on_barman[MAXLEN]; @@ -2836,14 +2972,14 @@ stop_backup: */ get_barman_property(conninfo_on_barman, "conninfo", local_repmgr_directory); - create_recovery_file(local_data_directory, upstream_conn, conninfo_on_barman); + create_recovery_file(local_data_directory, &upstream_conninfo, conninfo_on_barman); /* In Barman mode, remove local_repmgr_directory */ rmdir(local_repmgr_directory); } else { - create_recovery_file(local_data_directory, upstream_conn, NULL); + create_recovery_file(local_data_directory, &upstream_conninfo, NULL); } switch(mode) @@ -2907,8 +3043,20 @@ stop_backup: runtime_options.masterport); appendPQExpBuffer(&event_details, - _("; backup method: %s"), - runtime_options.rsync_only ? "rsync" : "pg_basebackup"); + _("; backup method: ")); + + switch(mode) + { + case rsync: + appendPQExpBuffer(&event_details, "rsync"); + break; + case pg_basebackup: + appendPQExpBuffer(&event_details, "pg_basebackup"); + break; + case barman: + appendPQExpBuffer(&event_details, "barman"); + break; + } appendPQExpBuffer(&event_details, _("; --force: %s"), @@ -2922,7 +3070,7 @@ stop_backup: event_details.data); } - PQfinish(upstream_conn); + PQfinish(source_conn); exit(retval); } @@ -3396,8 +3544,8 @@ do_standby_follow(void) log_info(_("changing standby's master\n")); /* write the recovery.conf file */ - if (!create_recovery_file(data_dir, master_conn, NULL)) - exit(ERR_BAD_CONFIG); +// if (!create_recovery_file(data_dir, master_conn, NULL)) +// exit(ERR_BAD_CONFIG); /* Finally, restart the service */ if (*options.restart_command) @@ -4483,8 +4631,8 @@ do_witness_create(void) get_conninfo_value(options.conninfo, "user", repmgr_user); get_conninfo_value(options.conninfo, "dbname", repmgr_db); - param_set_new(&source_conninfo, "user", repmgr_user); - param_set_new(&source_conninfo, "dbname", repmgr_db); + param_set(&source_conninfo, "user", repmgr_user); + param_set(&source_conninfo, "dbname", repmgr_db); /* We need to connect to check configuration and copy it */ masterconn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, false); @@ -4823,8 +4971,8 @@ do_witness_register(PGconn *masterconn) get_conninfo_value(options.conninfo, "user", repmgr_user); get_conninfo_value(options.conninfo, "dbname", repmgr_db); - param_set_new(&source_conninfo, "user", repmgr_user); - param_set_new(&source_conninfo, "dbname", repmgr_db); + param_set(&source_conninfo, "user", repmgr_user); + param_set(&source_conninfo, "dbname", repmgr_db); /* masterconn will only be set when called from do_witness_create() */ if (PQstatus(masterconn) != CONNECTION_OK) @@ -5162,7 +5310,7 @@ do_help(void) * Creates a recovery file for a standby. */ static bool -create_recovery_file(const char *data_dir, PGconn *primary_conn, const char *conninfo_on_barman) +create_recovery_file(const char *data_dir, t_conninfo_param_list *upstream_conninfo, const char *conninfo_on_barman) { FILE *recovery_file; char recovery_file_path[MAXLEN]; @@ -5187,42 +5335,8 @@ create_recovery_file(const char *data_dir, PGconn *primary_conn, const char *con log_debug(_("recovery.conf: %s"), line); - if (primary_conn == NULL) - { - char buf[MAXLEN]; - char where_condition[MAXLEN]; - PQExpBufferData command_output; - - switch(options.upstream_node) - { - case NO_UPSTREAM_NODE: - maxlen_snprintf(where_condition, "type='master'"); - break; - default: - maxlen_snprintf(where_condition, "id=%d", options.upstream_node); - break; - } - - initPQExpBuffer(&command_output); - maxlen_snprintf(buf, - "ssh %s \"psql -Aqt \\\"%s\\\" -c \\\"" - " SELECT conninfo" - " FROM repmgr_%s.repl_nodes" - " WHERE %s" - " AND active" - "\\\"\"", - options.barman_server, conninfo_on_barman, - options.cluster_name, where_condition); - (void)local_command(buf, &command_output); - maxlen_snprintf(buf, "%s", command_output.data); - string_remove_trailing_newlines(buf); - maxlen_snprintf(line, "primary_conninfo = '%s'\n", buf); - } - else - { - /* primary_conninfo = '...' */ - write_primary_conninfo(line, primary_conn); - } + /* primary_conninfo = '...' */ + write_primary_conninfo(line, upstream_conninfo); if (write_recovery_file_line(recovery_file, recovery_file_path, line) == false) return false; @@ -6069,37 +6183,34 @@ create_schema(PGconn *conn) static void -write_primary_conninfo(char *line, PGconn *primary_conn) +write_primary_conninfo(char *line, t_conninfo_param_list *param_list) { - PQconninfoOption *connOptions; - PQconninfoOption *option; PQExpBufferData conninfo_buf; bool application_name_provided = false; - - connOptions = PQconninfo(primary_conn); + int c; initPQExpBuffer(&conninfo_buf); - for (option = connOptions; option && option->keyword; option++) + for (c = 0; c < param_list->size && param_list->keywords[c] != NULL; c++) { /* * Skip empty settings and ones which don't make any sense in * recovery.conf */ - if (strcmp(option->keyword, "dbname") == 0 || - strcmp(option->keyword, "replication") == 0 || - (option->val == NULL) || - (option->val != NULL && option->val[0] == '\0')) + if (strcmp(param_list->keywords[c], "dbname") == 0 || + strcmp(param_list->keywords[c], "replication") == 0 || + (param_list->values[c] == NULL) || + (param_list->values[c] != NULL && param_list->values[c][0] == '\0')) continue; if (conninfo_buf.len != 0) appendPQExpBufferChar(&conninfo_buf, ' '); - if (strcmp(option->keyword, "application_name") == 0) + if (strcmp(param_list->keywords[c], "application_name") == 0) application_name_provided = true; - /* XXX escape option->val */ - appendPQExpBuffer(&conninfo_buf, "%s=%s", option->keyword, option->val); + /* XXX escape option->values */ + appendPQExpBuffer(&conninfo_buf, "%s=%s", param_list->keywords[c], param_list->values[c]); } /* `application_name` not provided - default to repmgr node name */ @@ -6785,7 +6896,7 @@ copy_file(const char *old_filename, const char *new_filename) static void -initialize_conninfo_params(t_conninfo_param_list *param_list) +initialize_conninfo_params(t_conninfo_param_list *param_list, bool set_defaults) { PQconninfoOption *defs = NULL; PQconninfoOption *def; @@ -6808,21 +6919,23 @@ initialize_conninfo_params(t_conninfo_param_list *param_list) param_list->values[c] = NULL; } - /* Pre-set any defaults */ - - for (def = defs; def->keyword; def++) + if (set_defaults == true) { - if (def->val != NULL && def->val[0] != '\0') + /* Pre-set any defaults */ + + for (def = defs; def->keyword; def++) { - param_set_new(param_list, def->keyword, def->val); + if (def->val != NULL && def->val[0] != '\0') + { + param_set(param_list, def->keyword, def->val); + } } } - } static void -param_set_new(t_conninfo_param_list *param_list, const char *param, const char *value) +param_set(t_conninfo_param_list *param_list, const char *param, const char *value) { int c; int value_len = strlen(value) + 1; From 751469a08d49c30187f54a1bb32d9984e8099e91 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 07:34:58 +0900 Subject: [PATCH 06/17] Check SSH and create data dir early so we can init the barman stuff, which we want to verify the upstream conninfo --- repmgr.c | 146 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 76 insertions(+), 70 deletions(-) diff --git a/repmgr.c b/repmgr.c index e976bdc7..29708123 100644 --- a/repmgr.c +++ b/repmgr.c @@ -1768,6 +1768,21 @@ do_standby_clone(void) else mode = pg_basebackup; + /* + * In rsync mode, we need to check the SSH connection early + */ + if (mode == rsync) + { + r = test_ssh_connection(runtime_options.host, runtime_options.remote_user); + if (r != 0) + { + log_err(_("aborting, remote host %s is not reachable.\n"), + runtime_options.host); + exit(ERR_BAD_SSH); + } + } + + /* * If dest_dir (-D/--pgdata) was provided, this will become the new data * directory (otherwise repmgr will default to the same directory as on the @@ -1780,8 +1795,53 @@ do_standby_clone(void) runtime_options.dest_dir); } + // XXX merge above + + /* + * target directory (-D/--pgdata) provided - use that as new data directory + * (useful when executing backup on local machine only or creating the backup + * in a different local directory when backup source is a remote host) + */ + if (target_directory_provided) + { + strncpy(local_data_directory, runtime_options.dest_dir, MAXPGPATH); + strncpy(local_config_file, runtime_options.dest_dir, MAXPGPATH); + strncpy(local_hba_file, runtime_options.dest_dir, MAXPGPATH); + strncpy(local_ident_file, runtime_options.dest_dir, MAXPGPATH); + } + else if (mode == barman) + { + log_err(_("Barman mode requires a destination directory\n")); + log_hint(_("use -D/--data-dir to explicitly specify a data directory\n")); + exit(ERR_BAD_CONFIG); + } + /* + * Otherwise use the same data directory as on the remote host + */ + else + { + strncpy(local_data_directory, master_data_directory, MAXPGPATH); + strncpy(local_config_file, master_config_file, MAXPGPATH); + strncpy(local_hba_file, master_hba_file, MAXPGPATH); + strncpy(local_ident_file, master_ident_file, MAXPGPATH); + + log_notice(_("setting data directory to: %s\n"), local_data_directory); + log_hint(_("use -D/--data-dir to explicitly specify a data directory\n")); + } + + /* Check the local data directory can be used */ + + if (!create_pg_dir(local_data_directory, runtime_options.force)) + { + log_err(_("unable to use directory %s ...\n"), + local_data_directory); + log_hint(_("use -F/--force option to force this directory to be overwritten\n")); + exit(ERR_BAD_CONFIG); + } + + // XXX set this to "repmgr ? - param_set(&source_conninfo, "application_name", options.node_name); + //param_set(&source_conninfo, "application_name", options.node_name); /* Attempt to connect to the upstream server to verify its configuration */ log_info(_("connecting to upstream node\n")); @@ -1883,6 +1943,17 @@ do_standby_clone(void) // - get barman connstr // - parse to list (no set defaults) // - set dbname to the one provided by the user + + char conninfo_on_barman[MAXLEN]; + + /* + * We don't have an upstream connection - attempt to connect + * to the upstream via the barman server to fetch the upstream's conninfo + */ + get_barman_property(conninfo_on_barman, "conninfo", local_repmgr_directory); + + create_recovery_file(local_data_directory, &upstream_conninfo, conninfo_on_barman); + /*char buf[MAXLEN]; char where_condition[MAXLEN]; PQExpBufferData command_output; @@ -2126,65 +2197,6 @@ do_standby_clone(void) } - /* - * target directory (-D/--pgdata) provided - use that as new data directory - * (useful when executing backup on local machine only or creating the backup - * in a different local directory when backup source is a remote host) - */ - if (target_directory_provided) - { - strncpy(local_data_directory, runtime_options.dest_dir, MAXPGPATH); - strncpy(local_config_file, runtime_options.dest_dir, MAXPGPATH); - strncpy(local_hba_file, runtime_options.dest_dir, MAXPGPATH); - strncpy(local_ident_file, runtime_options.dest_dir, MAXPGPATH); - } - else if (mode == barman) - { - log_err(_("Barman mode requires a destination directory\n")); - log_hint(_("use -D/--data-dir to explicitly specify a data directory\n")); - exit(ERR_BAD_CONFIG); - } - /* - * Otherwise use the same data directory as on the remote host - */ - else - { - strncpy(local_data_directory, master_data_directory, MAXPGPATH); - strncpy(local_config_file, master_config_file, MAXPGPATH); - strncpy(local_hba_file, master_hba_file, MAXPGPATH); - strncpy(local_ident_file, master_ident_file, MAXPGPATH); - - log_notice(_("setting data directory to: %s\n"), local_data_directory); - log_hint(_("use -D/--data-dir to explicitly specify a data directory\n")); - } - - /* - * In rsync mode, we need to check the SSH connection early - */ - if (mode == rsync) - { - r = test_ssh_connection(runtime_options.host, runtime_options.remote_user); - if (r != 0) - { - log_err(_("aborting, remote host %s is not reachable.\n"), - runtime_options.host); - retval = ERR_BAD_SSH; - goto stop_backup; - } - } - - /* Check the local data directory can be used */ - - if (!create_pg_dir(local_data_directory, runtime_options.force)) - { - log_err(_("unable to use directory %s ...\n"), - local_data_directory); - log_hint(_("use -F/--force option to force this directory to be overwritten\n")); - r = ERR_BAD_CONFIG; - retval = ERR_BAD_CONFIG; - goto stop_backup; - } - /* * If replication slots requested, create appropriate slot on * the primary; this must be done before pg_start_backup() is @@ -2962,24 +2974,18 @@ stop_backup: } /* Finally, write the recovery.conf file */ + + create_recovery_file(local_data_directory, &upstream_conninfo, NULL); + if (mode == barman && source_conn == NULL) { - char conninfo_on_barman[MAXLEN]; - - /* - * We don't have an upstream connection - attempt to connect - * to the upstream via the barman server to fetch the upstream's conninfo - */ - get_barman_property(conninfo_on_barman, "conninfo", local_repmgr_directory); - - create_recovery_file(local_data_directory, &upstream_conninfo, conninfo_on_barman); /* In Barman mode, remove local_repmgr_directory */ rmdir(local_repmgr_directory); } else { - create_recovery_file(local_data_directory, &upstream_conninfo, NULL); + } switch(mode) From d7456d879ddc9c58099df0066750ede1de127b1f Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 09:19:03 +0900 Subject: [PATCH 07/17] Add option --no-upstream-connection and parse Barman conninfo string --- repmgr.c | 261 +++++++++++++++++++++++++++++++++++-------------------- repmgr.h | 4 +- 2 files changed, 171 insertions(+), 94 deletions(-) diff --git a/repmgr.c b/repmgr.c index 29708123..2080b413 100644 --- a/repmgr.c +++ b/repmgr.c @@ -147,7 +147,7 @@ static bool read_backup_label(const char *local_data_directory, struct BackupLab static void initialize_conninfo_params(t_conninfo_param_list *param_list, bool set_defaults); static void param_set(t_conninfo_param_list *param_list, const char *param, const char *value); - +static bool parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char *errmsg); static void parse_pg_basebackup_options(const char *pg_basebackup_options, t_basebackup_options *backup_options); /* Global variables */ @@ -223,6 +223,7 @@ main(int argc, char **argv) {"csv", no_argument, NULL, OPT_CSV}, {"node", required_argument, NULL, OPT_NODE}, {"without-barman", no_argument, NULL, OPT_WITHOUT_BARMAN}, + {"no-upstream-connection", no_argument, NULL, OPT_NO_UPSTREAM_CONNECTION}, {"version", no_argument, NULL, 'V'}, /* Following options deprecated */ {"local-port", required_argument, NULL, 'l'}, @@ -501,6 +502,9 @@ main(int argc, char **argv) case OPT_WITHOUT_BARMAN: runtime_options.without_barman = true; break; + case OPT_NO_UPSTREAM_CONNECTION: + runtime_options.no_upstream_connection = true; + break; /* deprecated options - output a warning */ case 'l': @@ -1706,6 +1710,9 @@ do_standby_clone(void) bool upstream_record_found = false; int upstream_node_id; + + char datadir_list_filename[MAXLEN]; + /* * conninfo params for the actual upstream node (which might be different * to the node we're cloning from) @@ -1839,24 +1846,84 @@ do_standby_clone(void) exit(ERR_BAD_CONFIG); } + /* Sanity check barman connection and installation, */ + if (mode == barman) + { + char command[MAXLEN]; + bool command_ok; + /* + * Check that there is at least one valid backup + */ + + log_info(_("Connecting to Barman server to verify backup for %s\n"), options.cluster_name); + + maxlen_snprintf(command, "%s show-backup %s latest > /dev/null", + make_barman_ssh_command(), + options.cluster_name); + command_ok = local_command(command, NULL); + if (command_ok == false) + { + log_err(_("No valid backup for server %s was found in the Barman catalogue\n"), + options.cluster_name); + log_hint(_("Refer to the Barman documentation for more information\n")); + + // ERR_BARMAN + exit(ERR_INTERNAL); + } + + /* + * Create the local repmgr subdirectory + */ + + maxlen_snprintf(local_repmgr_directory, "%s/repmgr", local_data_directory ); + maxlen_snprintf(datadir_list_filename, "%s/data.txt", local_repmgr_directory); + + if (!create_pg_dir(local_repmgr_directory, runtime_options.force)) + { + log_err(_("unable to use directory %s ...\n"), + local_repmgr_directory); + log_hint(_("use -F/--force option to force this directory to be overwritten\n")); + + exit(ERR_BAD_CONFIG); + } + + /* + * Fetch server parameters from Barman + */ + + maxlen_snprintf(command, "%s show-server %s > %s/show-server.txt", + make_barman_ssh_command(), + options.cluster_name, + local_repmgr_directory); + (void)local_command(command, NULL); + + // XXX check success + } + // XXX set this to "repmgr ? //param_set(&source_conninfo, "application_name", options.node_name); - /* Attempt to connect to the upstream server to verify its configuration */ - log_info(_("connecting to upstream node\n")); - source_conn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, false); - /* - * Unless in barman mode, exit with an error; - * establish_db_connection_by_params() will have already logged an error message - */ - if (mode != barman && PQstatus(source_conn) != CONNECTION_OK) + if (runtime_options.no_upstream_connection == false) { - PQfinish(source_conn); - exit(ERR_DB_CON); + /* Attempt to connect to the upstream server to verify its configuration */ + log_info(_("connecting to upstream node\n")); + + source_conn = establish_db_connection_by_params((const char**)source_conninfo.keywords, (const char**)source_conninfo.values, false); + + /* + * Unless in barman mode, exit with an error; + * establish_db_connection_by_params() will have already logged an error message + */ + if (mode != barman && PQstatus(source_conn) != CONNECTION_OK) + { + PQfinish(source_conn); + exit(ERR_DB_CON); + } } + // XXX insert into above if() /* * If a connection was established, perform some sanity checks on the * provided upstream connection @@ -1906,20 +1973,12 @@ do_standby_clone(void) primary_conn = source_conn; } } - // XXX + --no-upstream-connection // now set up conninfo params for the actual upstream, as we may be cloning from a different node // if upstream conn // get upstream node rec directly - if (options.upstream_node == NO_UPSTREAM_NODE) - { - upstream_node_id = get_master_node_id(source_conn, options.cluster_name); - } - else - { - upstream_node_id = options.upstream_node; - } + @@ -1929,6 +1988,11 @@ do_standby_clone(void) t_node_info upstream_node_record = T_NODE_INFO_INITIALIZER; int query_result; + if (options.upstream_node == NO_UPSTREAM_NODE) + upstream_node_id = get_master_node_id(source_conn, options.cluster_name); + else + upstream_node_id = options.upstream_node; + query_result = get_node_record(source_conn, options.cluster_name, upstream_node_id, &upstream_node_record); if (query_result) @@ -1937,21 +2001,19 @@ do_standby_clone(void) strncpy(upstream_conninfo_str, upstream_node_record.conninfo_str, MAXLEN); } } - else + else if (mode == barman) { - // attempt to get upstream node rec via Barman: - // - get barman connstr - // - parse to list (no set defaults) - // - set dbname to the one provided by the user - - char conninfo_on_barman[MAXLEN]; - /* - * We don't have an upstream connection - attempt to connect - * to the upstream via the barman server to fetch the upstream's conninfo + * Here we don't have a connection to the upstream node, and are executing + * in Barman mode - we can try and connect via the Barman server to extract + * the upstream node's conninfo string. + * + * To do this we need to extract Barman's conninfo string, replace the database + * name with the repmgr one (they could well be different) and remotely execute + * psql. */ - get_barman_property(conninfo_on_barman, "conninfo", local_repmgr_directory); +<<<<<<< HEAD create_recovery_file(local_data_directory, &upstream_conninfo, conninfo_on_barman); /*char buf[MAXLEN]; @@ -1968,6 +2030,49 @@ do_standby_clone(void) break; } +======= + char barman_conninfo_str[MAXLEN]; + t_conninfo_param_list barman_conninfo; + char *errmsg = NULL; + bool parse_success; + PQExpBufferData command_output; + + char buf[MAXLEN]; + + // XXX barman also returns "streaming_conninfo" - what's the difference? + get_barman_property(barman_conninfo_str, "conninfo", local_repmgr_directory); + + initialize_conninfo_params(&barman_conninfo, false); + + parse_success = parse_conninfo_string(barman_conninfo_str, &barman_conninfo, errmsg); + + if(parse_success == false) + { + log_err(_("Unable to parse barman conninfo string \"%s\":\n%s\n"), + barman_conninfo_str, errmsg); + // ERR_BARMAN + exit(ERR_BAD_CONFIG); + } + + /* Overwrite database name */ + param_set(&barman_conninfo, "database", runtime_options.dbname); + + { + int c; + printf("conninfo str: %s; size: %i\n", barman_conninfo_str, barman_conninfo.size); + for (c = 0; c < barman_conninfo.size && barman_conninfo.keywords[c] != NULL; c++) + { + printf("%s: %s\n", barman_conninfo.keywords[c], barman_conninfo.values[c]); + } + // + } + + + if (options.upstream_node == NO_UPSTREAM_NODE) + upstream_node_id = get_master_node_id(source_conn, options.cluster_name); + else + upstream_node_id = options.upstream_node; + initPQExpBuffer(&command_output); maxlen_snprintf(buf, "ssh %s \"psql -Aqt \\\"%s\\\" -c \\\"" @@ -1980,7 +2085,7 @@ do_standby_clone(void) (void)local_command(buf, &command_output); maxlen_snprintf(buf, "%s", command_output.data); string_remove_trailing_newlines(buf); - maxlen_snprintf(line, "primary_conninfo = '%s'\n", buf);*/ + maxlen_snprintf(line, "primary_conninfo = '%s'\n", buf); } printf("upstream found? %c\n", upstream_record_found == true ? 'y' : 'n'); @@ -2036,30 +2141,19 @@ do_standby_clone(void) { /* parse returned upstream conninfo string to recovery primary_conninfo params*/ - PQconninfoOption *connOptions; - PQconninfoOption *option; char *errmsg = NULL; - - connOptions = PQconninfoParse(upstream_conninfo_str, &errmsg); + bool parse_success; printf("upstr conn: %s\n", upstream_conninfo_str); - if (connOptions == NULL) + + parse_success = parse_conninfo_string(upstream_conninfo_str, &upstream_conninfo, errmsg); + if (parse_success == false) { - log_err(_("Unable to parse conninfo string \"%s\" for upstream node %i\n"), - upstream_conninfo_str, upstream_node_id); + log_err(_("Unable to parse conninfo string \"%s\" for upstream node %i:\n%s\n"), + upstream_conninfo_str, upstream_node_id, errmsg); PQfinish(source_conn); exit(ERR_BAD_CONFIG); } - - for (option = connOptions; option && option->keyword; option++) - { - /* Ignore non-set or blank parameter values*/ - if((option->val == NULL) || - (option->val != NULL && option->val[0] == '\0')) - continue; - - param_set(&upstream_conninfo, option->keyword, option->val); - } } @@ -2236,7 +2330,6 @@ do_standby_clone(void) char buf[MAXLEN]; char backup_directory[MAXLEN]; char backup_id[MAXLEN] = ""; - char datadir_list_filename[MAXLEN]; char *p, *q; PQExpBufferData command_output; TablespaceDataList tablespace_list = { NULL, NULL }; @@ -2247,49 +2340,6 @@ do_standby_clone(void) if (mode == barman) { - bool command_ok; - /* - * Check that there is at least one valid backup - */ - - maxlen_snprintf(command, "%s show-backup %s latest > /dev/null", - make_barman_ssh_command(), - options.cluster_name); - command_ok = local_command(command, NULL); - if (command_ok == false) - { - log_err(_("No valid backup for server %s was found in the Barman catalogue\n"), - options.barman_server); - log_hint(_("Refer to the Barman documentation for more information\n")); - exit(ERR_INTERNAL); - } - - /* - * Create the local repmgr subdirectory - */ - - maxlen_snprintf(local_repmgr_directory, "%s/repmgr", local_data_directory ); - maxlen_snprintf(datadir_list_filename, "%s/data.txt", local_repmgr_directory); - - if (!create_pg_dir(local_repmgr_directory, runtime_options.force)) - { - log_err(_("unable to use directory %s ...\n"), - local_repmgr_directory); - log_hint(_("use -F/--force option to force this directory to be overwritten\n")); - r = ERR_BAD_CONFIG; - retval = ERR_BAD_CONFIG; - goto stop_backup; - } - - /* - * Fetch server parameters from Barman - */ - - maxlen_snprintf(command, "%s show-server %s > %s/show-server.txt", - make_barman_ssh_command(), - options.cluster_name, - local_repmgr_directory); - (void)local_command(command, NULL); /* * Locate Barman's backup directory @@ -5231,6 +5281,8 @@ do_witness_unregister(void) } + // XXX + --no-upstream-connection + static void do_help(void) { @@ -6983,6 +7035,29 @@ param_set(t_conninfo_param_list *param_list, const char *param, const char *valu */ } +static bool +parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char *errmsg) +{ + PQconninfoOption *connOptions; + PQconninfoOption *option; + + connOptions = PQconninfoParse(conninfo_str, &errmsg); + + if (connOptions == NULL) + return false; + + for (option = connOptions; option && option->keyword; option++) + { + /* Ignore non-set or blank parameter values*/ + if((option->val == NULL) || + (option->val != NULL && option->val[0] == '\0')) + continue; + + param_set(param_list, option->keyword, option->val); + } + + return true; +} static void diff --git a/repmgr.h b/repmgr.h index f719951f..152b4fff 100644 --- a/repmgr.h +++ b/repmgr.h @@ -58,6 +58,7 @@ #define OPT_CSV 8 #define OPT_NODE 9 #define OPT_WITHOUT_BARMAN 10 +#define OPT_NO_UPSTREAM_CONNECTION 11 /* deprecated command line options */ #define OPT_INITDB_NO_PWPROMPT 999 @@ -85,6 +86,7 @@ typedef struct bool ignore_external_config_files; bool csv_mode; bool without_barman; + bool no_upstream_connection; char masterport[MAXLEN]; /* * configuration file parameters which can be overridden on the @@ -108,7 +110,7 @@ typedef struct char recovery_min_apply_delay[MAXLEN]; } t_runtime_options; -#define T_RUNTIME_OPTIONS_INITIALIZER { "", "", "", "", "", "", "", DEFAULT_WAL_KEEP_SEGMENTS, false, false, false, false, false, false, false, false, false, false, false, "", "", "", "", "fast", "", 0, 0, "", ""} +#define T_RUNTIME_OPTIONS_INITIALIZER { "", "", "", "", "", "", "", DEFAULT_WAL_KEEP_SEGMENTS, false, false, false, false, false, false, false, false, false, false, false, false, "", "", "", "", "fast", "", 0, 0, "", ""} struct BackupLabel { From f8fe801225546c318939f89d2e8d6a72f2729a85 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 09:53:18 +0900 Subject: [PATCH 08/17] use repmgr db connection with barman --- repmgr.c | 91 +++++++++++++++++++++++++++++++------------------------- 1 file changed, 50 insertions(+), 41 deletions(-) diff --git a/repmgr.c b/repmgr.c index 2080b413..01852fda 100644 --- a/repmgr.c +++ b/repmgr.c @@ -1708,7 +1708,7 @@ do_standby_clone(void) char upstream_conninfo_str[MAXLEN]; bool upstream_record_found = false; - int upstream_node_id; + int upstream_node_id = UNKNOWN_NODE_ID; char datadir_list_filename[MAXLEN]; @@ -2012,32 +2012,16 @@ do_standby_clone(void) * name with the repmgr one (they could well be different) and remotely execute * psql. */ - -<<<<<<< HEAD - create_recovery_file(local_data_directory, &upstream_conninfo, conninfo_on_barman); - - /*char buf[MAXLEN]; - char where_condition[MAXLEN]; - PQExpBufferData command_output; - - switch(options.upstream_node) - { - case NO_UPSTREAM_NODE: - maxlen_snprintf(where_condition, "type='master'"); - break; - default: - maxlen_snprintf(where_condition, "id=%d", options.upstream_node); - break; - } - -======= + char buf[MAXLEN]; char barman_conninfo_str[MAXLEN]; t_conninfo_param_list barman_conninfo; char *errmsg = NULL; bool parse_success; + char where_condition[MAXLEN]; PQExpBufferData command_output; + PQExpBufferData repmgr_conninfo_buf; - char buf[MAXLEN]; + int c; // XXX barman also returns "streaming_conninfo" - what's the difference? get_barman_property(barman_conninfo_str, "conninfo", local_repmgr_directory); @@ -2054,24 +2038,38 @@ do_standby_clone(void) exit(ERR_BAD_CONFIG); } - /* Overwrite database name */ - param_set(&barman_conninfo, "database", runtime_options.dbname); + /* Overwrite database name in the parsed parameter list */ + param_set(&barman_conninfo, "dbname", runtime_options.dbname); + /* Rebuild the Barman conninfo string */ + + initPQExpBuffer(&repmgr_conninfo_buf); + + for (c = 0; c < barman_conninfo.size && barman_conninfo.keywords[c] != NULL; c++) { - int c; - printf("conninfo str: %s; size: %i\n", barman_conninfo_str, barman_conninfo.size); - for (c = 0; c < barman_conninfo.size && barman_conninfo.keywords[c] != NULL; c++) - { - printf("%s: %s\n", barman_conninfo.keywords[c], barman_conninfo.values[c]); - } - // + printf("%s: %s\n", barman_conninfo.keywords[c], barman_conninfo.values[c]); + if (repmgr_conninfo_buf.len != 0) + appendPQExpBufferChar(&repmgr_conninfo_buf, ' '); + + /* XXX escape option->values */ + appendPQExpBuffer(&repmgr_conninfo_buf, "%s=%s", + barman_conninfo.keywords[c], + barman_conninfo.values[c]); } + log_verbose(LOG_DEBUG, + "repmgr database conninfo string on barman server: %s\n", + repmgr_conninfo_buf.data); - if (options.upstream_node == NO_UPSTREAM_NODE) - upstream_node_id = get_master_node_id(source_conn, options.cluster_name); - else - upstream_node_id = options.upstream_node; + switch(options.upstream_node) + { + case NO_UPSTREAM_NODE: + maxlen_snprintf(where_condition, "type='master'"); + break; + default: + maxlen_snprintf(where_condition, "id=%d", options.upstream_node); + break; + } initPQExpBuffer(&command_output); maxlen_snprintf(buf, @@ -2080,12 +2078,22 @@ do_standby_clone(void) " FROM repmgr_%s.repl_nodes" " WHERE %s" " AND active" - "\\\"\"", options.barman_server, conninfo_on_barman, + "\\\"\"", options.barman_server, repmgr_conninfo_buf.data, options.cluster_name, where_condition); + + termPQExpBuffer(&repmgr_conninfo_buf); + + // XXX check success (void)local_command(buf, &command_output); - maxlen_snprintf(buf, "%s", command_output.data); - string_remove_trailing_newlines(buf); - maxlen_snprintf(line, "primary_conninfo = '%s'\n", buf); + maxlen_snprintf(upstream_conninfo_str, "%s", command_output.data); + string_remove_trailing_newlines(upstream_conninfo_str); + + upstream_record_found = true; + log_verbose(LOG_DEBUG, + "upstream node conninfo string extracted via barman server: %s\n", + upstream_conninfo_str); + + termPQExpBuffer(&command_output); } printf("upstream found? %c\n", upstream_record_found == true ? 'y' : 'n'); @@ -2126,7 +2134,7 @@ do_standby_clone(void) // exit with error if (!runtime_options.force) { - log_err(_("No record found for upstream node with id %i\n"), upstream_node_id); + log_err(_("No record found for upstream node\n")); PQfinish(source_conn); exit(ERR_BAD_CONFIG); } @@ -2149,8 +2157,9 @@ do_standby_clone(void) parse_success = parse_conninfo_string(upstream_conninfo_str, &upstream_conninfo, errmsg); if (parse_success == false) { - log_err(_("Unable to parse conninfo string \"%s\" for upstream node %i:\n%s\n"), - upstream_conninfo_str, upstream_node_id, errmsg); + log_err(_("Unable to parse conninfo string \"%s\" for upstream node:\n%s\n"), + upstream_conninfo_str, errmsg); + PQfinish(source_conn); exit(ERR_BAD_CONFIG); } From 5c4b477d84f1955977655763d5da447c0c6b5f8e Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 11:03:44 +0900 Subject: [PATCH 09/17] Consolidate various checks in do_standby_clone() --- repmgr.c | 290 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 146 insertions(+), 144 deletions(-) diff --git a/repmgr.c b/repmgr.c index 01852fda..cb8d8e16 100644 --- a/repmgr.c +++ b/repmgr.c @@ -1792,36 +1792,35 @@ do_standby_clone(void) /* * If dest_dir (-D/--pgdata) was provided, this will become the new data - * directory (otherwise repmgr will default to the same directory as on the - * source host) + * directory (otherwise repmgr will default to using the same directory + * path as on the source host) */ + if (runtime_options.dest_dir[0]) { target_directory_provided = true; log_notice(_("destination directory '%s' provided\n"), runtime_options.dest_dir); } - - // XXX merge above + else if (mode == barman) + { + log_err(_("Barman mode requires a destination directory\n")); + log_hint(_("use -D/--data-dir to explicitly specify a data directory\n")); + exit(ERR_BAD_CONFIG); + } /* * target directory (-D/--pgdata) provided - use that as new data directory * (useful when executing backup on local machine only or creating the backup * in a different local directory when backup source is a remote host) */ - if (target_directory_provided) + if (target_directory_provided == true) { strncpy(local_data_directory, runtime_options.dest_dir, MAXPGPATH); strncpy(local_config_file, runtime_options.dest_dir, MAXPGPATH); strncpy(local_hba_file, runtime_options.dest_dir, MAXPGPATH); strncpy(local_ident_file, runtime_options.dest_dir, MAXPGPATH); } - else if (mode == barman) - { - log_err(_("Barman mode requires a destination directory\n")); - log_hint(_("use -D/--data-dir to explicitly specify a data directory\n")); - exit(ERR_BAD_CONFIG); - } /* * Otherwise use the same data directory as on the remote host */ @@ -1890,21 +1889,25 @@ do_standby_clone(void) /* * Fetch server parameters from Barman */ + log_info(_("Connecting to Barman server to fetch server parameters\n")); maxlen_snprintf(command, "%s show-server %s > %s/show-server.txt", make_barman_ssh_command(), options.cluster_name, local_repmgr_directory); - (void)local_command(command, NULL); - // XXX check success + command_ok = local_command(command, NULL); + + if (command_ok == false) + { + log_err(_("Unable to fetch server parameters from Barman server\n")); + + // ERR_BARMAN + exit(ERR_INTERNAL); + } } - - // XXX set this to "repmgr ? - //param_set(&source_conninfo, "application_name", options.node_name); - - + /* By default attempt to connect to the upstream server */ if (runtime_options.no_upstream_connection == false) { /* Attempt to connect to the upstream server to verify its configuration */ @@ -1916,92 +1919,107 @@ do_standby_clone(void) * Unless in barman mode, exit with an error; * establish_db_connection_by_params() will have already logged an error message */ - if (mode != barman && PQstatus(source_conn) != CONNECTION_OK) + if (PQstatus(source_conn) != CONNECTION_OK) { - PQfinish(source_conn); - exit(ERR_DB_CON); - } - } - - // XXX insert into above if() - /* - * If a connection was established, perform some sanity checks on the - * provided upstream connection - */ - if (PQstatus(source_conn) == CONNECTION_OK) - { - /* Verify that upstream node is a supported server version */ - log_verbose(LOG_INFO, _("connected to upstream node, checking its state\n")); - server_version_num = check_server_version(source_conn, "master", true, NULL); - - check_upstream_config(source_conn, server_version_num, true); - - if (get_cluster_size(source_conn, cluster_size) == false) - exit(ERR_DB_QUERY); - - log_info(_("Successfully connected to upstream node. Current installation size is %s\n"), - cluster_size); - - /* - * If --recovery-min-apply-delay was passed, check that - * we're connected to PostgreSQL 9.4 or later - */ - if (*runtime_options.recovery_min_apply_delay) - { - if (server_version_num < 90400) + if (mode != barman) { - log_err(_("PostgreSQL 9.4 or greater required for --recovery-min-apply-delay\n")); PQfinish(source_conn); - exit(ERR_BAD_CONFIG); + exit(ERR_DB_CON); } } - - /* - * If the upstream node is a standby, try to connect to the primary too so we - * can write an event record - */ - if (is_standby(source_conn)) + else { - if (strlen(options.cluster_name)) + /* + * If a connection was established, perform some sanity checks on the + * provided upstream connection + */ + PQconninfoOption *connOptions; + PQconninfoOption *option; + + t_node_info upstream_node_record = T_NODE_INFO_INITIALIZER; + int query_result; + + /* Verify that upstream node is a supported server version */ + log_verbose(LOG_INFO, _("connected to upstream node, checking its state\n")); + server_version_num = check_server_version(source_conn, "master", true, NULL); + + check_upstream_config(source_conn, server_version_num, true); + + if (get_cluster_size(source_conn, cluster_size) == false) + exit(ERR_DB_QUERY); + + log_info(_("Successfully connected to upstream node. Current installation size is %s\n"), + cluster_size); + + /* + * If --recovery-min-apply-delay was passed, check that + * we're connected to PostgreSQL 9.4 or later + */ + if (*runtime_options.recovery_min_apply_delay) { - primary_conn = get_master_connection(source_conn, options.cluster_name, - NULL, NULL); + if (server_version_num < 90400) + { + log_err(_("PostgreSQL 9.4 or greater required for --recovery-min-apply-delay\n")); + PQfinish(source_conn); + exit(ERR_BAD_CONFIG); + } + } + + /* + * If the upstream node is a standby, try to connect to the primary too so we + * can write an event record + */ + if (is_standby(source_conn)) + { + if (strlen(options.cluster_name)) + { + primary_conn = get_master_connection(source_conn, options.cluster_name, + NULL, NULL); + } + } + else + { + primary_conn = source_conn; + } + + /* + * Copy the source connection so that we have some default values, + * particularly stuff like passwords extracted from PGPASSFILE; + * these will be overridden from the upstream conninfo, if provided + */ + + connOptions = PQconninfo(source_conn); + for (option = connOptions; option && option->keyword; option++) + { + /* Ignore non-set or blank parameter values*/ + if((option->val == NULL) || + (option->val != NULL && option->val[0] == '\0')) + continue; + + param_set(&upstream_conninfo, option->keyword, option->val); + } + + // now set up conninfo params for the actual upstream, as we may be cloning from a different node + // if upstream conn + // get upstream node rec directly + + + if (options.upstream_node == NO_UPSTREAM_NODE) + upstream_node_id = get_master_node_id(source_conn, options.cluster_name); + else + upstream_node_id = options.upstream_node; + + query_result = get_node_record(source_conn, options.cluster_name, upstream_node_id, &upstream_node_record); + + if (query_result) + { + upstream_record_found = true; + strncpy(upstream_conninfo_str, upstream_node_record.conninfo_str, MAXLEN); } } - else - { - primary_conn = source_conn; - } } - // now set up conninfo params for the actual upstream, as we may be cloning from a different node - // if upstream conn - // get upstream node rec directly - - - - - - // XXX merge with previous if() - if (PQstatus(source_conn) == CONNECTION_OK) - { - t_node_info upstream_node_record = T_NODE_INFO_INITIALIZER; - int query_result; - - if (options.upstream_node == NO_UPSTREAM_NODE) - upstream_node_id = get_master_node_id(source_conn, options.cluster_name); - else - upstream_node_id = options.upstream_node; - - query_result = get_node_record(source_conn, options.cluster_name, upstream_node_id, &upstream_node_record); - - if (query_result) - { - upstream_record_found = true; - strncpy(upstream_conninfo_str, upstream_node_record.conninfo_str, MAXLEN); - } - } - else if (mode == barman) + if (mode == barman && PQstatus(source_conn) != CONNECTION_OK) { /* * Here we don't have a connection to the upstream node, and are executing @@ -2023,7 +2041,7 @@ do_standby_clone(void) int c; - // XXX barman also returns "streaming_conninfo" - what's the difference? + /* XXX barman also returns "streaming_conninfo" - what's the difference? */ get_barman_property(barman_conninfo_str, "conninfo", local_repmgr_directory); initialize_conninfo_params(&barman_conninfo, false); @@ -2047,7 +2065,6 @@ do_standby_clone(void) for (c = 0; c < barman_conninfo.size && barman_conninfo.keywords[c] != NULL; c++) { - printf("%s: %s\n", barman_conninfo.keywords[c], barman_conninfo.values[c]); if (repmgr_conninfo_buf.len != 0) appendPQExpBufferChar(&repmgr_conninfo_buf, ' '); @@ -2099,60 +2116,14 @@ do_standby_clone(void) printf("upstream found? %c\n", upstream_record_found == true ? 'y' : 'n'); initialize_conninfo_params(&upstream_conninfo, true); - /* - * Copy the source connection so that we have some default values, - * particularly stuff like passwords extracted from PGPASSFILE; - * these will be overridden from the upstream conninfo, if provided - */ - // XXX merge with previous if() - if (PQstatus(source_conn) == CONNECTION_OK) - { - PQconninfoOption *connOptions; - PQconninfoOption *option; - connOptions = PQconninfo(source_conn); - for (option = connOptions; option && option->keyword; option++) - { - /* Ignore non-set or blank parameter values*/ - if((option->val == NULL) || - (option->val != NULL && option->val[0] == '\0')) - continue; - - param_set(&upstream_conninfo, option->keyword, option->val); - } - } - - /* - * If no upstream node record found, we'll abort with an error here, unless - * --force is used, in which case we'll use the conninfo parameters - * from source_conn, and if that's not available, using those provided - * on the command line (and assume the user knows what they're doing). - */ - if (upstream_record_found == false) - { - // if not --force - // exit with error - if (!runtime_options.force) - { - log_err(_("No record found for upstream node\n")); - PQfinish(source_conn); - exit(ERR_BAD_CONFIG); - } - // copy upstream_conn params to recovery primary_conninfo params - if (PQstatus(source_conn) != CONNECTION_OK) - { - // copy command line options ??? - } - - } - else + if (upstream_record_found == true) { /* parse returned upstream conninfo string to recovery primary_conninfo params*/ - char *errmsg = NULL; bool parse_success; - printf("upstr conn: %s\n", upstream_conninfo_str); + log_verbose(LOG_DEBUG, "parsing upstream conninfo string \"%s\"\n", upstream_conninfo_str); parse_success = parse_conninfo_string(upstream_conninfo_str, &upstream_conninfo, errmsg); if (parse_success == false) @@ -2164,7 +2135,39 @@ do_standby_clone(void) exit(ERR_BAD_CONFIG); } } + else + { + /* + * If no upstream node record found, we'll abort with an error here, + * unless -F/--force is used, in which case we'll use the parameters + * provided on the command line (and assume the user knows what they're + * doing). + */ + if (!runtime_options.force) + { + log_err(_("No record found for upstream node\n")); + PQfinish(source_conn); + exit(ERR_BAD_CONFIG); + } + + if (strlen(runtime_options.host)) + { + param_set(&upstream_conninfo, "host", runtime_options.host); + } + if (strlen(runtime_options.masterport)) + { + param_set(&upstream_conninfo, "port", runtime_options.masterport); + } + if (strlen(runtime_options.dbname)) + { + param_set(&upstream_conninfo, "dbname", runtime_options.dbname); + } + if (strlen(runtime_options.username)) + { + param_set(&upstream_conninfo, "user", runtime_options.username); + } + } if (mode != barman) { @@ -2297,7 +2300,6 @@ do_standby_clone(void) } PQclear(res); - } /* From a8afa843ee78f8d1711ddd7f8b132133734928b1 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 11:35:23 +0900 Subject: [PATCH 10/17] Add parameter checks and help output for --no-upstream-connection --- repmgr.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/repmgr.c b/repmgr.c index cb8d8e16..946bb29d 100644 --- a/repmgr.c +++ b/repmgr.c @@ -1845,6 +1845,8 @@ do_standby_clone(void) exit(ERR_BAD_CONFIG); } + initialize_conninfo_params(&upstream_conninfo, true); + /* Sanity check barman connection and installation, */ if (mode == barman) { @@ -1863,7 +1865,7 @@ do_standby_clone(void) if (command_ok == false) { log_err(_("No valid backup for server %s was found in the Barman catalogue\n"), - options.cluster_name); + options.barman_server); log_hint(_("Refer to the Barman documentation for more information\n")); // ERR_BARMAN @@ -2114,7 +2116,6 @@ do_standby_clone(void) } printf("upstream found? %c\n", upstream_record_found == true ? 'y' : 'n'); - initialize_conninfo_params(&upstream_conninfo, true); if (upstream_record_found == true) @@ -3046,7 +3047,7 @@ stop_backup: } else { - + } switch(mode) @@ -5336,6 +5337,7 @@ do_help(void) printf(_("Command-specific configuration options:\n")); printf(_(" -c, --fast-checkpoint (standby clone) force fast checkpoint\n")); printf(_(" -r, --rsync-only (standby clone) use only rsync, not pg_basebackup\n")); + printf(_(" --no-upstream-connection (standby clone) when using Barman, do not connect to upstream node\n")); printf(_(" --without-barman (standby clone) do not use Barman even if configured\n")); printf(_(" --recovery-min-apply-delay=VALUE (standby clone, follow) set recovery_min_apply_delay\n" \ " in recovery.conf (PostgreSQL 9.4 and later)\n")); @@ -5855,6 +5857,13 @@ check_parameters_for_action(const int action) { item_list_append(&cli_warnings, _("-c/--fast-checkpoint has no effect when using -r/--rsync-only")); } + + if (runtime_options.no_upstream_connection == true && + (strcmp(options.barman_server, "") == 0 || runtime_options.without_barman == true)) + { + item_list_append(&cli_warnings, _("--no-upstream-connection only effective in Barman mode")); + } + config_file_required = false; break; case STANDBY_SWITCHOVER: @@ -5927,6 +5936,11 @@ check_parameters_for_action(const int action) { item_list_append(&cli_warnings, _("-w/--wal-keep-segments can only be used when executing STANDBY CLONE")); } + + if (runtime_options.no_upstream_connection == true) + { + item_list_append(&cli_warnings, _("--no-upstream-connection can only be used when executing STANDBY CLONE in Barman mode")); + } } /* Warn about parameters which apply to STANDBY SWITCHOVER only */ From 1d05345aa35ae569276b51b6ba248d2648e5e70e Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 11:39:48 +0900 Subject: [PATCH 11/17] Add error code ERR_BARMAN Indicates unrecoverable error condition when accessing the barman server --- README.md | 1 + errcode.h | 1 + repmgr.c | 13 +++++-------- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index a69b527e..afdf7744 100644 --- a/README.md +++ b/README.md @@ -1584,6 +1584,7 @@ exit: * ERR_MONITORING_FAIL (16) Unrecoverable error encountered during monitoring (repmgrd only) * ERR_BAD_BACKUP_LABEL (17) Corrupt or unreadable backup label encountered (repmgr only) * ERR_SWITCHOVER_FAIL (18) Error encountered during switchover (repmgr only) +* ERR_BARMAN (19) Unrecoverable error while accessing the barman server (repmgr only) Support and Assistance diff --git a/errcode.h b/errcode.h index d190bf0b..45c43c77 100644 --- a/errcode.h +++ b/errcode.h @@ -38,5 +38,6 @@ #define ERR_MONITORING_FAIL 16 #define ERR_BAD_BACKUP_LABEL 17 #define ERR_SWITCHOVER_FAIL 18 +#define ERR_BARMAN 19 #endif /* _ERRCODE_H_ */ diff --git a/repmgr.c b/repmgr.c index 946bb29d..430f96d7 100644 --- a/repmgr.c +++ b/repmgr.c @@ -1868,8 +1868,7 @@ do_standby_clone(void) options.barman_server); log_hint(_("Refer to the Barman documentation for more information\n")); - // ERR_BARMAN - exit(ERR_INTERNAL); + exit(ERR_BARMAN); } /* @@ -1904,8 +1903,7 @@ do_standby_clone(void) { log_err(_("Unable to fetch server parameters from Barman server\n")); - // ERR_BARMAN - exit(ERR_INTERNAL); + exit(ERR_BARMAN); } } @@ -2054,8 +2052,7 @@ do_standby_clone(void) { log_err(_("Unable to parse barman conninfo string \"%s\":\n%s\n"), barman_conninfo_str, errmsg); - // ERR_BARMAN - exit(ERR_BAD_CONFIG); + exit(ERR_BARMAN); } /* Overwrite database name in the parsed parameter list */ @@ -2384,7 +2381,7 @@ do_standby_clone(void) if (fi == NULL) { log_err("Cannot launch command: %s\n", command); - exit(ERR_INTERNAL); + exit(ERR_BARMAN); } fd = fopen(datadir_list_filename, "w"); @@ -2405,7 +2402,7 @@ do_standby_clone(void) { log_err("Unexpected output from \"barman list-files\": %s\n", output); - exit(ERR_INTERNAL); + exit(ERR_BARMAN); } /* From e87399afc17512640258fd4622281a20b8c9cd27 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 13:03:59 +0900 Subject: [PATCH 12/17] Clean up create_recovery_file() function Caller is responsible for providing a t_conninfo_param_list from which the value for "primary_conninfo" is created. --- repmgr.c | 63 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/repmgr.c b/repmgr.c index 430f96d7..eb47a768 100644 --- a/repmgr.c +++ b/repmgr.c @@ -96,7 +96,7 @@ static int copy_remote_files(char *host, char *remote_user, char *remote_path, static int run_basebackup(const char *data_dir, int server_version); static void check_parameters_for_action(const int action); static bool create_schema(PGconn *conn); -static bool create_recovery_file(const char *data_dir, t_conninfo_param_list *upstream_conninfo, const char *barmans_conninfo); +static bool create_recovery_file(const char *data_dir, t_conninfo_param_list *upstream_conninfo); static void write_primary_conninfo(char *line, t_conninfo_param_list *param_list); static bool write_recovery_file_line(FILE *recovery_file, char *recovery_file_path, char *line); static void check_master_standby_version_match(PGconn *conn, PGconn *master_conn); @@ -148,6 +148,7 @@ static bool read_backup_label(const char *local_data_directory, struct BackupLab static void initialize_conninfo_params(t_conninfo_param_list *param_list, bool set_defaults); static void param_set(t_conninfo_param_list *param_list, const char *param, const char *value); static bool parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char *errmsg); +static void conn_to_param_list(PGconn *conn, t_conninfo_param_list *param_list); static void parse_pg_basebackup_options(const char *pg_basebackup_options, t_basebackup_options *backup_options); /* Global variables */ @@ -1847,7 +1848,7 @@ do_standby_clone(void) initialize_conninfo_params(&upstream_conninfo, true); - /* Sanity check barman connection and installation, */ + /* Sanity-check barman connection and installation */ if (mode == barman) { char command[MAXLEN]; @@ -1933,9 +1934,6 @@ do_standby_clone(void) * If a connection was established, perform some sanity checks on the * provided upstream connection */ - PQconninfoOption *connOptions; - PQconninfoOption *option; - t_node_info upstream_node_record = T_NODE_INFO_INITIALIZER; int query_result; @@ -1987,17 +1985,7 @@ do_standby_clone(void) * particularly stuff like passwords extracted from PGPASSFILE; * these will be overridden from the upstream conninfo, if provided */ - - connOptions = PQconninfo(source_conn); - for (option = connOptions; option && option->keyword; option++) - { - /* Ignore non-set or blank parameter values*/ - if((option->val == NULL) || - (option->val != NULL && option->val[0] == '\0')) - continue; - - param_set(&upstream_conninfo, option->keyword, option->val); - } + conn_to_param_list(source_conn, &upstream_conninfo); // now set up conninfo params for the actual upstream, as we may be cloning from a different node // if upstream conn @@ -3034,18 +3022,13 @@ stop_backup: /* Finally, write the recovery.conf file */ - create_recovery_file(local_data_directory, &upstream_conninfo, NULL); + create_recovery_file(local_data_directory, &upstream_conninfo); - if (mode == barman && source_conn == NULL) + if (mode == barman) { - /* In Barman mode, remove local_repmgr_directory */ rmdir(local_repmgr_directory); } - else - { - - } switch(mode) { @@ -3476,6 +3459,7 @@ do_standby_follow(void) char master_conninfo[MAXLEN]; PGconn *master_conn; int master_id = 0; + t_conninfo_param_list upstream_conninfo; int r, retval; @@ -3609,8 +3593,11 @@ do_standby_follow(void) log_info(_("changing standby's master\n")); /* write the recovery.conf file */ -// if (!create_recovery_file(data_dir, master_conn, NULL)) -// exit(ERR_BAD_CONFIG); + initialize_conninfo_params(&upstream_conninfo, false); + conn_to_param_list(master_conn, &upstream_conninfo); + + if (!create_recovery_file(data_dir, &upstream_conninfo)) + exit(ERR_BAD_CONFIG); /* Finally, restart the service */ if (*options.restart_command) @@ -5290,8 +5277,6 @@ do_witness_unregister(void) } - // XXX + --no-upstream-connection - static void do_help(void) { @@ -5375,10 +5360,10 @@ do_help(void) /* - * Creates a recovery file for a standby. + * Creates a recovery.conf file for a standby */ static bool -create_recovery_file(const char *data_dir, t_conninfo_param_list *upstream_conninfo, const char *conninfo_on_barman) +create_recovery_file(const char *data_dir, t_conninfo_param_list *upstream_conninfo) { FILE *recovery_file; char recovery_file_path[MAXLEN]; @@ -7057,6 +7042,7 @@ param_set(t_conninfo_param_list *param_list, const char *param, const char *valu */ } + static bool parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char *errmsg) { @@ -7082,6 +7068,25 @@ parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_lis } +static void +conn_to_param_list(PGconn *conn, t_conninfo_param_list *param_list) +{ + PQconninfoOption *connOptions; + PQconninfoOption *option; + + connOptions = PQconninfo(conn); + for (option = connOptions; option && option->keyword; option++) + { + /* Ignore non-set or blank parameter values*/ + if((option->val == NULL) || + (option->val != NULL && option->val[0] == '\0')) + continue; + + param_set(param_list, option->keyword, option->val); + } +} + + static void parse_pg_basebackup_options(const char *pg_basebackup_options, t_basebackup_options *backup_options) { From 719ad3cf95459ed799156e4b75accaa2b28ea198 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 13:37:17 +0900 Subject: [PATCH 13/17] repmgr: properly remove temporary directory created when cloning from Barman --- repmgr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repmgr.c b/repmgr.c index eb47a768..6334c80c 100644 --- a/repmgr.c +++ b/repmgr.c @@ -3027,7 +3027,7 @@ stop_backup: if (mode == barman) { /* In Barman mode, remove local_repmgr_directory */ - rmdir(local_repmgr_directory); + rmtree(local_repmgr_directory, true); } switch(mode) From 5276cb279c3d6833217e36fd044187140c22eb10 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 13:46:44 +0900 Subject: [PATCH 14/17] repmgr: add correct return codes for get_tablespace_data() Remove unused variable retval. --- repmgr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/repmgr.c b/repmgr.c index 6334c80c..7bb1817d 100644 --- a/repmgr.c +++ b/repmgr.c @@ -1555,7 +1555,7 @@ tablespace_data_append(TablespaceDataList *list, const char *name, const char *o int get_tablespace_data(PGconn *upstream_conn, TablespaceDataList *list) { - int i, retval; + int i; char sqlquery[QUERY_STR_LEN]; PGresult *res; @@ -1573,7 +1573,7 @@ get_tablespace_data(PGconn *upstream_conn, TablespaceDataList *list) PQclear(res); - return retval; + return ERR_DB_QUERY; } for (i = 0; i < PQntuples(res); i++) @@ -1581,7 +1581,7 @@ get_tablespace_data(PGconn *upstream_conn, TablespaceDataList *list) PQgetvalue(res, i, 2)); PQclear(res); - return retval; + return SUCCESS; } char * From d12ecba63c533a2b71eb02cb81d1fe6e33054ded Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 31 Aug 2016 13:50:16 +0900 Subject: [PATCH 15/17] repmgr: fix Barman error message in standy clone Report the Barman catalogue name for the managed server, not the hostname of the Barman server itself. --- repmgr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repmgr.c b/repmgr.c index 7bb1817d..072ec76a 100644 --- a/repmgr.c +++ b/repmgr.c @@ -1866,7 +1866,7 @@ do_standby_clone(void) if (command_ok == false) { log_err(_("No valid backup for server %s was found in the Barman catalogue\n"), - options.barman_server); + options.cluster_name); log_hint(_("Refer to the Barman documentation for more information\n")); exit(ERR_BARMAN); From 2105837ef4ce91e7aaf315513df4088152c4c632 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Thu, 1 Sep 2016 12:06:19 +0900 Subject: [PATCH 16/17] repmgr: in standby clone, always add user-supplied connection information to upstream default parameters Also misc other cleanup and documentation. --- repmgr.c | 77 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/repmgr.c b/repmgr.c index 072ec76a..9005aa5c 100644 --- a/repmgr.c +++ b/repmgr.c @@ -1846,8 +1846,36 @@ do_standby_clone(void) exit(ERR_BAD_CONFIG); } + /* + * Initialise list of conninfo parameters which will later be used + * to create the `primary_conninfo` string in recovery.conf . + * + * We'll initialise it with the default values as seen by libpq, + * and overwrite them with the host settings specified on the command + * line. As it's possible the standby will be cloned from a node different + * to its intended upstream, we'll later attempt to fetch the + * upstream node record and overwrite the values set here with + * those from the upstream node record. + */ initialize_conninfo_params(&upstream_conninfo, true); + if (strlen(runtime_options.host)) + { + param_set(&upstream_conninfo, "host", runtime_options.host); + } + if (strlen(runtime_options.masterport)) + { + param_set(&upstream_conninfo, "port", runtime_options.masterport); + } + if (strlen(runtime_options.dbname)) + { + param_set(&upstream_conninfo, "dbname", runtime_options.dbname); + } + if (strlen(runtime_options.username)) + { + param_set(&upstream_conninfo, "user", runtime_options.username); + } + /* Sanity-check barman connection and installation */ if (mode == barman) { @@ -1987,11 +2015,9 @@ do_standby_clone(void) */ conn_to_param_list(source_conn, &upstream_conninfo); - // now set up conninfo params for the actual upstream, as we may be cloning from a different node - // if upstream conn - // get upstream node rec directly - - + /* + * Attempt to find the upstream node record + */ if (options.upstream_node == NO_UPSTREAM_NODE) upstream_node_id = get_master_node_id(source_conn, options.cluster_name); else @@ -2018,18 +2044,18 @@ do_standby_clone(void) * name with the repmgr one (they could well be different) and remotely execute * psql. */ - char buf[MAXLEN]; - char barman_conninfo_str[MAXLEN]; + char buf[MAXLEN]; + char barman_conninfo_str[MAXLEN]; t_conninfo_param_list barman_conninfo; - char *errmsg = NULL; - bool parse_success; - char where_condition[MAXLEN]; + char *errmsg = NULL; + bool parse_success, + command_success; + char where_condition[MAXLEN]; PQExpBufferData command_output; PQExpBufferData repmgr_conninfo_buf; int c; - /* XXX barman also returns "streaming_conninfo" - what's the difference? */ get_barman_property(barman_conninfo_str, "conninfo", local_repmgr_directory); initialize_conninfo_params(&barman_conninfo, false); @@ -2047,7 +2073,6 @@ do_standby_clone(void) param_set(&barman_conninfo, "dbname", runtime_options.dbname); /* Rebuild the Barman conninfo string */ - initPQExpBuffer(&repmgr_conninfo_buf); for (c = 0; c < barman_conninfo.size && barman_conninfo.keywords[c] != NULL; c++) @@ -2087,8 +2112,13 @@ do_standby_clone(void) termPQExpBuffer(&repmgr_conninfo_buf); - // XXX check success - (void)local_command(buf, &command_output); + command_success = local_command(buf, &command_output); + + if (command_success == false) + { + log_err(_("Unable to execute database query via Barman server\n")); + exit(ERR_BARMAN); + } maxlen_snprintf(upstream_conninfo_str, "%s", command_output.data); string_remove_trailing_newlines(upstream_conninfo_str); @@ -2100,9 +2130,6 @@ do_standby_clone(void) termPQExpBuffer(&command_output); } - printf("upstream found? %c\n", upstream_record_found == true ? 'y' : 'n'); - - if (upstream_record_found == true) { /* parse returned upstream conninfo string to recovery primary_conninfo params*/ @@ -2137,22 +2164,6 @@ do_standby_clone(void) exit(ERR_BAD_CONFIG); } - if (strlen(runtime_options.host)) - { - param_set(&upstream_conninfo, "host", runtime_options.host); - } - if (strlen(runtime_options.masterport)) - { - param_set(&upstream_conninfo, "port", runtime_options.masterport); - } - if (strlen(runtime_options.dbname)) - { - param_set(&upstream_conninfo, "dbname", runtime_options.dbname); - } - if (strlen(runtime_options.username)) - { - param_set(&upstream_conninfo, "user", runtime_options.username); - } } if (mode != barman) From 171df20386c4a15d0a06f615a5fd0b0b3ef12b39 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Thu, 1 Sep 2016 12:14:36 +0900 Subject: [PATCH 17/17] repmgr: avoid (null) in local_command debug output --- repmgr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/repmgr.c b/repmgr.c index 9005aa5c..72f0975c 100644 --- a/repmgr.c +++ b/repmgr.c @@ -6875,7 +6875,10 @@ local_command(const char *command, PQExpBufferData *outputbuf) pclose(fp); - log_verbose(LOG_DEBUG, "local_command(): output returned was:\n%s", outputbuf->data); + if (outputbuf->data != NULL) + log_verbose(LOG_DEBUG, "local_command(): output returned was:\n%s", outputbuf->data); + else + log_verbose(LOG_DEBUG, "local_command(): no output returned\n"); return true; }