PGPRO-1892: Continue failed merge command

Author: za-arthur

Diff for: src/backup.c

@@ -790,7 +790,7 @@ do_backup_instance(void)
 	}
 
 	/* Print the list of files to backup catalog */
-	pgBackupWriteFileList(&current, backup_files_list, pgdata);
+	write_backup_filelist(&current, backup_files_list, pgdata);
 
 	/* Compute summary of size of regular files in the backup */
 	for (i = 0; i < parray_num(backup_files_list); i++)

Diff for: src/catalog.c

@@ -509,18 +509,22 @@ write_backup(pgBackup *backup)
 	fp = fopen(conf_path, "wt");
 	if (fp == NULL)
 		elog(ERROR, "Cannot open configuration file \"%s\": %s", conf_path,
-			strerror(errno));
+			 strerror(errno));
 
 	pgBackupWriteControl(fp, backup);
 
-	fclose(fp);
+	if (fflush(fp) != 0 ||
+		fsync(fileno(fp)) != 0 ||
+		fclose(fp))
+		elog(ERROR, "Cannot write configuration file \"%s\": %s",
+			 conf_path, strerror(errno));
 }
 
 /*
  * Output the list of files to backup catalog DATABASE_FILE_LIST
  */
 void
-pgBackupWriteFileList(pgBackup *backup, parray *files, const char *root)
+write_backup_filelist(pgBackup *backup, parray *files, const char *root)
 {
 	FILE	   *fp;
 	char		path[MAXPGPATH];
@@ -529,15 +533,15 @@ pgBackupWriteFileList(pgBackup *backup, parray *files, const char *root)
 
 	fp = fopen(path, "wt");
 	if (fp == NULL)
-		elog(ERROR, "cannot open file list \"%s\": %s", path,
+		elog(ERROR, "Cannot open file list \"%s\": %s", path,
 			strerror(errno));
 
 	print_file_list(fp, files, root);
 
 	if (fflush(fp) != 0 ||
 		fsync(fileno(fp)) != 0 ||
 		fclose(fp))
-		elog(ERROR, "cannot write file list \"%s\": %s", path, strerror(errno));
+		elog(ERROR, "Cannot write file list \"%s\": %s", path, strerror(errno));
 }
 
 /*

Diff for: src/data.c

@@ -1037,22 +1037,6 @@ copy_file(const char *from_root, const char *to_root, pgFile *file)
 	return true;
 }
 
-/*
- * Move file from one backup to another.
- * We do not apply compression to these files, because
- * it is either small control file or already compressed cfs file.
- */
-void
-move_file(const char *from_root, const char *to_root, pgFile *file)
-{
-	char		to_path[MAXPGPATH];
-
-	join_path_components(to_path, to_root, file->path + strlen(from_root) + 1);
-	if (rename(file->path, to_path) == -1)
-		elog(ERROR, "Cannot move file \"%s\" to path \"%s\": %s",
-			 file->path, to_path, strerror(errno));
-}
-
 #ifdef HAVE_LIBZ
 /*
  * Show error during work with compressed file

Diff for: src/delete.c

@@ -14,7 +14,6 @@
 #include <time.h>
 #include <unistd.h>
 
-static int delete_backup_files(pgBackup *backup);
 static void delete_walfiles(XLogRecPtr oldest_lsn, TimeLineID oldest_tli,
 							uint32 xlog_seg_size);
 
@@ -245,7 +244,7 @@ do_retention_purge(void)
  * Delete backup files of the backup and update the status of the backup to
  * BACKUP_STATUS_DELETED.
  */
-static int
+void
 delete_backup_files(pgBackup *backup)
 {
 	size_t		i;
@@ -257,11 +256,15 @@ delete_backup_files(pgBackup *backup)
 	 * If the backup was deleted already, there is nothing to do.
 	 */
 	if (backup->status == BACKUP_STATUS_DELETED)
-		return 0;
+	{
+		elog(WARNING, "Backup %s already deleted",
+			 base36enc(backup->start_time));
+		return;
+	}
 
 	time2iso(timestamp, lengthof(timestamp), backup->recovery_time);
 
-	elog(INFO, "delete: %s %s",
+	elog(INFO, "Delete: %s %s",
 		 base36enc(backup->start_time), timestamp);
 
 	/*
@@ -283,25 +286,25 @@ delete_backup_files(pgBackup *backup)
 		pgFile	   *file = (pgFile *) parray_get(files, i);
 
 		/* print progress */
-		elog(VERBOSE, "delete file(%zd/%lu) \"%s\"", i + 1,
+		elog(VERBOSE, "Delete file(%zd/%lu) \"%s\"", i + 1,
 			 (unsigned long) parray_num(files), file->path);
 
 		if (remove(file->path))
 		{
-			elog(WARNING, "can't remove \"%s\": %s", file->path,
-				strerror(errno));
-			parray_walk(files, pgFileFree);
-			parray_free(files);
-
-			return 1;
+			if (errno == ENOENT)
+				elog(VERBOSE, "File \"%s\" is absent", file->path);
+			else
+				elog(ERROR, "Cannot remove \"%s\": %s", file->path,
+					 strerror(errno));
+			return;
 		}
 	}
 
 	parray_walk(files, pgFileFree);
 	parray_free(files);
 	backup->status = BACKUP_STATUS_DELETED;
 
-	return 0;
+	return;
 }
 
 /*

Diff for: src/merge.c

@@ -77,7 +77,8 @@ do_merge(time_t backup_id)
 		{
 			if (backup->status != BACKUP_STATUS_OK &&
 				/* It is possible that previous merging was interrupted */
-				backup->status != BACKUP_STATUS_MERGING)
+				backup->status != BACKUP_STATUS_MERGING &&
+				backup->status != BACKUP_STATUS_DELETING)
 				elog(ERROR, "Backup %s has status: %s",
 					 base36enc(backup->start_time), status2str(backup->status));
 
@@ -164,7 +165,14 @@ merge_backups(pgBackup *to_backup, pgBackup *from_backup)
 	int			i;
 	bool		merge_isok = true;
 
-	elog(LOG, "Merging backup %s with backup %s", from_backup_id, to_backup_id);
+	elog(INFO, "Merging backup %s with backup %s", from_backup_id, to_backup_id);
+
+	/*
+	 * Previous merging was interrupted during deleting source backup. It is
+	 * safe just to delete it again.
+	 */
+	if (from_backup->status == BACKUP_STATUS_DELETING)
+		goto delete_source_backup;
 
 	to_backup->status = BACKUP_STATUS_MERGING;
 	write_backup_status(to_backup);
@@ -244,37 +252,38 @@ merge_backups(pgBackup *to_backup, pgBackup *from_backup)
 		elog(ERROR, "Data files merging failed");
 
 	/*
-	 * Files were copied into to_backup and deleted from from_backup. Remove
-	 * remaining directories from from_backup.
+	 * Update to_backup metadata.
 	 */
-	parray_qsort(files, pgFileComparePathDesc);
+	to_backup->status = BACKUP_STATUS_OK;
+	/* Compute summary of size of regular files in the backup */
+	to_backup->data_bytes = 0;
 	for (i = 0; i < parray_num(files); i++)
 	{
 		pgFile	   *file = (pgFile *) parray_get(files, i);
 
-		if (!S_ISDIR(file->mode))
-			continue;
-
-		if (rmdir(file->path))
-			elog(ERROR, "Could not remove directory \"%s\": %s",
-				 file->path, strerror(errno));
+		if (S_ISDIR(file->mode))
+			to_backup->data_bytes += 4096;
+		/* Count the amount of the data actually copied */
+		else if (S_ISREG(file->mode))
+			to_backup->data_bytes += file->write_size;
 	}
-	if (rmdir(from_database_path))
-		elog(ERROR, "Could not remove directory \"%s\": %s",
-			 from_database_path, strerror(errno));
-	if (unlink(control_file))
-		elog(ERROR, "Could not remove file \"%s\": %s",
-			 control_file, strerror(errno));
+	/* compute size of wal files of this backup stored in the archive */
+	if (!to_backup->stream)
+		to_backup->wal_bytes = xlog_seg_size *
+			(to_backup->stop_lsn / xlog_seg_size -
+			 to_backup->start_lsn / xlog_seg_size + 1);
+	else
+		to_backup->wal_bytes = BYTES_INVALID;
 
-	pgBackupGetPath(from_backup, control_file, lengthof(control_file),
-					BACKUP_CONTROL_FILE);
-	if (unlink(control_file))
-		elog(ERROR, "Could not remove file \"%s\": %s",
-			 control_file, strerror(errno));
+	write_backup_filelist(to_backup, files, from_database_path);
+	write_backup(to_backup);
 
-	if (rmdir(from_backup_path))
-		elog(ERROR, "Could not remove directory \"%s\": %s",
-			 from_backup_path, strerror(errno));
+delete_source_backup:
+	/*
+	 * Files were copied into to_backup. It is time to remove source backup
+	 * entirely.
+	 */
+	delete_backup_files(from_backup);
 
 	/*
 	 * Delete files which are not in from_backup file list.
@@ -286,7 +295,7 @@ merge_backups(pgBackup *to_backup, pgBackup *from_backup)
 		if (parray_bsearch(files, file, pgFileComparePathDesc) == NULL)
 		{
 			pgFileDelete(file);
-			elog(LOG, "Deleted \"%s\"", file->path);
+			elog(VERBOSE, "Deleted \"%s\"", file->path);
 		}
 	}
 
@@ -298,34 +307,14 @@ merge_backups(pgBackup *to_backup, pgBackup *from_backup)
 			 to_backup_path, from_backup_path, strerror(errno));
 
 	/*
-	 * Update to_backup metadata.
+	 * Merging finished, now we can safely update ID of the destination backup.
 	 */
 	pgBackupCopy(to_backup, from_backup);
+	elog(INFO, "to_backup: %s", base36enc(to_backup->start_time));
 	/* Correct metadata */
 	to_backup->backup_mode = BACKUP_MODE_FULL;
 	to_backup->status = BACKUP_STATUS_OK;
 	to_backup->parent_backup = INVALID_BACKUP_ID;
-	/* Compute summary of size of regular files in the backup */
-	to_backup->data_bytes = 0;
-	for (i = 0; i < parray_num(files); i++)
-	{
-		pgFile	   *file = (pgFile *) parray_get(files, i);
-
-		if (S_ISDIR(file->mode))
-			to_backup->data_bytes += 4096;
-		/* Count the amount of the data actually copied */
-		else if (S_ISREG(file->mode))
-			to_backup->data_bytes += file->write_size;
-	}
-	/* compute size of wal files of this backup stored in the archive */
-	if (!to_backup->stream)
-		to_backup->wal_bytes = xlog_seg_size *
-			(to_backup->stop_lsn / xlog_seg_size -
-			 to_backup->start_lsn / xlog_seg_size + 1);
-	else
-		to_backup->wal_bytes = BYTES_INVALID;
-
-	pgBackupWriteFileList(to_backup, files, from_database_path);
 	write_backup(to_backup);
 
 	/* Cleanup */
@@ -508,10 +497,9 @@ merge_files(void *arg)
 				file->write_size = pgFileSize(to_path_tmp);
 				file->crc = pgFileGetCRC(to_path_tmp, false);
 			}
-			pgFileDelete(file);
 		}
 		else
-			move_file(argument->from_root, argument->to_root, file);
+			copy_file(argument->from_root, argument->to_root, file);
 
 		if (file->write_size != BYTES_INVALID)
 			elog(LOG, "Moved file \"%s\": " INT64_FORMAT " bytes",

Diff for: src/pg_probackup.h

@@ -448,6 +448,7 @@ extern int do_show(time_t requested_backup_id);
 
 /* in delete.c */
 extern void do_delete(time_t backup_id);
+extern void delete_backup_files(pgBackup *backup);
 extern int do_retention_purge(void);
 extern int do_delete_instance(void);
 
@@ -478,10 +479,11 @@ 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 pgBackupWriteFileList(pgBackup *backup, parray *files,
+extern void write_backup_filelist(pgBackup *backup, parray *files,
 								  const char *root);
 
-extern void pgBackupGetPath(const pgBackup *backup, char *path, size_t len, const char *subdir);
+extern void pgBackupGetPath(const pgBackup *backup, char *path, size_t len,
+							const char *subdir);
 extern void pgBackupGetPath2(const pgBackup *backup, char *path, size_t len,
 							 const char *subdir1, const char *subdir2);
 extern int pgBackupCreateDir(pgBackup *backup);
@@ -543,7 +545,6 @@ extern void restore_data_file(const char *to_path,
 							  bool write_header,
 							  uint32 backup_version);
 extern bool copy_file(const char *from_root, const char *to_root, pgFile *file);
-extern void move_file(const char *from_root, const char *to_root, pgFile *file);
 extern void push_wal_file(const char *from_path, const char *to_path,
 						  bool is_compress, bool overwrite);
 extern void get_wal_file(const char *from_path, const char *to_path);

Diff for: tests/merge.py

@@ -602,7 +602,7 @@ def test_continue_failed_merge(self):
 
         gdb = self.merge_backup(backup_dir, "node", backup_id, gdb=True)
 
-        gdb.set_breakpoint('move_file')
+        gdb.set_breakpoint('copy_file')
         gdb.run_until_break()
 
         if gdb.continue_execution_until_break(20) != 'breakpoint-hit':
@@ -615,3 +615,10 @@ def test_continue_failed_merge(self):
 
         # Try to continue failed MERGE
         self.merge_backup(backup_dir, "node", backup_id)
+
+        # Drop node and restore it
+        node.cleanup()
+        self.restore_node(backup_dir, 'node', node)
+
+        # Clean after yourself
+        self.del_test_dir(module_name, fname)