From 4c5f065ef237332ae77665fb26f09cff4c145df1 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Fri, 12 May 2017 08:46:49 +0900 Subject: [PATCH] Check data directory not used by an active instance before cloning This involves adding some infrastructure to parse pg_control in a reasonably version-independent fashion. This will probably be useful in other contexts where we need to verify the status of a data directory where the instance isn't running. --- Makefile.in | 3 +- controldata.c | 110 ++++++++++++++++++++++++++++++++++++++++ controldata.h | 27 ++++++++++ repmgr-action-master.c | 2 +- repmgr-action-standby.c | 26 +++++++++- 5 files changed, 165 insertions(+), 3 deletions(-) create mode 100644 controldata.c create mode 100644 controldata.h diff --git a/Makefile.in b/Makefile.in index 8b443b50..d56721c7 100644 --- a/Makefile.in +++ b/Makefile.in @@ -27,7 +27,7 @@ include Makefile.global $(info Building against PostgreSQL $(MAJORVERSION)) REPMGR_CLIENT_OBJS = repmgr-client.o repmgr-action-master.o repmgr-action-standby.o repmgr-action-cluster.o \ - config.o log.o strutil.o dbutils.o dirutil.o compat.o + config.o log.o strutil.o dbutils.o dirutil.o compat.o controldata.o REPMGRD_OBJS = repmgrd.o $(REPMGR_CLIENT_OBJS): repmgr-client.h @@ -58,6 +58,7 @@ additional-clean: rm -f repmgrd.o rm -f compat.o rm -f config.o + rm -f controldata.o rm -f dbutils.o rm -f dirutil.o rm -f log.o diff --git a/controldata.c b/controldata.c new file mode 100644 index 00000000..77612418 --- /dev/null +++ b/controldata.c @@ -0,0 +1,110 @@ +/* + * controldata.c + * Copyright (c) 2ndQuadrant, 2010-2017 + * + * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + */ + +#include +#include +#include + +#include "postgres_fe.h" +#include "port/pg_crc32c.h" + +#include "repmgr.h" +#include "controldata.h" + +static ControlFileInfo *get_controlfile(char *DataDir); + +DBState +get_db_state(char *data_directory) +{ + ControlFileInfo *control_file_info; + DBState state; + + control_file_info = get_controlfile(data_directory); + + if (control_file_info->control_file_processed == true) + state = control_file_info->control_file->state; + else + /* if we were unable to parse the control file, assume DB is shut down */ + state = DB_SHUTDOWNED; + + pfree(control_file_info->control_file); + pfree(control_file_info); + return state; +} + +const char * +describe_db_state(DBState state) +{ + switch (state) + { + case DB_STARTUP: + return _("starting up"); + case DB_SHUTDOWNED: + return _("shut down"); + case DB_SHUTDOWNED_IN_RECOVERY: + return _("shut down in recovery"); + case DB_SHUTDOWNING: + return _("shutting down"); + case DB_IN_CRASH_RECOVERY: + return _("in crash recovery"); + case DB_IN_ARCHIVE_RECOVERY: + return _("in archive recovery"); + case DB_IN_PRODUCTION: + return _("in production"); + } + return _("unrecognized status code"); +} + + +/* + * we maintain our own version of get_controlfile() as we don't care if + * the file isn't readable + */ +static ControlFileInfo * +get_controlfile(char *DataDir) +{ + ControlFileInfo *control_file_info; + int fd; + char ControlFilePath[MAXPGPATH]; + pg_crc32c crc; + + control_file_info = palloc0(sizeof(ControlFileInfo)); + control_file_info->control_file_processed = false; + control_file_info->control_file = palloc0(sizeof(ControlFileData)); + + snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir); + + if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1) + { + log_debug("could not open file \"%s\" for reading: %s", + ControlFilePath, strerror(errno)); + return control_file_info; + } + + if (read(fd, control_file_info->control_file, sizeof(ControlFileData)) != sizeof(ControlFileData)) + { + log_debug("could not read file \"%s\": %s", + ControlFilePath, strerror(errno)); + return control_file_info; + } + + close(fd); + + control_file_info->control_file_processed = true; + + /* + * We don't check the CRC here as we're potentially checking a pg_control + * file from a different PostgreSQL version to the one repmgr was compiled + * against. However we're only interested in the first few fields, which + * should be constant across supported versions + * + * XXX double-check this + */ + + return control_file_info; +} diff --git a/controldata.h b/controldata.h new file mode 100644 index 00000000..4b5a1a8b --- /dev/null +++ b/controldata.h @@ -0,0 +1,27 @@ +/* + * controldata.h + * Copyright (c) 2ndQuadrant, 2010-2017 + * + * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + */ + +#ifndef _CONTROLDATA_H_ +#define _CONTROLDATA_H_ + +#include "postgres_fe.h" +#include "catalog/pg_control.h" + +typedef struct +{ + bool control_file_processed; + ControlFileData *control_file; +} ControlFileInfo; + +extern DBState +get_db_state(char *data_directory); + +extern const char * +describe_db_state(DBState state); + +#endif /* _CONTROLDATA_H_ */ diff --git a/repmgr-action-master.c b/repmgr-action-master.c index acfbbce8..086df23a 100644 --- a/repmgr-action-master.c +++ b/repmgr-action-master.c @@ -138,7 +138,7 @@ do_master_register(void) node_info.priority = config_file_options.priority; initPQExpBuffer(&event_description); - puts("here"); + if (record_found) { record_created = update_node_record(conn, diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c index a33833c6..0023d10d 100644 --- a/repmgr-action-standby.c +++ b/repmgr-action-standby.c @@ -11,6 +11,7 @@ #include "repmgr.h" #include "dirutil.h" #include "compat.h" +#include "controldata.h" #include "repmgr-client-global.h" #include "repmgr-action-standby.h" @@ -148,15 +149,21 @@ do_standby_clone(void) } else if (mode == barman) { + /* XXX in Barman mode it's still possible to connect to the upstream, + * so only fail if that's not available. + */ log_error(_("Barman mode requires a data directory")); log_hint(_("use -D/--pgdata to explicitly specify a data directory")); exit(ERR_BAD_CONFIG); } /* - * target directory (-D/--pgdata) provided - use that as new data directory + * 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) + * + * Note: if no directory provided, check_source_server() will later set + * local_data_directory from the upstream configuration. */ if (local_data_directory_provided == true) { @@ -245,6 +252,23 @@ do_standby_clone(void) check_source_server_via_barman(); } + /* + * by this point we should know the target data directory - check + * there's no running Pg instance + */ + if (is_pg_dir(local_data_directory)) + { + DBState state = get_db_state(local_data_directory); + + if (state != DB_SHUTDOWNED && state != DB_SHUTDOWNED_IN_RECOVERY) + { + log_error(_("target data directory appears to contain an active PostgreSQL instance")); + log_detail(_("instance state is %s"), describe_db_state(state)); + exit(ERR_BAD_CONFIG); + } + } + + if (upstream_record_found == true) { /* parse returned upstream conninfo string to recovery primary_conninfo params*/