Refactoring. Get rid of global variable backup_instance_path

Author: alubennikova

src/backup.c

@@ -50,12 +50,12 @@ static void *backup_files(void *arg);
 static void do_backup_pg(InstanceState *instanceState, PGconn *backup_conn,
 						 PGNodeInfo *nodeInfo, bool no_sync, bool backup_logs);
 
-static void pg_start_backup(const char *label, bool smooth, pgBackup *backup,
+static void pg_start_backup(InstanceState *instanceState, const char *label, bool smooth, pgBackup *backup,
 							PGNodeInfo *nodeInfo, PGconn *conn);
 static void pg_switch_wal(PGconn *conn);
-static void pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn, PGNodeInfo *nodeInfo);
+static void pg_stop_backup(InstanceState *instanceState, pgBackup *backup, PGconn *pg_startbackup_conn, PGNodeInfo *nodeInfo);
 
-static XLogRecPtr wait_wal_lsn(XLogRecPtr lsn, bool is_start_lsn, TimeLineID tli,
+static XLogRecPtr wait_wal_lsn(InstanceState *instanceState, XLogRecPtr lsn, bool is_start_lsn, TimeLineID tli,
 								bool in_prev_segment, bool segment_only,
 								int timeout_elevel, bool in_stream_dir);
 
@@ -77,14 +77,15 @@ static void set_cfs_datafiles(parray *files, const char *root, char *relative, s
 static void
 backup_stopbackup_callback(bool fatal, void *userdata)
 {
-	PGconn *pg_startbackup_conn = (PGconn *) userdata;
+	InstanceState *instanceState = (InstanceState *) userdata;
+	PGconn *pg_startbackup_conn = instanceState->conn;
 	/*
 	 * If backup is in progress, notify stop of backup to PostgreSQL
 	 */
 	if (backup_in_progress)
 	{
 		elog(WARNING, "backup in progress, stop backup");
-		pg_stop_backup(NULL, pg_startbackup_conn, NULL);	/* don't care about stop_lsn in case of error */
+		pg_stop_backup(instanceState, NULL, pg_startbackup_conn, NULL);	/* don't care about stop_lsn in case of error */
 	}
 }
 
@@ -139,7 +140,7 @@ do_backup_pg(InstanceState *instanceState, PGconn *backup_conn,
 			strlen(" with pg_probackup"));
 
 	/* Call pg_start_backup function in PostgreSQL connect */
-	pg_start_backup(label, smooth_checkpoint, &current, nodeInfo, backup_conn);
+	pg_start_backup(instanceState, label, smooth_checkpoint, &current, nodeInfo, backup_conn);
 
 	/* Obtain current timeline */
 #if PG_VERSION_NUM >= 90600
@@ -270,7 +271,7 @@ do_backup_pg(InstanceState *instanceState, PGconn *backup_conn,
 		 * Because WAL streaming will start after pg_start_backup() in stream
 		 * mode.
 		 */
-		wait_wal_lsn(current.start_lsn, true, current.tli, false, true, ERROR, false);
+		wait_wal_lsn(instanceState, current.start_lsn, true, current.tli, false, true, ERROR, false);
 	}
 
 	/* start stream replication */
@@ -527,7 +528,7 @@ do_backup_pg(InstanceState *instanceState, PGconn *backup_conn,
 	}
 
 	/* Notify end of backup */
-	pg_stop_backup(&current, backup_conn, nodeInfo);
+	pg_stop_backup(instanceState, &current, backup_conn, nodeInfo);
 
 	/* In case of backup from replica >= 9.6 we must fix minRecPoint,
 	 * First we must find pg_control in backup_files_list.
@@ -1028,7 +1029,7 @@ confirm_block_size(PGconn *conn, const char *name, int blcksz)
  * Notify start of backup to PostgreSQL server.
  */
 static void
-pg_start_backup(const char *label, bool smooth, pgBackup *backup,
+pg_start_backup(InstanceState *instanceState, const char *label, bool smooth, pgBackup *backup,
 				PGNodeInfo *nodeInfo, PGconn *conn)
 {
 	PGresult   *res;
@@ -1056,7 +1057,8 @@ pg_start_backup(const char *label, bool smooth, pgBackup *backup,
 	 * is necessary to call pg_stop_backup() in backup_cleanup().
 	 */
 	backup_in_progress = true;
-	pgut_atexit_push(backup_stopbackup_callback, conn);
+	instanceState->conn = conn;
+	pgut_atexit_push(backup_stopbackup_callback, instanceState);
 
 	/* Extract timeline and LSN from results of pg_start_backup() */
 	XLogDataFromLSN(PQgetvalue(res, 0, 0), &lsn_hi, &lsn_lo);
@@ -1262,7 +1264,7 @@ pg_is_superuser(PGconn *conn)
  * Returns InvalidXLogRecPtr if 'segment_only' flag is used.
  */
 static XLogRecPtr
-wait_wal_lsn(XLogRecPtr target_lsn, bool is_start_lsn, TimeLineID tli,
+wait_wal_lsn(InstanceState *instanceState, XLogRecPtr target_lsn, bool is_start_lsn, TimeLineID tli,
 			 bool in_prev_segment, bool segment_only,
 			 int timeout_elevel, bool in_stream_dir)
 {
@@ -1296,8 +1298,9 @@ wait_wal_lsn(XLogRecPtr target_lsn, bool is_start_lsn, TimeLineID tli,
 	 */
 	if (in_stream_dir)
 	{
-		pgBackupGetPath2(&current, pg_wal_dir, lengthof(pg_wal_dir),
-						 DATABASE_DIR, PG_XLOG_DIR);
+		snprintf(pg_wal_dir, lengthof(pg_wal_dir), "%s/%s/%s/%s",
+				 instanceState->instance_backup_subdir_path, base36enc(current.start_time),
+				 DATABASE_DIR, PG_XLOG_DIR);
 		join_path_components(wal_segment_path, pg_wal_dir, wal_segment);
 		wal_segment_dir = pg_wal_dir;
 	}
@@ -1438,7 +1441,7 @@ wait_wal_lsn(XLogRecPtr target_lsn, bool is_start_lsn, TimeLineID tli,
  * Notify end of backup to PostgreSQL server.
  */
 static void
-pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn,
+pg_stop_backup(InstanceState *instanceState, pgBackup *backup, PGconn *pg_startbackup_conn,
 				PGNodeInfo *nodeInfo)
 {
 	PGconn		*conn;
@@ -1566,7 +1569,8 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn,
 	}
 
 	/* After we have sent pg_stop_backup, we don't need this callback anymore */
-	pgut_atexit_pop(backup_stopbackup_callback, pg_startbackup_conn);
+	instanceState->conn = pg_startbackup_conn;
+	pgut_atexit_pop(backup_stopbackup_callback, instanceState);
 
 	/*
 	 * Wait for the result of pg_stop_backup(), but no longer than
@@ -1671,9 +1675,10 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn,
 
 			if (stream_wal)
 			{
-				pgBackupGetPath2(backup, stream_xlog_path,
-								 lengthof(stream_xlog_path),
-								 DATABASE_DIR, PG_XLOG_DIR);
+				snprintf(stream_xlog_path, lengthof(stream_xlog_path),
+						 "%s/%s/%s/%s", instanceState->instance_backup_subdir_path,
+				 		 base36enc(backup->start_time),
+						 DATABASE_DIR, PG_XLOG_DIR);
 				xlog_path = stream_xlog_path;
 			}
 			else
@@ -1702,7 +1707,7 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn,
 			if (stop_backup_lsn_tmp % instance_config.xlog_seg_size == 0)
 			{
 				/* Wait for segment with current stop_lsn, it is ok for it to never arrive */
-				wait_wal_lsn(stop_backup_lsn_tmp, false, backup->tli,
+				wait_wal_lsn(instanceState, stop_backup_lsn_tmp, false, backup->tli,
 							 false, true, WARNING, stream_wal);
 
 				/* Get the first record in segment with current stop_lsn */
@@ -1730,7 +1735,7 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn,
 					/* Despite looking for previous record there is not guarantee of success
 					 * because previous record can be the contrecord.
 					 */
-					lsn_tmp = wait_wal_lsn(stop_backup_lsn_tmp, false, backup->tli,
+					lsn_tmp = wait_wal_lsn(instanceState, stop_backup_lsn_tmp, false, backup->tli,
 											true, false, ERROR, stream_wal);
 
 					/* sanity */
@@ -1744,7 +1749,7 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn,
 			else if (stop_backup_lsn_tmp % XLOG_BLCKSZ == 0)
 			{
 				/* Wait for segment with current stop_lsn */
-				wait_wal_lsn(stop_backup_lsn_tmp, false, backup->tli,
+				wait_wal_lsn(instanceState, stop_backup_lsn_tmp, false, backup->tli,
 							 false, true, ERROR, stream_wal);
 
 				/* Get the next closest record in segment with current stop_lsn */
@@ -1776,7 +1781,8 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn,
 		if (!exclusive_backup)
 		{
 			Assert(PQnfields(res) >= 4);
-			pgBackupGetPath2(backup, path, lengthof(path), DATABASE_DIR, NULL);
+			snprintf(path, lengthof(path), "%s/%s/%s", instanceState->instance_backup_subdir_path,
+				 base36enc(backup->start_time), DATABASE_DIR);
 
 			/* Write backup_label */
 			join_path_components(backup_label, path, PG_BACKUP_LABEL_FILE);
@@ -1873,7 +1879,7 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn,
 		 * look for previous record with endpoint >= STOP_LSN.
 		 */
 		if (!stop_lsn_exists)
-			stop_backup_lsn = wait_wal_lsn(stop_backup_lsn_tmp, false, backup->tli,
+			stop_backup_lsn = wait_wal_lsn(instanceState, stop_backup_lsn_tmp, false, backup->tli,
 											false, false, ERROR, stream_wal);
 
 		if (stream_wal)
@@ -1883,9 +1889,10 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn,
 			if(wait_WAL_streaming_end(backup_files_list))
 				elog(ERROR, "WAL streaming failed");
 
-			pgBackupGetPath2(backup, stream_xlog_path,
-							 lengthof(stream_xlog_path),
-							 DATABASE_DIR, PG_XLOG_DIR);
+			snprintf(stream_xlog_path, lengthof(stream_xlog_path), "%s/%s/%s/%s",
+				 instanceState->instance_backup_subdir_path, base36enc(backup->start_time),
+				 DATABASE_DIR, PG_XLOG_DIR);
+
 			xlog_path = stream_xlog_path;
 		}
 		else

src/catalog.c

@@ -733,7 +733,8 @@ catalog_get_instance_list(CatalogState *catalogState)
 							catalogState->backup_subdir_path, instanceState->instance_name);
 		join_path_components(instanceState->instance_wal_subdir_path,
 							catalogState->wal_subdir_path, instanceState->instance_name);
-
+		join_path_components(instanceState->instance_config_path,
+							 instanceState->instance_backup_subdir_path, BACKUP_CATALOG_CONF_FILE);
 		parray_append(instances, instanceState);
 	}
 
@@ -2808,27 +2809,6 @@ pgBackupCompareIdDesc(const void *l, const void *r)
 	return -pgBackupCompareId(l, r);
 }
 
-/*
- * Construct absolute path of the backup directory.
- * Append "subdir1" and "subdir2" to the backup directory.
- */
-void
-pgBackupGetPath2(const pgBackup *backup, char *path, size_t len,
-				 const char *subdir1, const char *subdir2)
-{
-	/* If "subdir1" is NULL do not check "subdir2" */
-	if (!subdir1)
-		snprintf(path, len, "%s/%s", backup_instance_path,
-				 base36enc(backup->start_time));
-	else if (!subdir2)
-		snprintf(path, len, "%s/%s/%s", backup_instance_path,
-				 base36enc(backup->start_time), subdir1);
-	/* "subdir1" and "subdir2" is not NULL */
-	else
-		snprintf(path, len, "%s/%s/%s/%s", backup_instance_path,
-				 base36enc(backup->start_time), subdir1, subdir2);
-}
-
 /*
  * Check if multiple backups consider target backup to be their direct parent
  */

src/configure.c

@@ -277,18 +277,16 @@ do_show_config(void)
  * values into the file.
  */
 void
-do_set_config(bool missing_ok)
+do_set_config(InstanceState *instanceState, bool missing_ok)
 {
-	char		path[MAXPGPATH];
 	char		path_temp[MAXPGPATH];
 	FILE	   *fp;
 	int			i;
 
-	join_path_components(path, backup_instance_path, BACKUP_CATALOG_CONF_FILE);
-	snprintf(path_temp, sizeof(path_temp), "%s.tmp", path);
+	snprintf(path_temp, sizeof(path_temp), "%s.tmp", instanceState->instance_config_path);
 
-	if (!missing_ok && !fileExists(path, FIO_LOCAL_HOST))
-		elog(ERROR, "Configuration file \"%s\" doesn't exist", path);
+	if (!missing_ok && !fileExists(instanceState->instance_config_path, FIO_LOCAL_HOST))
+		elog(ERROR, "Configuration file \"%s\" doesn't exist", instanceState->instance_config_path);
 
 	fp = fopen(path_temp, "wt");
 	if (fp == NULL)
@@ -327,12 +325,12 @@ do_set_config(bool missing_ok)
 
 	fclose(fp);
 
-	if (rename(path_temp, path) < 0)
+	if (rename(path_temp, instanceState->instance_config_path) < 0)
 	{
 		int			errno_temp = errno;
 		unlink(path_temp);
 		elog(ERROR, "Cannot rename configuration file \"%s\" to \"%s\": %s",
-			 path_temp, path, strerror(errno_temp));
+			 path_temp, instanceState->instance_config_path, strerror(errno_temp));
 	}
 }
 
@@ -376,7 +374,6 @@ init_config(InstanceConfig *config, const char *instance_name)
 InstanceConfig *
 readInstanceConfigFile(InstanceState *instanceState)
 {
-	char	path[MAXPGPATH];
 	InstanceConfig   *instance = pgut_new(InstanceConfig);
 	char	   *log_level_console = NULL;
 	char	   *log_level_file = NULL;
@@ -602,21 +599,19 @@ readInstanceConfigFile(InstanceState *instanceState)
 			instanceState->catalog_state->catalog_path, "wal", instanceState->instance_name);
 	canonicalize_path(instance->arclog_path);
 
-	join_path_components(path, instance->backup_instance_path,
-						 BACKUP_CATALOG_CONF_FILE);
-
-	if (fio_access(path, F_OK, FIO_BACKUP_HOST) != 0)
+	if (fio_access(instanceState->instance_config_path, F_OK, FIO_BACKUP_HOST) != 0)
 	{
-		elog(WARNING, "Control file \"%s\" doesn't exist", path);
+		elog(WARNING, "Control file \"%s\" doesn't exist", instanceState->instance_config_path);
 		pfree(instance);
 		return NULL;
 	}
 
-	parsed_options = config_read_opt(path, instance_options, WARNING, true, true);
+	parsed_options = config_read_opt(instanceState->instance_config_path,
+									 instance_options, WARNING, true, true);
 
 	if (parsed_options == 0)
 	{
-		elog(WARNING, "Control file \"%s\" is empty", path);
+		elog(WARNING, "Control file \"%s\" is empty", instanceState->instance_config_path);
 		pfree(instance);
 		return NULL;
 	}

src/delete.c

@@ -972,8 +972,6 @@ do_delete_instance(InstanceState *instanceState)
 {
 	parray		*backup_list;
 	int 		i;
-	char		instance_config_path[MAXPGPATH];
-
 
 	/* Delete all backups. */
 	backup_list = catalog_get_backup_list(instanceState, INVALID_BACKUP_ID);
@@ -994,16 +992,15 @@ do_delete_instance(InstanceState *instanceState)
 	pgut_rmtree(arclog_path, false, true);
 
 	/* Delete backup instance config file */
-	join_path_components(instance_config_path, backup_instance_path, BACKUP_CATALOG_CONF_FILE);
-	if (remove(instance_config_path))
+	if (remove(instanceState->instance_config_path))
 	{
-		elog(ERROR, "Can't remove \"%s\": %s", instance_config_path,
+		elog(ERROR, "Can't remove \"%s\": %s", instanceState->instance_config_path,
 			strerror(errno));
 	}
 
 	/* Delete instance root directories */
-	if (rmdir(backup_instance_path) != 0)
-		elog(ERROR, "Can't remove \"%s\": %s", backup_instance_path,
+	if (rmdir(instanceState->instance_backup_subdir_path) != 0)
+		elog(ERROR, "Can't remove \"%s\": %s", instanceState->instance_backup_subdir_path,
 			strerror(errno));
 
 	if (rmdir(arclog_path) != 0)

src/init.c

@@ -119,7 +119,7 @@ do_add_instance(InstanceState *instanceState, InstanceConfig *instance)
 				   SOURCE_DEFAULT);
 
 	/* pgdata was set through command line */
-	do_set_config(true);
+	do_set_config(instanceState, true);
 
 	elog(INFO, "Instance '%s' successfully inited", instanceState->instance_name);
 	return 0;

src/pg_probackup.c

@@ -68,7 +68,7 @@ static char	   *backup_path = NULL;
  * path or to the data files in the backup catalog
  * $BACKUP_PATH/backups/instance_name
  */
-char		backup_instance_path[MAXPGPATH];
+static char		backup_instance_path[MAXPGPATH];
 /*
  * path or to the wal files in the backup catalog
  * $BACKUP_PATH/wal/instance_name
@@ -491,6 +491,9 @@ main(int argc, char *argv[])
 							catalogState->backup_subdir_path, instanceState->instance_name);
 		join_path_components(instanceState->instance_wal_subdir_path,
 							catalogState->wal_subdir_path, instanceState->instance_name);
+		join_path_components(instanceState->instance_config_path,
+							 instanceState->instance_backup_subdir_path, BACKUP_CATALOG_CONF_FILE);
+
 	}
 	/* ===== instanceState (END) ======*/
 
@@ -872,7 +875,7 @@ main(int argc, char *argv[])
 			do_show_config();
 			break;
 		case SET_CONFIG_CMD:
-			do_set_config(false);
+			do_set_config(instanceState, false);
 			break;
 		case SET_BACKUP_CMD:
 			if (!backup_id_string)