Fix creating a secondary server in an already existing directory. (#96)

* Fix creating a secondary server in an already existing directory.

This could have been a one-liner, but raises the question of the hard-coded
choice for the backup directory. The major reason of the target PGDATA
already existing is that it's targeting a mount-point. Then our default
choice of ${PGDATA}/../backup for the pg_basebackup target is not a good
one, because the renaming of the directory will cross file system boundaries
and not be atomic anymore.

This patch then allows to configure the backup directory. In passing, the
default value now includes the nodename, so that it's easier to run tests
environments with multiple instances on the same machine.

* Simplify the nodename in the docs configuration examples.

docs/ref/configuration.rst

@@ -116,7 +116,7 @@ An example configuration file looks like the following::
   monitor = postgres://autoctl_node@192.168.1.34:6000/pg_auto_failover
   formation = default
   group = 0
-  nodename = node1.db.local.tld
+  nodename = node1.db
   nodekind = standalone
   
   [postgresql]
@@ -129,6 +129,7 @@ An example configuration file looks like the following::
   [replication]
   slot = pgautofailover_standby
   maximum_backup_rate = 100M
+  backup_directory = /data/backup/node1.db
   
   [timeout]
   network_partition_timeout = 20
@@ -178,6 +179,18 @@ When pg_auto_failover (re-)builds a standby node using the ``pg_basebackup``
 command, this parameter is given to ``pg_basebackup`` to throttle the
 network bandwidth used. Defaults to 100Mbps.
 
+**replication.backup_directory**
+
+When pg_auto_failover (re-)builds a standby node using the ``pg_basebackup``
+command, this parameter is the target directory where to copy the bits from
+the primary server. When the copy has been successful, then the directory is
+renamed to **postgresql.pgdata**.
+
+The default value is computed from ``${PGDATA}/../backup/${nodename}`` and
+can be set to any value of your preference. Remember that the directory
+renaming is an atomic operation only when both the source and the target of
+the copy are in the same filesystem, at least in Unix systems.
+
 **timeout**
 
 This section allows to setup the behavior of the pg_auto_failover keeper in

docs/ref/reference.rst

@@ -360,7 +360,7 @@ When initializing a pg_auto_failover keeper with ``--pgdata /data/pgsql``, then:
       monitor = postgres://autoctl_node@192.168.1.34:6000/pg_auto_failover
       formation = default
       group = 1
-      nodename = node1.db.local.tld
+      nodename = node1.db
 
       [postgresql]
       pgdata = /data/pgsql/

src/bin/pg_autoctl/cli_do_misc.c

@@ -329,6 +329,7 @@ keeper_cli_init_standby(int argc, char **argv)
 	replicationSource.password = config.replication_password;
 	replicationSource.slotName = config.replication_slot_name;
 	replicationSource.maximumBackupRate = MAXIMUM_BACKUP_RATE;
+	replicationSource.backupDir = config.backupDirectory;
 
 	if (!standby_init_database(&postgres, &replicationSource))
 	{

src/bin/pg_autoctl/fsm_transition.c

@@ -421,6 +421,7 @@ fsm_init_standby(Keeper *keeper)
 	replicationSource.password = config->replication_password;
 	replicationSource.slotName = config->replication_slot_name;
 	replicationSource.maximumBackupRate = config->maximum_backup_rate;
+	replicationSource.backupDir = config->backupDirectory;
 
 	if (!standby_init_database(postgres, &replicationSource))
 	{
@@ -458,6 +459,7 @@ fsm_rewind_or_init(Keeper *keeper)
 	replicationSource.password = config->replication_password;
 	replicationSource.slotName = config->replication_slot_name;
 	replicationSource.maximumBackupRate = config->maximum_backup_rate;
+	replicationSource.backupDir = config->backupDirectory;
 
 	if (!primary_rewind_to_standby(postgres, &replicationSource))
 	{

src/bin/pg_autoctl/keeper_config.c

@@ -93,6 +93,10 @@
 							   false, &config->maximum_backup_rate, \
 							   MAXIMUM_BACKUP_RATE)
 
+#define OPTION_REPLICATION_BACKUP_DIR(config) \
+	make_strbuf_option("replication", "backup_directory", NULL, \
+					   false, MAXPGPATH, config->backupDirectory)
+
 #define OPTION_TIMEOUT_NETWORK_PARTITION(config) \
 	make_int_option_default("timeout", "network_partition_timeout", \
 							NULL, false, \
@@ -146,6 +150,7 @@
 		OPTION_REPLICATION_PASSWORD(config), \
 		OPTION_REPLICATION_SLOT_NAME(config), \
 		OPTION_REPLICATION_MAXIMUM_BACKUP_RATE(config), \
+		OPTION_REPLICATION_BACKUP_DIR(config), \
 		OPTION_TIMEOUT_NETWORK_PARTITION(config), \
 		OPTION_TIMEOUT_PREPARE_PROMOTION_CATCHUP(config), \
 		OPTION_TIMEOUT_PREPARE_PROMOTION_WALRECEIVER(config), \
@@ -155,6 +160,7 @@
 	}
 
 static bool keeper_config_init_nodekind(KeeperConfig *config);
+static bool keeper_config_set_backup_directory(KeeperConfig *config);
 
 
 /*
@@ -235,6 +241,16 @@ keeper_config_init(KeeperConfig *config,
 	 */
 	memcpy(&(config->pgSetup), &pgSetup, sizeof(PostgresSetup));
 
+	/*
+	 * Compute the backupDirectory from pgdata, or check the one given in the
+	 * configuration file already.
+	 */
+	if (!keeper_config_set_backup_directory(config))
+	{
+		/* errors have already been logged */
+		exit(EXIT_CODE_BAD_CONFIG);
+	}
+
 	/* set our configuration and state file pathnames */
 	if (!SetConfigFilePath(&(config->pathnames), config->pgSetup.pgdata))
 	{
@@ -725,8 +741,60 @@ keeper_config_init_nodekind(KeeperConfig *config)
 
 
 /*
- * keeper_config_update_with_absolute_pgdata verifies that the pgdata path is an absolute one
- * If not, the config->pgSetup is updated and we rewrite the config file
+ * keeper_config_set_backup_directory sets the pg_basebackup target directory
+ * to ${PGDATA}/../backup/${nodename} by default. Adding the local nodename
+ * makes it possible to run several instances of Postgres and pg_autoctl on the
+ * same host, which is nice for development and testing scenarios.
+ */
+static bool
+keeper_config_set_backup_directory(KeeperConfig *config)
+{
+	char absoluteBackupDirectory[PATH_MAX];
+
+	/* create the temporary backup directory at $pgdata/../backup */
+	if (IS_EMPTY_STRING_BUFFER(config->backupDirectory))
+	{
+		char *pgdata = config->pgSetup.pgdata;
+		char subdirs[MAXPGPATH];
+
+		snprintf(subdirs, MAXPGPATH, "backup/%s", config->nodename);
+		path_in_same_directory(pgdata, subdirs, config->backupDirectory);
+	}
+
+	/*
+	 * The best way to make sure we are allowed to create the backup directory
+	 * is to just go ahead and create it now.
+	 */
+	log_debug("mkdir -p \"%s\"", config->backupDirectory);
+	if (!ensure_empty_dir(config->backupDirectory, 0700))
+	{
+		log_fatal("Failed to create the backup directory \"%s\", "
+				  "see above for details", config->backupDirectory);
+		return false;
+	}
+
+	/* Now get the realpath() of the directory we just created */
+	if (!realpath(config->backupDirectory, absoluteBackupDirectory))
+	{
+		/* non-fatal error, just keep the computed or given directory path */
+		log_warn("Failed to get the realpath of backup directory \"%s\": %s",
+				 config->backupDirectory, strerror(errno));
+		return true;
+	}
+
+	if (strcmp(config->backupDirectory, absoluteBackupDirectory) != 0)
+	{
+		strlcpy(config->backupDirectory, absoluteBackupDirectory, MAXPGPATH);
+	}
+
+	return true;
+}
+
+
+/*
+ * keeper_config_update_with_absolute_pgdata verifies that the pgdata path is
+ * an absolute one If not, the config->pgSetup is updated and we rewrite the
+ * config file
  */
 bool
 keeper_config_update_with_absolute_pgdata(KeeperConfig *config)

src/bin/pg_autoctl/keeper_config.h

@@ -37,6 +37,7 @@ typedef struct KeeperConfig
 	char *replication_slot_name;
 	char *replication_password;
 	char *maximum_backup_rate;
+	char backupDirectory[MAXPGPATH];
 
 	/* pg_autoctl timeouts */
 	int network_partition_timeout;

src/bin/pg_autoctl/pgctl.c

@@ -414,6 +414,7 @@ ensure_default_settings_file_exists(const char *configFilePath,
 bool
 pg_basebackup(const char *pgdata,
 			  const char *pg_ctl,
+			  const char *backupdir,
 			  const char *maximum_backup_rate,
 			  const char *replication_username,
 			  const char *replication_password,
@@ -423,12 +424,8 @@ pg_basebackup(const char *pgdata,
 	int returnCode;
 	Program program;
 	char primary_port_str[10];
-	char backupdir[MAXPGPATH];
 	char pg_basebackup[MAXPGPATH];
 
-	/* create the temporary backup directory at $pgdata/../backup */
-	path_in_same_directory(pgdata, "backup", backupdir);
-
 	log_debug("mkdir -p \"%s\"", backupdir);
 	if (!ensure_empty_dir(backupdir, 0700))
 	{

src/bin/pg_autoctl/pgctl.h

@@ -30,14 +30,17 @@ char * pg_ctl_version(const char *pg_ctl_path);
 bool pg_add_auto_failover_default_settings(PostgresSetup *pgSetup,
 										   char *configFilePath,
 										   GUC *settings);
-bool pg_basebackup(const char *pgdata, const char *pg_ctl,
+bool pg_basebackup(const char *pgdata,
+				   const char *pg_ctl,
+				   const char *backupdir,
 				   const char *maximum_backup_rate,
 				   const char *replication_username,
 				   const char *replication_password,
 				   const char *replication_slot_name,
 				   const char *primary_hostname, int primary_port);
 bool pg_rewind(const char *pgdata, const char *pg_ctl, const char *primaryHost,
-			   int primaryPort, const char *databaseName, const char *replicationUsername,
+			   int primaryPort, const char *databaseName,
+			   const char *replicationUsername,
 			   const char *replicationPassword);
 
 bool pg_ctl_initdb(const char *pg_ctl, const char *pgdata);

src/bin/pg_autoctl/pgsql.h

@@ -74,6 +74,7 @@ typedef struct ReplicationSource
 	char *slotName;
 	char *password;
 	char *maximumBackupRate;
+	char *backupDir;
 } ReplicationSource;
 
 

src/bin/pg_autoctl/primary_standby.c

@@ -474,7 +474,7 @@ standby_init_database(LocalPostgresServer *postgres,
 	log_trace("standby_init_database");
 	log_info("Initialising PostgreSQL as a hot standby");
 
-	if (directory_exists(pgSetup->pgdata))
+	if (pg_setup_pgdata_exists(pgSetup))
 	{
 		log_info("Target directory exists: \"%s\", stopping PostgreSQL",
 				 pgSetup->pgdata);
@@ -494,6 +494,7 @@ standby_init_database(LocalPostgresServer *postgres,
 	 */
 	if (!pg_basebackup(pgSetup->pgdata,
 					   pgSetup->pg_ctl,
+					   replicationSource->backupDir,
 					   replicationSource->maximumBackupRate,
 					   replicationSource->userName,
 					   replicationSource->password,