Refactoring. Get rid of global variable backup_path

Author: alubennikova

src/backup.c

@@ -157,7 +157,7 @@ do_backup_pg(InstanceState *instanceState, PGconn *backup_conn,
 		current.backup_mode == BACKUP_MODE_DIFF_DELTA)
 	{
 		/* get list of backups already taken */
-		backup_list = catalog_get_backup_list(instanceState->instance_name, INVALID_BACKUP_ID);
+		backup_list = catalog_get_backup_list(instanceState, INVALID_BACKUP_ID);
 
 		prev_backup = catalog_get_last_data_backup(backup_list, current.tli, current.start_time);
 		if (prev_backup == NULL)
@@ -168,7 +168,7 @@ do_backup_pg(InstanceState *instanceState, PGconn *backup_conn,
 						current.tli);
 
 			/* TODO: use read_timeline_history */
-			tli_list = catalog_get_timelines(&instance_config);
+			tli_list = catalog_get_timelines(instanceState, &instance_config);
 
 			if (parray_num(tli_list) == 0)
 				elog(WARNING, "Cannot find valid backup on previous timelines, "

src/catalog.c

@@ -688,38 +688,35 @@ IsDir(const char *dirpath, const char *entry, fio_location location)
 /*
  * Create list of instances in given backup catalog.
  *
- * Returns parray of "InstanceConfig" structures, filled with
- * actual config of each instance.
+ * Returns parray of InstanceState structures.
  */
 parray *
-catalog_get_instance_list(char *backup_catalog_path)
+catalog_get_instance_list(CatalogState *catalogState)
 {
-	char		path[MAXPGPATH];
 	DIR		   *dir;
 	struct dirent *dent;
 	parray		*instances;
 
 	instances = parray_new();
 
 	/* open directory and list contents */
-	join_path_components(path, backup_catalog_path, BACKUPS_DIR);
-	dir = opendir(path);
+	dir = opendir(catalogState->backup_subdir_path);
 	if (dir == NULL)
 		elog(ERROR, "Cannot open directory \"%s\": %s",
-			 path, strerror(errno));
+			 catalogState->backup_subdir_path, strerror(errno));
 
 	while (errno = 0, (dent = readdir(dir)) != NULL)
 	{
 		char		child[MAXPGPATH];
 		struct stat	st;
-		InstanceConfig *instance;
+		InstanceState *instanceState = NULL;
 
 		/* skip entries point current dir or parent dir */
 		if (strcmp(dent->d_name, ".") == 0 ||
 			strcmp(dent->d_name, "..") == 0)
 			continue;
 
-		join_path_components(child, path, dent->d_name);
+		join_path_components(child, catalogState->backup_subdir_path, dent->d_name);
 
 		if (lstat(child, &st) == -1)
 			elog(ERROR, "Cannot stat file \"%s\": %s",
@@ -728,9 +725,16 @@ catalog_get_instance_list(char *backup_catalog_path)
 		if (!S_ISDIR(st.st_mode))
 			continue;
 
-		instance = readInstanceConfigFile(dent->d_name);
+		instanceState = pgut_new(InstanceState);
+		instanceState->config = readInstanceConfigFile(instanceState);
+
+		strncpy(instanceState->instance_name, dent->d_name, MAXPGPATH);
+		join_path_components(instanceState->instance_backup_subdir_path,
+							catalogState->backup_subdir_path, instanceState->instance_name);
+		join_path_components(instanceState->instance_wal_subdir_path,
+							catalogState->wal_subdir_path, instanceState->instance_name);
 
-		parray_append(instances, instance);
+		parray_append(instances, instanceState);
 	}
 
 	/* TODO 3.0: switch to ERROR */
@@ -739,11 +743,11 @@ catalog_get_instance_list(char *backup_catalog_path)
 
 	if (errno)
 		elog(ERROR, "Cannot read directory \"%s\": %s",
-				path, strerror(errno));
+				catalogState->backup_subdir_path, strerror(errno));
 
 	if (closedir(dir))
 		elog(ERROR, "Cannot close directory \"%s\": %s",
-				path, strerror(errno));
+				catalogState->backup_subdir_path, strerror(errno));
 
 	return instances;
 }
@@ -755,22 +759,18 @@ catalog_get_instance_list(char *backup_catalog_path)
  * If valid backup id is passed only matching backup will be added to the list.
  */
 parray *
-catalog_get_backup_list(const char *instance_name, time_t requested_backup_id)
+catalog_get_backup_list(InstanceState *instanceState, time_t requested_backup_id)
 {
 	DIR		   *data_dir = NULL;
 	struct dirent *data_ent = NULL;
 	parray	   *backups = NULL;
 	int			i;
-	char backup_instance_path[MAXPGPATH];
-
-	sprintf(backup_instance_path, "%s/%s/%s",
-			backup_path, BACKUPS_DIR, instance_name);
 
 	/* open backup instance backups directory */
-	data_dir = fio_opendir(backup_instance_path, FIO_BACKUP_HOST);
+	data_dir = fio_opendir(instanceState->instance_backup_subdir_path, FIO_BACKUP_HOST);
 	if (data_dir == NULL)
 	{
-		elog(WARNING, "cannot open directory \"%s\": %s", backup_instance_path,
+		elog(WARNING, "cannot open directory \"%s\": %s", instanceState->instance_backup_subdir_path,
 			strerror(errno));
 		goto err_proc;
 	}
@@ -784,12 +784,12 @@ catalog_get_backup_list(const char *instance_name, time_t requested_backup_id)
 		pgBackup   *backup = NULL;
 
 		/* skip not-directory entries and hidden entries */
-		if (!IsDir(backup_instance_path, data_ent->d_name, FIO_BACKUP_HOST)
+		if (!IsDir(instanceState->instance_backup_subdir_path, data_ent->d_name, FIO_BACKUP_HOST)
 			|| data_ent->d_name[0] == '.')
 			continue;
 
 		/* open subdirectory of specific backup */
-		join_path_components(data_path, backup_instance_path, data_ent->d_name);
+		join_path_components(data_path, instanceState->instance_backup_subdir_path, data_ent->d_name);
 
 		/* read backup information from BACKUP_CONTROL_FILE */
 		snprintf(backup_conf_path, MAXPGPATH, "%s/%s", data_path, BACKUP_CONTROL_FILE);
@@ -835,7 +835,7 @@ catalog_get_backup_list(const char *instance_name, time_t requested_backup_id)
 	if (errno)
 	{
 		elog(WARNING, "cannot read backup root directory \"%s\": %s",
-			backup_instance_path, strerror(errno));
+			instanceState->instance_backup_subdir_path, strerror(errno));
 		goto err_proc;
 	}
 
@@ -1345,22 +1345,21 @@ create_backup_dir(pgBackup *backup, const char *backup_instance_path)
  * TODO: '.partial' and '.part' segno information should be added to tlinfo.
  */
 parray *
-catalog_get_timelines(InstanceConfig *instance)
+catalog_get_timelines(InstanceState *instanceState, InstanceConfig *instance)
 {
 	int i,j,k;
 	parray *xlog_files_list = parray_new();
 	parray *timelineinfos;
 	parray *backups;
 	timelineInfo *tlinfo;
-	char		arclog_path[MAXPGPATH];
 
 	/* for fancy reporting */
 	char begin_segno_str[MAXFNAMELEN];
 	char end_segno_str[MAXFNAMELEN];
 
 	/* read all xlog files that belong to this archive */
-	sprintf(arclog_path, "%s/%s/%s", backup_path, "wal", instance->name);
-	dir_list_file(xlog_files_list, arclog_path, false, false, false, false, true, 0, FIO_BACKUP_HOST);
+	dir_list_file(xlog_files_list, instanceState->instance_wal_subdir_path,
+				  false, false, false, false, true, 0, FIO_BACKUP_HOST);
 	parray_qsort(xlog_files_list, pgFileCompareName);
 
 	timelineinfos = parray_new();
@@ -1530,7 +1529,7 @@ catalog_get_timelines(InstanceConfig *instance)
 			TimeLineHistoryEntry *tln;
 
 			sscanf(file->name, "%08X.history", &tli);
-			timelines = read_timeline_history(arclog_path, tli, true);
+			timelines = read_timeline_history(instanceState->instance_wal_subdir_path, tli, true);
 
 			if (!tlinfo || tlinfo->tli != tli)
 			{
@@ -1564,7 +1563,7 @@ catalog_get_timelines(InstanceConfig *instance)
 	}
 
 	/* save information about backups belonging to each timeline */
-	backups = catalog_get_backup_list(instance->name, INVALID_BACKUP_ID);
+	backups = catalog_get_backup_list(instanceState, INVALID_BACKUP_ID);
 
 	for (i = 0; i < parray_num(timelineinfos); i++)
 	{
@@ -2037,7 +2036,7 @@ get_oldest_backup(timelineInfo *tlinfo)
  * Overwrite backup metadata.
  */
 void
-do_set_backup(const char *instance_name, time_t backup_id,
+do_set_backup(InstanceState *instanceState, time_t backup_id,
 			  pgSetBackupParams *set_backup_params)
 {
 	pgBackup	*target_backup = NULL;
@@ -2046,7 +2045,7 @@ do_set_backup(const char *instance_name, time_t backup_id,
 	if (!set_backup_params)
 		elog(ERROR, "Nothing to set by 'set-backup' command");
 
-	backup_list = catalog_get_backup_list(instance_name, backup_id);
+	backup_list = catalog_get_backup_list(instanceState, backup_id);
 	if (parray_num(backup_list) != 1)
 		elog(ERROR, "Failed to find backup %s", base36enc(backup_id));
 

src/configure.c

@@ -374,7 +374,7 @@ init_config(InstanceConfig *config, const char *instance_name)
  * read instance config from file
  */
 InstanceConfig *
-readInstanceConfigFile(const char *instance_name)
+readInstanceConfigFile(InstanceState *instanceState)
 {
 	char	path[MAXPGPATH];
 	InstanceConfig   *instance = pgut_new(InstanceConfig);
@@ -592,14 +592,14 @@ readInstanceConfigFile(const char *instance_name)
 	};
 
 
-	init_config(instance, instance_name);
+	init_config(instance, instanceState->instance_name);
 
 	sprintf(instance->backup_instance_path, "%s/%s/%s",
-			backup_path, BACKUPS_DIR, instance_name);
+			instanceState->catalog_state->catalog_path, BACKUPS_DIR, instanceState->instance_name);
 	canonicalize_path(instance->backup_instance_path);
 
 	sprintf(instance->arclog_path, "%s/%s/%s",
-			backup_path, "wal", instance_name);
+			instanceState->catalog_state->catalog_path, "wal", instanceState->instance_name);
 	canonicalize_path(instance->arclog_path);
 
 	join_path_components(path, instance->backup_instance_path,

src/delete.c

@@ -21,7 +21,7 @@ static void do_retention_internal(parray *backup_list, parray *to_keep_list,
 static void do_retention_merge(InstanceState *instanceState, parray *backup_list,
 							   parray *to_keep_list, parray *to_purge_list);
 static void do_retention_purge(parray *to_keep_list, parray *to_purge_list);
-static void do_retention_wal(bool dry_run);
+static void do_retention_wal(InstanceState *instanceState, bool dry_run);
 
 // TODO: more useful messages for dry run.
 static bool backup_deleted = false;   /* At least one backup was deleted */
@@ -39,7 +39,7 @@ do_delete(InstanceState *instanceState, time_t backup_id)
 	char		size_to_delete_pretty[20];
 
 	/* Get complete list of backups */
-	backup_list = catalog_get_backup_list(instanceState->instance_name, INVALID_BACKUP_ID);
+	backup_list = catalog_get_backup_list(instanceState, INVALID_BACKUP_ID);
 
 	delete_list = parray_new();
 
@@ -105,7 +105,7 @@ do_delete(InstanceState *instanceState, time_t backup_id)
 
 	/* Clean WAL segments */
 	if (delete_wal)
-		do_retention_wal(dry_run);
+		do_retention_wal(instanceState, dry_run);
 
 	/* cleanup */
 	parray_free(delete_list);
@@ -139,7 +139,7 @@ void do_retention(InstanceState *instanceState)
 	MyLocation = FIO_LOCAL_HOST;
 
 	/* Get a complete list of backups. */
-	backup_list = catalog_get_backup_list(instanceState->instance_name, INVALID_BACKUP_ID);
+	backup_list = catalog_get_backup_list(instanceState, INVALID_BACKUP_ID);
 
 	if (parray_num(backup_list) == 0)
 		backup_list_is_empty = true;
@@ -179,7 +179,7 @@ void do_retention(InstanceState *instanceState)
 
 	/* TODO: some sort of dry run for delete_wal */
 	if (delete_wal)
-		do_retention_wal(dry_run);
+		do_retention_wal(instanceState, dry_run);
 
 	/* TODO: consider dry-run flag */
 
@@ -653,12 +653,13 @@ do_retention_purge(parray *to_keep_list, parray *to_purge_list)
  * and delete them.
  */
 static void
-do_retention_wal(bool dry_run)
+do_retention_wal(InstanceState *instanceState, bool dry_run)
 {
 	parray 		*tli_list;
 	int i;
 
-	tli_list = catalog_get_timelines(&instance_config);
+	//TODO check that instanceState is not NULL
+	tli_list = catalog_get_timelines(instanceState, &instance_config);
 
 	for (i = 0; i < parray_num(tli_list); i++)
 	{
@@ -975,7 +976,7 @@ do_delete_instance(InstanceState *instanceState)
 
 
 	/* Delete all backups. */
-	backup_list = catalog_get_backup_list(instanceState->instance_name, INVALID_BACKUP_ID);
+	backup_list = catalog_get_backup_list(instanceState, INVALID_BACKUP_ID);
 
 	catalog_lock_backup_list(backup_list, 0, parray_num(backup_list) - 1, true, true);
 
@@ -1015,7 +1016,7 @@ do_delete_instance(InstanceState *instanceState)
 
 /* Delete all backups of given status in instance */
 void
-do_delete_status(InstanceConfig *instance_config, const char *status)
+do_delete_status(InstanceState *instanceState, InstanceConfig *instance_config, const char *status)
 {
 	int         i;
 	parray     *backup_list, *delete_list;
@@ -1038,11 +1039,11 @@ do_delete_status(InstanceConfig *instance_config, const char *status)
 	 */
 	pretty_status = status2str(status_for_delete);
 
-	backup_list = catalog_get_backup_list(instance_config->name, INVALID_BACKUP_ID);
+	backup_list = catalog_get_backup_list(instanceState, INVALID_BACKUP_ID);
 
 	if (parray_num(backup_list) == 0)
 	{
-		elog(WARNING, "Instance '%s' has no backups", instance_config->name);
+		elog(WARNING, "Instance '%s' has no backups", instanceState->instance_name);
 		return;
 	}
 

src/merge.c

@@ -86,7 +86,7 @@ do_merge(InstanceState *instanceState, time_t backup_id)
 	elog(INFO, "Merge started");
 
 	/* Get list of all backups sorted in order of descending start time */
-	backups = catalog_get_backup_list(instanceState->instance_name, INVALID_BACKUP_ID);
+	backups = catalog_get_backup_list(instanceState, INVALID_BACKUP_ID);
 
 	/* Find destination backup first */
 	for (i = 0; i < parray_num(backups); i++)

src/pg_probackup.c

@@ -63,7 +63,7 @@ const char  *PROGRAM_EMAIL = "https://github.com/postgrespro/pg_probackup/issues
 /* ================ catalogState =========== */
 /* directory options */
 /* TODO make it local variable, pass as an argument to all commands that need it.  */
-char	   *backup_path = NULL;
+static char	   *backup_path = NULL;
 /*
  * path or to the data files in the backup catalog
  * $BACKUP_PATH/backups/instance_name
@@ -844,7 +844,7 @@ main(int argc, char *argv[])
 						  restore_params,
 						  no_sync);
 		case SHOW_CMD:
-			return do_show(backup_path, instance_name, current.backup_id, show_archive);
+			return do_show(catalogState, instanceState, current.backup_id, show_archive);
 		case DELETE_CMD:
 			if (delete_expired && backup_id_string)
 				elog(ERROR, "You cannot specify --delete-expired and (-i, --backup-id) options together");
@@ -858,7 +858,7 @@ main(int argc, char *argv[])
 			if (!backup_id_string)
 			{
 				if (delete_status)
-					do_delete_status(&instance_config, delete_status);
+					do_delete_status(instanceState, &instance_config, delete_status);
 				else
 					do_retention(instanceState);
 			}
@@ -877,7 +877,7 @@ main(int argc, char *argv[])
 		case SET_BACKUP_CMD:
 			if (!backup_id_string)
 				elog(ERROR, "You must specify parameter (-i, --backup-id) for 'set-backup' command");
-			do_set_backup(instance_name, current.backup_id, set_backup_params);
+			do_set_backup(instanceState, current.backup_id, set_backup_params);
 			break;
 		case CHECKDB_CMD:
 			do_checkdb(need_amcheck,