PGPRO-1918: Remove instance-level lock, introduce backup-level lock

Author: za-arthur

src/backup.c

@@ -943,9 +943,6 @@ do_backup(time_t start_time)
 								   instance_config.master_user);
 	}
 
-	/* Get exclusive lock of backup catalog */
-	catalog_lock();
-
 	/*
 	 * Ensure that backup directory was initialized for the same PostgreSQL
 	 * instance we opened connection to. And that target backup database PGDATA
@@ -955,7 +952,6 @@ do_backup(time_t start_time)
 	if (!is_remote_backup)
 		check_system_identifiers();
 
-
 	/* Start backup. Update backup status. */
 	current.status = BACKUP_STATUS_RUNNING;
 	current.start_time = start_time;
@@ -965,6 +961,7 @@ do_backup(time_t start_time)
 	/* Create backup directory and BACKUP_CONTROL_FILE */
 	if (pgBackupCreateDir(&current))
 		elog(ERROR, "cannot create backup directory");
+	lock_backup(&current);
 	write_backup(&current);
 
 	elog(LOG, "Backup destination is initialized");

src/catalog.c

@@ -21,23 +21,73 @@ static const char *backupModes[] = {"", "PAGE", "PTRACK", "DELTA", "FULL"};
 static pgBackup *readBackupControlFile(const char *path);
 
 static bool exit_hook_registered = false;
-static char lock_file[MAXPGPATH];
+static parray *lock_files = NULL;
 
 static void
 unlink_lock_atexit(void)
 {
-	int			res;
-	res = unlink(lock_file);
-	if (res != 0 && res != ENOENT)
-		elog(WARNING, "%s: %s", lock_file, strerror(errno));
+	int			i;
+
+	if (lock_files == NULL)
+		return;
+
+	for (i = 0; i < parray_num(lock_files); i++)
+	{
+		char	   *lock_file = (char *) parray_get(lock_files, i);
+		int			res;
+
+		res = unlink(lock_file);
+		if (res != 0 && res != ENOENT)
+			elog(WARNING, "%s: %s", lock_file, strerror(errno));
+	}
+
+	parray_walk(lock_files, pfree);
+	parray_free(lock_files);
+	lock_files = NULL;
 }
 
 /*
- * Create a lockfile.
+ * Read backup meta information from BACKUP_CONTROL_FILE.
+ * If no backup matches, return NULL.
+ */
+pgBackup *
+read_backup(time_t timestamp)
+{
+	pgBackup	tmp;
+	char		conf_path[MAXPGPATH];
+
+	tmp.start_time = timestamp;
+	pgBackupGetPath(&tmp, conf_path, lengthof(conf_path), BACKUP_CONTROL_FILE);
+
+	return readBackupControlFile(conf_path);
+}
+
+/*
+ * Save the backup status into BACKUP_CONTROL_FILE.
+ *
+ * We need to reread the backup using its ID and save it changing only its
+ * status.
  */
 void
-catalog_lock(void)
+write_backup_status(pgBackup *backup)
 {
+	pgBackup   *tmp;
+
+	tmp = read_backup(backup->start_time);
+
+	tmp->status = backup->status;
+	write_backup(tmp);
+
+	pgBackupFree(tmp);
+}
+
+/*
+ * Create exclusive lockfile in the backup's directory.
+ */
+void
+lock_backup(pgBackup *backup)
+{
+	char		lock_file[MAXPGPATH];
 	int			fd;
 	char		buffer[MAXPGPATH * 2 + 256];
 	int			ntries;
@@ -46,7 +96,7 @@ catalog_lock(void)
 	pid_t		my_pid,
 				my_p_pid;
 
-	join_path_components(lock_file, backup_instance_path, BACKUP_CATALOG_PID);
+	pgBackupGetPath(backup, lock_file, lengthof(lock_file), BACKUP_CATALOG_PID);
 
 	/*
 	 * If the PID in the lockfile is our own PID or our parent's or
@@ -200,41 +250,11 @@ catalog_lock(void)
 		atexit(unlink_lock_atexit);
 		exit_hook_registered = true;
 	}
-}
 
-/*
- * Read backup meta information from BACKUP_CONTROL_FILE.
- * If no backup matches, return NULL.
- */
-pgBackup *
-read_backup(time_t timestamp)
-{
-	pgBackup	tmp;
-	char		conf_path[MAXPGPATH];
-
-	tmp.start_time = timestamp;
-	pgBackupGetPath(&tmp, conf_path, lengthof(conf_path), BACKUP_CONTROL_FILE);
-
-	return readBackupControlFile(conf_path);
-}
-
-/*
- * Save the backup status into BACKUP_CONTROL_FILE.
- *
- * We need to reread the backup using its ID and save it changing only its
- * status.
- */
-void
-write_backup_status(pgBackup *backup)
-{
-	pgBackup   *tmp;
-
-	tmp = read_backup(backup->start_time);
-
-	tmp->status = backup->status;
-	write_backup(tmp);
-
-	pgBackupFree(tmp);
+	/* Use parray so that the lock files are unlinked in a loop */
+	if (lock_files == NULL)
+		lock_files = parray_new();
+	parray_append(lock_files, pgut_strdup(lock_file));
 }
 
 /*
@@ -381,6 +401,26 @@ catalog_get_backup_list(time_t requested_backup_id)
 	return NULL;
 }
 
+/*
+ * Lock list of backups. Function goes in backward direction.
+ */
+void
+catalog_lock_backup_list(parray *backup_list, int from_idx, int to_idx)
+{
+	int			start_idx,
+				end_idx;
+	int			i;
+
+	if (parray_num(backup_list) == 0)
+		return;
+
+	start_idx = Max(from_idx, to_idx);
+	end_idx = Min(from_idx, to_idx);
+
+	for (i = start_idx; i >= end_idx; i--)
+		lock_backup((pgBackup *) parray_get(backup_list, i));
+}
+
 /*
  * Find the last completed backup on given timeline
  */

src/delete.c

@@ -28,9 +28,6 @@ do_delete(time_t backup_id)
 	XLogRecPtr	oldest_lsn = InvalidXLogRecPtr;
 	TimeLineID	oldest_tli = 0;
 
-	/* Get exclusive lock of backup catalog */
-	catalog_lock();
-
 	/* Get complete list of backups */
 	backup_list = catalog_get_backup_list(INVALID_BACKUP_ID);
 
@@ -76,6 +73,8 @@ do_delete(time_t backup_id)
 		if (parray_num(delete_list) == 0)
 			elog(ERROR, "no backup found, cannot delete");
 
+		catalog_lock_backup_list(delete_list, parray_num(delete_list) - 1, 0);
+
 		/* Delete backups from the end of list */
 		for (i = (int) parray_num(delete_list) - 1; i >= 0; i--)
 		{
@@ -146,9 +145,6 @@ do_retention_purge(void)
 		}
 	}
 
-	/* Get exclusive lock of backup catalog */
-	catalog_lock();
-
 	/* Get a complete list of backups. */
 	backup_list = catalog_get_backup_list(INVALID_BACKUP_ID);
 	if (parray_num(backup_list) == 0)
@@ -206,6 +202,8 @@ do_retention_purge(void)
 				continue;
 			}
 
+			lock_backup(backup);
+
 			/* Delete backup and update status to DELETED */
 			delete_backup_files(backup);
 			backup_deleted = true;
@@ -438,6 +436,8 @@ do_delete_instance(void)
 	/* Delete all backups. */
 	backup_list = catalog_get_backup_list(INVALID_BACKUP_ID);
 
+	catalog_lock_backup_list(backup_list, 0, parray_num(backup_list) - 1);
+
 	for (i = 0; i < parray_num(backup_list); i++)
 	{
 		pgBackup   *backup = (pgBackup *) parray_get(backup_list, i);

src/merge.c

@@ -61,8 +61,6 @@ do_merge(time_t backup_id)
 
 	elog(INFO, "Merge started");
 
-	catalog_lock();
-
 	/* Get list of all backups sorted in order of descending start time */
 	backups = catalog_get_backup_list(INVALID_BACKUP_ID);
 
@@ -125,6 +123,8 @@ do_merge(time_t backup_id)
 
 	Assert(full_backup_idx != dest_backup_idx);
 
+	catalog_lock_backup_list(backups, full_backup_idx, dest_backup_idx);
+
 	/*
 	 * Found target and full backups, merge them and intermediate backups
 	 */

src/pg_probackup.h

@@ -43,7 +43,7 @@
 #define PG_GLOBAL_DIR			"global"
 #define BACKUP_CONTROL_FILE		"backup.control"
 #define BACKUP_CATALOG_CONF_FILE	"pg_probackup.conf"
-#define BACKUP_CATALOG_PID		"pg_probackup.pid"
+#define BACKUP_CATALOG_PID		"backup.pid"
 #define DATABASE_FILE_LIST		"backup_content.control"
 #define PG_BACKUP_LABEL_FILE	"backup_label"
 #define PG_BLACK_LIST			"black_list"
@@ -459,13 +459,15 @@ extern int do_validate_all(void);
 extern pgBackup *read_backup(time_t timestamp);
 extern void write_backup(pgBackup *backup);
 extern void write_backup_status(pgBackup *backup);
+extern void lock_backup(pgBackup *backup);
 
 extern const char *pgBackupGetBackupMode(pgBackup *backup);
 
 extern parray *catalog_get_backup_list(time_t requested_backup_id);
+extern void catalog_lock_backup_list(parray *backup_list, int from_idx,
+									 int to_idx);
 extern pgBackup *catalog_get_last_data_backup(parray *backup_list,
 											  TimeLineID tli);
-extern void catalog_lock(void);
 extern void pgBackupWriteControl(FILE *out, pgBackup *backup);
 extern void write_backup_filelist(pgBackup *backup, parray *files,
 								  const char *root);

src/restore.c

@@ -74,8 +74,6 @@ do_restore_or_validate(time_t target_backup_id, pgRecoveryTarget *rt,
 
 	elog(LOG, "%s begin.", action);
 
-	/* Get exclusive lock of backup catalog */
-	catalog_lock();
 	/* Get list of all backups sorted in order of descending start time */
 	backups = catalog_get_backup_list(INVALID_BACKUP_ID);
 
@@ -300,7 +298,7 @@ do_restore_or_validate(time_t target_backup_id, pgRecoveryTarget *rt,
 
 			if (is_parent(base_full_backup->start_time, tmp_backup, true))
 			{
-
+				lock_backup(tmp_backup);
 				pgBackupValidate(tmp_backup);
 				/* Maybe we should be more paranoid and check for !BACKUP_STATUS_OK? */
 				if (tmp_backup->status == BACKUP_STATUS_CORRUPT)
@@ -388,6 +386,13 @@ do_restore_or_validate(time_t target_backup_id, pgRecoveryTarget *rt,
 				elog(ERROR, "Backup %s was created for version %s which doesn't support recovery_target_lsn",
 						base36enc(dest_backup->start_time), dest_backup->server_version);
 
+			/*
+			 * Backup was locked during validation if no-validate wasn't
+			 * specified.
+			 */
+			if (rt->restore_no_validate)
+				lock_backup(backup);
+
 			restore_backup(backup);
 		}
 

src/validate.c

@@ -53,8 +53,8 @@ pgBackupValidate(pgBackup *backup)
 	int			i;
 
 	/* Check backup version */
-	if (backup->program_version &&
-		parse_program_version(backup->program_version) > parse_program_version(PROGRAM_VERSION))
+	if (parse_program_version(backup->program_version) >
+		parse_program_version(PROGRAM_VERSION))
 		elog(ERROR, "pg_probackup binary version is %s, but backup %s version is %s. "
 			"pg_probackup do not guarantee to be forward compatible. "
 			"Please upgrade pg_probackup binary.",
@@ -356,9 +356,6 @@ do_validate_instance(void)
 
 	elog(INFO, "Validate backups of the instance '%s'", instance_name);
 
-	/* Get exclusive lock of backup catalog */
-	catalog_lock();
-
 	/* Get list of all backups sorted in order of descending start time */
 	backups = catalog_get_backup_list(INVALID_BACKUP_ID);
 
@@ -439,6 +436,7 @@ do_validate_instance(void)
 		else
 			base_full_backup = current_backup;
 
+		lock_backup(current_backup);
 		/* Valiate backup files*/
 		pgBackupValidate(current_backup);
 
@@ -525,6 +523,7 @@ do_validate_instance(void)
 
 						if (backup->status == BACKUP_STATUS_ORPHAN)
 						{
+							lock_backup(backup);
 							/* Revaliate backup files*/
 							pgBackupValidate(backup);