[Issue #183] If backup deletion failed to lock backup due to "out of space" condition, then treat backup as locked and carry on.

Author: gsmolk

src/backup.c

@@ -230,7 +230,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync)
 		prev_backup_start_lsn = prev_backup->start_lsn;
 		current.parent_backup = prev_backup->start_time;
 
-		write_backup(&current);
+		write_backup(&current, true);
 	}
 
 	/*
@@ -287,7 +287,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync)
 				base36enc(prev_backup->start_time));
 
 	/* Update running backup meta with START LSN */
-	write_backup(&current);
+	write_backup(&current, true);
 
 	pgBackupGetPath(&current, database_path, lengthof(database_path),
 					DATABASE_DIR);
@@ -496,7 +496,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync)
 	/* write initial backup_content.control file and update backup.control  */
 	write_backup_filelist(&current, backup_files_list,
 						  instance_config.pgdata, external_dirs);
-	write_backup(&current);
+	write_backup(&current, true);
 
 	/* init thread args with own file lists */
 	threads = (pthread_t *) palloc(sizeof(pthread_t) * num_threads);
@@ -661,7 +661,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync)
 	write_backup_filelist(&current, backup_files_list, instance_config.pgdata,
 						  external_dirs);
 	/* update backup control file to update size info */
-	write_backup(&current);
+	write_backup(&current, true);
 
 	/* Sync all copied files unless '--no-sync' flag is used */
 	if (no_sync)
@@ -840,10 +840,10 @@ do_backup(time_t start_time, bool no_validate,
 	/* Create backup directory and BACKUP_CONTROL_FILE */
 	if (pgBackupCreateDir(&current))
 		elog(ERROR, "Cannot create backup directory");
-	if (!lock_backup(&current))
+	if (!lock_backup(&current, true))
 		elog(ERROR, "Cannot lock backup %s directory",
 			 base36enc(current.start_time));
-	write_backup(&current);
+	write_backup(&current, true);
 
 	/* set the error processing function for the backup process */
 	pgut_atexit_push(backup_cleanup, NULL);
@@ -931,7 +931,7 @@ do_backup(time_t start_time, bool no_validate,
 	/* Backup is done. Update backup status */
 	current.end_time = time(NULL);
 	current.status = BACKUP_STATUS_DONE;
-	write_backup(&current);
+	write_backup(&current, true);
 
 	/* Pin backup if requested */
 	if (set_backup_params &&
@@ -2020,7 +2020,7 @@ backup_cleanup(bool fatal, void *userdata)
 			 base36enc(current.start_time));
 		current.end_time = time(NULL);
 		current.status = BACKUP_STATUS_ERROR;
-		write_backup(&current);
+		write_backup(&current, true);
 	}
 }
 
@@ -2065,7 +2065,7 @@ backup_files(void *arg)
 				write_backup_filelist(&current, arguments->files_list, arguments->from_root,
 									  arguments->external_dirs);
 				/* update backup control file to update size info */
-				write_backup(&current);
+				write_backup(&current, true);
 			}
 		}
 

src/catalog.c

@@ -89,14 +89,11 @@ unlink_lock_atexit(void)
  * If no backup matches, return NULL.
  */
 pgBackup *
-read_backup(const char *instance_name, time_t timestamp)
+read_backup(const char *root_dir)
 {
-	pgBackup	tmp;
 	char		conf_path[MAXPGPATH];
 
-	tmp.start_time = timestamp;
-	pgBackupGetPathInInstance(instance_name, &tmp, conf_path,
-					 lengthof(conf_path), BACKUP_CONTROL_FILE, NULL);
+	join_path_components(conf_path, root_dir, BACKUP_CONTROL_FILE);
 
 	return readBackupControlFile(conf_path);
 }
@@ -109,11 +106,11 @@ read_backup(const char *instance_name, time_t timestamp)
  */
 void
 write_backup_status(pgBackup *backup, BackupStatus status,
-					const char *instance_name)
+					const char *instance_name, bool strict)
 {
 	pgBackup   *tmp;
 
-	tmp = read_backup(instance_name, backup->start_time);
+	tmp = read_backup(backup->root_dir);
 	if (!tmp)
 	{
 		/*
@@ -125,7 +122,9 @@ write_backup_status(pgBackup *backup, BackupStatus status,
 
 	backup->status = status;
 	tmp->status = backup->status;
-	write_backup(tmp);
+	tmp->root_dir = pgut_strdup(backup->root_dir);
+
+	write_backup(tmp, strict);
 
 	pgBackupFree(tmp);
 }
@@ -134,7 +133,7 @@ write_backup_status(pgBackup *backup, BackupStatus status,
  * Create exclusive lockfile in the backup's directory.
  */
 bool
-lock_backup(pgBackup *backup)
+lock_backup(pgBackup *backup, bool strict)
 {
 	char		lock_file[MAXPGPATH];
 	int			fd;
@@ -280,6 +279,14 @@ lock_backup(pgBackup *backup)
 		fio_unlink(lock_file, FIO_BACKUP_HOST);
 		/* if write didn't set errno, assume problem is no disk space */
 		errno = save_errno ? save_errno : ENOSPC;
+
+		/* In lax mode if we failed to grab lock because of 'out of space error',
+		 * then treat backup as locked.
+		 * Only delete command should be run in lax mode.
+		 */
+		if (!strict && errno == ENOSPC)
+			return true;
+
 		elog(ERROR, "Could not write lock file \"%s\": %s",
 			 lock_file, strerror(errno));
 	}
@@ -536,7 +543,7 @@ get_backup_filelist(pgBackup *backup)
 	parray		*files = NULL;
 	char		backup_filelist_path[MAXPGPATH];
 
-	pgBackupGetPath(backup, backup_filelist_path, lengthof(backup_filelist_path), DATABASE_FILE_LIST);
+	join_path_components(backup_filelist_path, backup->root_dir, DATABASE_FILE_LIST);
 	files = dir_read_file_list(NULL, NULL, backup_filelist_path, FIO_BACKUP_HOST);
 
 	/* redundant sanity? */
@@ -550,7 +557,7 @@ get_backup_filelist(pgBackup *backup)
  * Lock list of backups. Function goes in backward direction.
  */
 void
-catalog_lock_backup_list(parray *backup_list, int from_idx, int to_idx)
+catalog_lock_backup_list(parray *backup_list, int from_idx, int to_idx, bool strict)
 {
 	int			start_idx,
 				end_idx;
@@ -565,7 +572,7 @@ catalog_lock_backup_list(parray *backup_list, int from_idx, int to_idx)
 	for (i = start_idx; i >= end_idx; i--)
 	{
 		pgBackup   *backup = (pgBackup *) parray_get(backup_list, i);
-		if (!lock_backup(backup))
+		if (!lock_backup(backup, strict))
 			elog(ERROR, "Cannot lock backup %s directory",
 				 base36enc(backup->start_time));
 	}
@@ -837,7 +844,7 @@ pgBackupCreateDir(pgBackup *backup)
 	/* create directories for actual backup files */
 	for (i = 0; i < parray_num(subdirs); i++)
 	{
-		pgBackupGetPath(backup, path, lengthof(path), parray_get(subdirs, i));
+		join_path_components(path, backup->root_dir, parray_get(subdirs, i));
 		fio_mkdir(path, DIR_PERMISSION, FIO_BACKUP_HOST);
 	}
 
@@ -1580,7 +1587,7 @@ pin_backup(pgBackup	*target_backup, pgSetBackupParams *set_backup_params)
 		return;
 
 	/* Update backup.control */
-	write_backup(target_backup);
+	write_backup(target_backup, true);
 
 	if (set_backup_params->ttl > 0 || set_backup_params->expire_time > 0)
 	{
@@ -1630,7 +1637,7 @@ add_note(pgBackup *target_backup, char *note)
 	}
 
 	/* Update backup.control */
-	write_backup(target_backup);
+	write_backup(target_backup, true);
 }
 
 /*
@@ -1735,27 +1742,34 @@ pgBackupWriteControl(FILE *out, pgBackup *backup)
  * Save the backup content into BACKUP_CONTROL_FILE.
  */
 void
-write_backup(pgBackup *backup)
+write_backup(pgBackup *backup, bool strict)
 {
-	FILE	   *fp = NULL;
-	char		path[MAXPGPATH];
-	char		path_temp[MAXPGPATH];
-	int			errno_temp;
+	FILE   *fp = NULL;
+	char    path[MAXPGPATH];
+	char    path_temp[MAXPGPATH];
+	int     errno_temp;
+	char    buf[4096];
 
-	pgBackupGetPath(backup, path, lengthof(path), BACKUP_CONTROL_FILE);
+	join_path_components(path, backup->root_dir, BACKUP_CONTROL_FILE);
 	snprintf(path_temp, sizeof(path_temp), "%s.tmp", path);
 
 	fp = fio_fopen(path_temp, PG_BINARY_W, FIO_BACKUP_HOST);
 	if (fp == NULL)
 		elog(ERROR, "Cannot open configuration file \"%s\": %s",
 			 path_temp, strerror(errno));
 
+	setvbuf(fp, buf, _IOFBF, sizeof(buf));
+
 	pgBackupWriteControl(fp, backup);
 
-	if (fio_fflush(fp) || fio_fclose(fp))
+	if (fio_fclose(fp))
 	{
 		errno_temp = errno;
 		fio_unlink(path_temp, FIO_BACKUP_HOST);
+
+		if (!strict &&  errno_temp == ENOSPC)
+			return;
+
 		elog(ERROR, "Cannot write configuration file \"%s\": %s",
 			 path_temp, strerror(errno_temp));
 	}
@@ -1788,7 +1802,7 @@ write_backup_filelist(pgBackup *backup, parray *files, const char *root,
 	int64 		uncompressed_size_on_disk = 0;
 	int64 		wal_size_on_disk = 0;
 
-	pgBackupGetPath(backup, path, lengthof(path), DATABASE_FILE_LIST);
+	join_path_components(path, backup->root_dir, DATABASE_FILE_LIST);
 	snprintf(path_temp, sizeof(path_temp), "%s.tmp", path);
 
 	out = fio_fopen(path_temp, PG_BINARY_W, FIO_BACKUP_HOST);

src/delete.c

@@ -89,7 +89,7 @@ do_delete(time_t backup_id)
 	if (!dry_run)
 	{
 		/* Lock marked for delete backups */
-		catalog_lock_backup_list(delete_list, parray_num(delete_list) - 1, 0);
+		catalog_lock_backup_list(delete_list, parray_num(delete_list) - 1, 0, false);
 
 		/* Delete backups from the end of list */
 		for (i = (int) parray_num(delete_list) - 1; i >= 0; i--)
@@ -510,7 +510,7 @@ do_retention_merge(parray *backup_list, parray *to_keep_list, parray *to_purge_l
 		parray_rm(to_purge_list, full_backup, pgBackupCompareId);
 
 		/* Lock merge chain */
-		catalog_lock_backup_list(merge_list, parray_num(merge_list) - 1, 0);
+		catalog_lock_backup_list(merge_list, parray_num(merge_list) - 1, 0, true);
 
 		/* Consider this extreme case */
 		//  PAGEa1    PAGEb1   both valid
@@ -627,7 +627,7 @@ do_retention_purge(parray *to_keep_list, parray *to_purge_list)
 			continue;
 
 		/* Actual purge */
-		if (!lock_backup(delete_backup))
+		if (!lock_backup(delete_backup, false))
 		{
 			/* If the backup still is used, do not interrupt and go to the next */
 			elog(WARNING, "Cannot lock backup %s directory, skip purging",
@@ -746,7 +746,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.
 	 */
-	write_backup_status(backup, BACKUP_STATUS_DELETING, instance_name);
+	write_backup_status(backup, BACKUP_STATUS_DELETING, instance_name, false);
 
 	/* list files to be deleted */
 	files = parray_new();
@@ -968,7 +968,7 @@ do_delete_instance(void)
 	/* Delete all backups. */
 	backup_list = catalog_get_backup_list(instance_name, INVALID_BACKUP_ID);
 
-	catalog_lock_backup_list(backup_list, 0, parray_num(backup_list) - 1);
+	catalog_lock_backup_list(backup_list, 0, parray_num(backup_list) - 1, true);
 
 	for (i = 0; i < parray_num(backup_list); i++)
 	{
@@ -1091,7 +1091,7 @@ do_delete_status(InstanceConfig *instance_config, const char *status)
 		if (backup->stream)
 			size_to_delete += backup->wal_bytes;
 
-		if (!dry_run && lock_backup(backup))
+		if (!dry_run && lock_backup(backup, false))
 			delete_backup_files(backup);
 
 		n_deleted++;

src/merge.c

@@ -397,7 +397,7 @@ do_merge(time_t backup_id)
 	parray_append(merge_list, full_backup);
 
 	/* Lock merge chain */
-	catalog_lock_backup_list(merge_list, parray_num(merge_list) - 1, 0);
+	catalog_lock_backup_list(merge_list, parray_num(merge_list) - 1, 0, true);
 
 	/* do actual merge */
 	merge_chain(merge_list, full_backup, dest_backup);
@@ -583,10 +583,10 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 			 */
 			backup->merge_dest_backup = dest_backup->start_time;
 			backup->status = BACKUP_STATUS_MERGING;
-			write_backup(backup);
+			write_backup(backup, true);
 		}
 		else
-			write_backup_status(backup, BACKUP_STATUS_MERGING, instance_name);
+			write_backup_status(backup, BACKUP_STATUS_MERGING, instance_name, true);
 	}
 
 	/* Create directories */
@@ -704,9 +704,13 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 
 	/* If incremental backup is pinned,
 	 * then result FULL backup must also be pinned.
+	 * And reverse, if FULL backup was pinned and dest was not,
+	 * then pinning is no more.
 	 */
-	if (dest_backup->expire_time)
-		full_backup->expire_time = dest_backup->expire_time;
+	full_backup->expire_time = dest_backup->expire_time;
+
+	pg_free(full_backup->note);
+	full_backup->note = NULL;
 
 	if (dest_backup->note)
 		full_backup->note = pgut_strdup(dest_backup->note);
@@ -724,7 +728,7 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 	parray_qsort(result_filelist, pgFileCompareRelPathWithExternal);
 
 	write_backup_filelist(full_backup, result_filelist, full_database_dir, NULL);
-	write_backup(full_backup);
+	write_backup(full_backup, true);
 
 	/* Delete FULL backup files, that do not exists in destination backup
 	 * Both arrays must be sorted in in reversed order to delete from leaf
@@ -760,7 +764,7 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 	 * Files are merged into FULL backup. It is time to remove incremental chain.
 	 */
 	full_backup->status = BACKUP_STATUS_MERGED;
-	write_backup(full_backup);
+	write_backup(full_backup, true);
 
 merge_delete:
 	for (i = parray_num(parent_chain) - 2; i >= 0; i--)
@@ -787,6 +791,10 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 		if (rename(full_backup->root_dir, dest_backup->root_dir) == -1)
 			elog(ERROR, "Could not rename directory \"%s\" to \"%s\": %s",
 				 full_backup->root_dir, dest_backup->root_dir, strerror(errno));
+
+		/* update root_dir after rename */
+		pg_free(full_backup->root_dir);
+		full_backup->root_dir = pgut_strdup(dest_backup->root_dir);
 	}
 	else
 	{
@@ -804,6 +812,10 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 		if (rename(full_backup->root_dir, destination_path) == -1)
 			elog(ERROR, "Could not rename directory \"%s\" to \"%s\": %s",
 				 full_backup->root_dir, destination_path, strerror(errno));
+
+		/* update root_dir after rename */
+		pg_free(full_backup->root_dir);
+		full_backup->root_dir = pgut_strdup(destination_path);
 	}
 
 	/* If we crash here, it will produce full backup in MERGED
@@ -821,7 +833,7 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 	full_backup->status = BACKUP_STATUS_OK;
 	full_backup->start_time = full_backup->merge_dest_backup;
 	full_backup->merge_dest_backup = INVALID_BACKUP_ID;
-	write_backup(full_backup);
+	write_backup(full_backup, true);
 	/* Critical section end */
 
 	/* Cleanup */