Merge branch 'pgpro-1918'

Author: gsmolk

src/backup.c

@@ -959,9 +959,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
@@ -971,7 +968,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;
@@ -980,7 +976,10 @@ do_backup(time_t start_time)
 
 	/* Create backup directory and BACKUP_CONTROL_FILE */
 	if (pgBackupCreateDir(&current))
-		elog(ERROR, "cannot create backup directory");
+		elog(ERROR, "Cannot create backup directory");
+	if (!lock_backup(&current))
+		elog(ERROR, "Cannot lock backup %s directory",
+			 base36enc(current.start_time));
 	write_backup(&current);
 
 	elog(LOG, "Backup destination is initialized");

src/catalog.c

@@ -21,23 +21,74 @@ 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, BackupStatus status)
+{
+	pgBackup   *tmp;
+
+	tmp = read_backup(backup->start_time);
+
+	backup->status = status;
+	tmp->status = backup->status;
+	write_backup(tmp);
+
+	pgBackupFree(tmp);
+}
+
+/*
+ * Create exclusive lockfile in the backup's directory.
+ */
+bool
+lock_backup(pgBackup *backup)
 {
+	char		lock_file[MAXPGPATH];
 	int			fd;
 	char		buffer[MAXPGPATH * 2 + 256];
 	int			ntries;
@@ -46,7 +97,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
@@ -99,7 +150,7 @@ catalog_lock(void)
 		 * Couldn't create the pid file. Probably it already exists.
 		 */
 		if ((errno != EEXIST && errno != EACCES) || ntries > 100)
-			elog(ERROR, "could not create lock file \"%s\": %s",
+			elog(ERROR, "Could not create lock file \"%s\": %s",
 				 lock_file, strerror(errno));
 
 		/*
@@ -111,22 +162,22 @@ catalog_lock(void)
 		{
 			if (errno == ENOENT)
 				continue;		/* race condition; try again */
-			elog(ERROR, "could not open lock file \"%s\": %s",
+			elog(ERROR, "Could not open lock file \"%s\": %s",
 				 lock_file, strerror(errno));
 		}
 		if ((len = read(fd, buffer, sizeof(buffer) - 1)) < 0)
-			elog(ERROR, "could not read lock file \"%s\": %s",
+			elog(ERROR, "Could not read lock file \"%s\": %s",
 				 lock_file, strerror(errno));
 		close(fd);
 
 		if (len == 0)
-			elog(ERROR, "lock file \"%s\" is empty", lock_file);
+			elog(ERROR, "Lock file \"%s\" is empty", lock_file);
 
 		buffer[len] = '\0';
 		encoded_pid = atoi(buffer);
 
 		if (encoded_pid <= 0)
-			elog(ERROR, "bogus data in lock file \"%s\": \"%s\"",
+			elog(ERROR, "Bogus data in lock file \"%s\": \"%s\"",
 				 lock_file, buffer);
 
 		/*
@@ -140,9 +191,21 @@ catalog_lock(void)
 		 */
 		if (encoded_pid != my_pid && encoded_pid != my_p_pid)
 		{
-			if (kill(encoded_pid, 0) == 0 ||
-				(errno != ESRCH && errno != EPERM))
-				elog(ERROR, "lock file \"%s\" already exists", lock_file);
+			if (kill(encoded_pid, 0) == 0)
+			{
+				elog(WARNING, "Process %d is using backup %s and still is running",
+					 encoded_pid, base36enc(backup->start_time));
+				return false;
+			}
+			else
+			{
+				if (errno == ESRCH)
+					elog(WARNING, "Process %d which used backup %s no longer exists",
+						 encoded_pid, base36enc(backup->start_time));
+				else
+					elog(ERROR, "Failed to send signal 0 to a process %d: %s",
+						encoded_pid, strerror(errno));
+			}
 		}
 
 		/*
@@ -151,7 +214,7 @@ catalog_lock(void)
 		 * would-be creators.
 		 */
 		if (unlink(lock_file) < 0)
-			elog(ERROR, "could not remove old lock file \"%s\": %s",
+			elog(ERROR, "Could not remove old lock file \"%s\": %s",
 				 lock_file, strerror(errno));
 	}
 
@@ -169,7 +232,7 @@ catalog_lock(void)
 		unlink(lock_file);
 		/* if write didn't set errno, assume problem is no disk space */
 		errno = save_errno ? save_errno : ENOSPC;
-		elog(ERROR, "could not write lock file \"%s\": %s",
+		elog(ERROR, "Could not write lock file \"%s\": %s",
 			 lock_file, strerror(errno));
 	}
 	if (fsync(fd) != 0)
@@ -179,7 +242,7 @@ catalog_lock(void)
 		close(fd);
 		unlink(lock_file);
 		errno = save_errno;
-		elog(ERROR, "could not write lock file \"%s\": %s",
+		elog(ERROR, "Could not write lock file \"%s\": %s",
 			 lock_file, strerror(errno));
 	}
 	if (close(fd) != 0)
@@ -188,7 +251,7 @@ catalog_lock(void)
 
 		unlink(lock_file);
 		errno = save_errno;
-		elog(ERROR, "could not write lock file \"%s\": %s",
+		elog(ERROR, "Culd not write lock file \"%s\": %s",
 			 lock_file, strerror(errno));
 	}
 
@@ -200,41 +263,13 @@ 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];
+	/* 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));
 
-	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);
+	return true;
 }
 
 /*
@@ -381,6 +416,31 @@ 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--)
+	{
+		pgBackup   *backup = (pgBackup *) parray_get(backup_list, i);
+		if (!lock_backup(backup))
+			elog(ERROR, "Cannot lock backup %s directory",
+				 base36enc(backup->start_time));
+	}
+}
+
 /*
  * 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,17 @@ do_retention_purge(void)
 				continue;
 			}
 
+			/*
+			 * If the backup still is used do not interrupt go to the next
+			 * backup.
+			 */
+			if (!lock_backup(backup))
+			{
+				elog(WARNING, "Cannot lock backup %s directory, skip purging",
+					 base36enc(backup->start_time));
+				continue;
+			}
+
 			/* Delete backup and update status to DELETED */
 			delete_backup_files(backup);
 			backup_deleted = true;
@@ -240,7 +247,7 @@ do_retention_purge(void)
 	if (backup_deleted)
 		elog(INFO, "Purging finished");
 	else
-		elog(INFO, "Nothing to delete by retention policy");
+		elog(INFO, "There are no backups to delete by retention policy");
 
 	return 0;
 }
@@ -277,8 +284,7 @@ delete_backup_files(pgBackup *backup)
 	 * Update STATUS to BACKUP_STATUS_DELETING in preparation for the case which
 	 * the error occurs before deleting all backup files.
 	 */
-	backup->status = BACKUP_STATUS_DELETING;
-	write_backup_status(backup);
+	write_backup_status(backup, BACKUP_STATUS_DELETING);
 
 	/* list files to be deleted */
 	files = parray_new();
@@ -430,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);