[Issue #324] "--no-validate" and "--no-sync" flags for merge and delete commands

Author: gsmolk

src/backup.c

@@ -887,7 +887,7 @@ do_backup(pgSetBackupParams *set_backup_params,
 	 * which are expired according to retention policies
 	 */
 	if (delete_expired || merge_expired || delete_wal)
-		do_retention();
+		do_retention(no_validate, no_sync);
 
 	return 0;
 }

src/delete.c

@@ -19,7 +19,7 @@ static void delete_walfiles_in_tli(XLogRecPtr keep_lsn, timelineInfo *tli,
 static void do_retention_internal(parray *backup_list, parray *to_keep_list,
 									parray *to_purge_list);
 static void do_retention_merge(parray *backup_list, parray *to_keep_list,
-									parray *to_purge_list);
+									parray *to_purge_list, bool no_validate, bool no_sync);
 static void do_retention_purge(parray *to_keep_list, parray *to_purge_list);
 static void do_retention_wal(bool dry_run);
 
@@ -123,7 +123,7 @@ do_delete(time_t backup_id)
  * which FULL backup should be keeped for redundancy obligation(only valid do),
  * but if invalid backup is not guarded by retention - it is removed
  */
-void do_retention(void)
+void do_retention(bool no_validate, bool no_sync)
 {
 	parray	   *backup_list = NULL;
 	parray	   *to_keep_list = parray_new();
@@ -172,7 +172,7 @@ void do_retention(void)
 		do_retention_internal(backup_list, to_keep_list, to_purge_list);
 
 	if (merge_expired && !dry_run && !backup_list_is_empty)
-		do_retention_merge(backup_list, to_keep_list, to_purge_list);
+		do_retention_merge(backup_list, to_keep_list, to_purge_list, no_validate, no_sync);
 
 	if (delete_expired && !dry_run && !backup_list_is_empty)
 		do_retention_purge(to_keep_list, to_purge_list);
@@ -424,7 +424,8 @@ do_retention_internal(parray *backup_list, parray *to_keep_list, parray *to_purg
 
 /* Merge partially expired incremental chains */
 static void
-do_retention_merge(parray *backup_list, parray *to_keep_list, parray *to_purge_list)
+do_retention_merge(parray *backup_list, parray *to_keep_list, parray *to_purge_list,
+				   bool no_validate, bool no_sync)
 {
 	int i;
 	int j;
@@ -543,7 +544,7 @@ do_retention_merge(parray *backup_list, parray *to_keep_list, parray *to_purge_l
 		 */
 
 		keep_backup = parray_get(merge_list, 0);
-		merge_chain(merge_list, full_backup, keep_backup);
+		merge_chain(merge_list, full_backup, keep_backup, no_validate, no_sync);
 		backup_merged = true;
 
 		for (j = parray_num(merge_list) - 2; j >= 0; j--)
@@ -554,8 +555,8 @@ do_retention_merge(parray *backup_list, parray *to_keep_list, parray *to_purge_l
 			parray_rm(to_purge_list, tmp_backup, pgBackupCompareId);
 			parray_set(to_keep_list, i, NULL);
 		}
-
-		pgBackupValidate(full_backup, NULL);
+		if (!no_validate)
+			pgBackupValidate(full_backup, NULL);
 		if (full_backup->status == BACKUP_STATUS_CORRUPT)
 			elog(ERROR, "Merging of backup %s failed", base36enc(full_backup->start_time));
 

src/help.c

@@ -197,11 +197,12 @@ help_pg_probackup(void)
 	printf(_("                 [--wal-depth=wal-depth]\n"));
 	printf(_("                 [-i backup-id | --delete-expired | --merge-expired | --status=backup_status]\n"));
 	printf(_("                 [--delete-wal]\n"));
-	printf(_("                 [--dry-run]\n"));
+	printf(_("                 [--dry-run] [--no-validate] [--no-sync]\n"));
 	printf(_("                 [--help]\n"));
 
 	printf(_("\n  %s merge -B backup-path --instance=instance_name\n"), PROGRAM_NAME);
 	printf(_("                 -i backup-id [--progress] [-j num-threads]\n"));
+	printf(_("                 [--no-validate] [--no-sync]\n"));
 	printf(_("                 [--help]\n"));
 
 	printf(_("\n  %s add-instance -B backup-path -D pgdata-path\n"), PROGRAM_NAME);
@@ -631,13 +632,16 @@ help_delete(void)
 	printf(_("                 [-j num-threads] [--progress]\n"));
 	printf(_("                 [--retention-redundancy=retention-redundancy]\n"));
 	printf(_("                 [--retention-window=retention-window]\n"));
-	printf(_("                 [--wal-depth=wal-depth]\n\n"));
+	printf(_("                 [--wal-depth=wal-depth]\n"));
+	printf(_("                 [--no-validate] [--no-sync]\n\n"));
 
 	printf(_("  -B, --backup-path=backup-path    location of the backup storage area\n"));
 	printf(_("      --instance=instance_name     name of the instance\n"));
 	printf(_("  -i, --backup-id=backup-id        backup to delete\n"));
 	printf(_("  -j, --threads=NUM                number of parallel threads\n"));
 	printf(_("      --progress                   show progress\n"));
+	printf(_("      --no-validate                disable validation during retention merge\n"));
+	printf(_("      --no-sync                    do not sync merged files to disk\n"));
 
 	printf(_("\n  Retention options:\n"));
 	printf(_("      --delete-expired             delete backups expired according to current\n"));
@@ -681,6 +685,7 @@ help_merge(void)
 {
 	printf(_("\n%s merge -B backup-path --instance=instance_name\n"), PROGRAM_NAME);
 	printf(_("                 -i backup-id [-j num-threads] [--progress]\n"));
+	printf(_("                 [--no-validate] [--no-sync]\n"));
 	printf(_("                 [--log-level-console=log-level-console]\n"));
 	printf(_("                 [--log-level-file=log-level-file]\n"));
 	printf(_("                 [--log-filename=log-filename]\n"));
@@ -695,6 +700,8 @@ help_merge(void)
 
 	printf(_("  -j, --threads=NUM                number of parallel threads\n"));
 	printf(_("      --progress                   show progress\n"));
+	printf(_("      --no-validate                disable validation during retention merge\n"));
+	printf(_("      --no-sync                    do not sync merged files to disk\n"));
 
 	printf(_("\n  Logging options:\n"));
 	printf(_("      --log-level-console=log-level-console\n"));

src/merge.c

@@ -30,6 +30,7 @@ typedef struct
 	bool		program_version_match;
 	bool        use_bitmap;
 	bool        is_retry;
+	bool        no_sync;
 
 	/*
 	 * Return value from the thread.
@@ -50,13 +51,13 @@ static void
 merge_data_file(parray *parent_chain, pgBackup *full_backup,
 				pgBackup *dest_backup, pgFile *dest_file,
 				pgFile *tmp_file, const char *to_root, bool use_bitmap,
-				bool is_retry);
+				bool is_retry, bool no_sync);
 
 static void
 merge_non_data_file(parray *parent_chain, pgBackup *full_backup,
 				pgBackup *dest_backup, pgFile *dest_file,
 				pgFile *tmp_file, const char *full_database_dir,
-				const char *full_external_prefix);
+				const char *full_external_prefix, bool no_sync);
 
 static bool is_forward_compatible(parray *parent_chain);
 
@@ -68,7 +69,7 @@ static bool is_forward_compatible(parray *parent_chain);
  * - Remove unnecessary files, which doesn't exist in the target backup anymore
  */
 void
-do_merge(time_t backup_id)
+do_merge(time_t backup_id, bool no_validate, bool no_sync)
 {
 	parray	   *backups;
 	parray	   *merge_list = parray_new();
@@ -405,9 +406,10 @@ do_merge(time_t backup_id)
 	catalog_lock_backup_list(merge_list, parray_num(merge_list) - 1, 0, true, true);
 
 	/* do actual merge */
-	merge_chain(merge_list, full_backup, dest_backup);
+	merge_chain(merge_list, full_backup, dest_backup, no_validate, no_sync);
 
-	pgBackupValidate(full_backup, NULL);
+	if (!no_validate)
+		pgBackupValidate(full_backup, NULL);
 	if (full_backup->status == BACKUP_STATUS_CORRUPT)
 		elog(ERROR, "Merging of backup %s failed", base36enc(backup_id));
 
@@ -434,7 +436,8 @@ do_merge(time_t backup_id)
  * that chain is ok.
  */
 void
-merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
+merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup,
+			bool no_validate, bool no_sync)
 {
 	int			i;
 	char 		*dest_backup_id;
@@ -554,25 +557,28 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 	 * with sole exception of FULL backup. If it has MERGING status
 	 * then it isn't valid backup until merging is finished.
 	 */
-	elog(INFO, "Validate parent chain for backup %s",
-					base36enc(dest_backup->start_time));
-
-	for (i = parray_num(parent_chain) - 1; i >= 0; i--)
+	if (!no_validate)
 	{
-		pgBackup   *backup = (pgBackup *) parray_get(parent_chain, i);
+		elog(INFO, "Validate parent chain for backup %s",
+						base36enc(dest_backup->start_time));
 
-		/* FULL backup is not to be validated if its status is MERGING */
-		if (backup->backup_mode == BACKUP_MODE_FULL &&
-			backup->status == BACKUP_STATUS_MERGING)
+		for (i = parray_num(parent_chain) - 1; i >= 0; i--)
 		{
-			continue;
-		}
+			pgBackup   *backup = (pgBackup *) parray_get(parent_chain, i);
 
-		pgBackupValidate(backup, NULL);
+			/* FULL backup is not to be validated if its status is MERGING */
+			if (backup->backup_mode == BACKUP_MODE_FULL &&
+				backup->status == BACKUP_STATUS_MERGING)
+			{
+				continue;
+			}
 
-		if (backup->status != BACKUP_STATUS_OK)
-			elog(ERROR, "Backup %s has status %s, merge is aborted",
-				base36enc(backup->start_time), status2str(backup->status));
+			pgBackupValidate(backup, NULL);
+
+			if (backup->status != BACKUP_STATUS_OK)
+				elog(ERROR, "Backup %s has status %s, merge is aborted",
+					base36enc(backup->start_time), status2str(backup->status));
+		}
 	}
 
 	/*
@@ -665,6 +671,7 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 		arg->program_version_match = program_version_match;
 		arg->use_bitmap = use_bitmap;
 		arg->is_retry = is_retry;
+		arg->no_sync = no_sync;
 		/* By default there are some error */
 		arg->ret = 1;
 
@@ -1102,14 +1109,16 @@ merge_files(void *arg)
 							dest_file, tmp_file,
 							arguments->full_database_dir,
 							arguments->use_bitmap,
-							arguments->is_retry);
+							arguments->is_retry,
+							arguments->no_sync);
 		else
 			merge_non_data_file(arguments->parent_chain,
 								arguments->full_backup,
 								arguments->dest_backup,
 								dest_file, tmp_file,
 								arguments->full_database_dir,
-								arguments->full_external_prefix);
+								arguments->full_external_prefix,
+								arguments->no_sync);
 
 done:
 		parray_append(arguments->merge_filelist, tmp_file);
@@ -1202,7 +1211,8 @@ reorder_external_dirs(pgBackup *to_backup, parray *to_external,
 void
 merge_data_file(parray *parent_chain, pgBackup *full_backup,
 				pgBackup *dest_backup, pgFile *dest_file, pgFile *tmp_file,
-				const char *full_database_dir, bool use_bitmap, bool is_retry)
+				const char *full_database_dir, bool use_bitmap, bool is_retry,
+				bool no_sync)
 {
 	FILE   *out = NULL;
 	char   *buffer = pgut_malloc(STDIO_BUFSIZE);
@@ -1273,7 +1283,7 @@ merge_data_file(parray *parent_chain, pgBackup *full_backup,
 		return;
 
 	/* sync second temp file to disk */
-	if (fio_sync(to_fullpath_tmp2, FIO_BACKUP_HOST) != 0)
+	if (!no_sync && fio_sync(to_fullpath_tmp2, FIO_BACKUP_HOST) != 0)
 		elog(ERROR, "Cannot sync merge temp file \"%s\": %s",
 			to_fullpath_tmp2, strerror(errno));
 
@@ -1294,7 +1304,8 @@ merge_data_file(parray *parent_chain, pgBackup *full_backup,
 void
 merge_non_data_file(parray *parent_chain, pgBackup *full_backup,
 				pgBackup *dest_backup, pgFile *dest_file, pgFile *tmp_file,
-				const char *full_database_dir, const char *to_external_prefix)
+				const char *full_database_dir, const char *to_external_prefix,
+				bool no_sync)
 {
 	int		i;
 	char	to_fullpath[MAXPGPATH];
@@ -1378,7 +1389,7 @@ merge_non_data_file(parray *parent_chain, pgBackup *full_backup,
 						 to_fullpath_tmp, BACKUP_MODE_FULL, 0, false);
 
 	/* sync temp file to disk */
-	if (fio_sync(to_fullpath_tmp, FIO_BACKUP_HOST) != 0)
+	if (!no_sync && fio_sync(to_fullpath_tmp, FIO_BACKUP_HOST) != 0)
 		elog(ERROR, "Cannot sync merge temp file \"%s\": %s",
 			to_fullpath_tmp, strerror(errno));
 

src/pg_probackup.c

@@ -128,10 +128,6 @@ bool 		compress_shortcut = false;
 /* other options */
 char	   *instance_name;
 
-/* TODO: quick hack */
-bool merge_no_validate = false;
-bool merge_no_sync = false;
-
 /* archive push options */
 int		batch_size = 1;
 static char *wal_file_path;
@@ -834,8 +830,6 @@ main(int argc, char *argv[])
 		case SHOW_CMD:
 			return do_show(instance_name, current.backup_id, show_archive);
 		case DELETE_CMD:
-			merge_no_validate = no_validate;
-			merge_no_sync = no_sync;
 
 			if (delete_expired && backup_id_string)
 				elog(ERROR, "You cannot specify --delete-expired and (-i, --backup-id) options together");
@@ -851,15 +845,13 @@ main(int argc, char *argv[])
 				if (delete_status)
 					do_delete_status(&instance_config, delete_status);
 				else
-					do_retention();
+					do_retention(no_validate, no_sync);
 			}
 			else
 					do_delete(current.backup_id);
 			break;
 		case MERGE_CMD:
-			merge_no_validate = no_validate;
-			merge_no_sync = no_sync;
-			do_merge(current.backup_id);
+			do_merge(current.backup_id, no_validate, no_sync);
 			break;
 		case SHOW_CONFIG_CMD:
 			do_show_config();

src/pg_probackup.h

@@ -786,10 +786,6 @@ extern bool		compress_shortcut;
 /* other options */
 extern char *instance_name;
 
-/* temp merge options */
-extern bool merge_no_validate;
-extern bool merge_no_sync;
-
 /* show options */
 extern ShowFormat show_format;
 
@@ -843,10 +839,10 @@ extern parray *read_timeline_history(const char *arclog_path, TimeLineID targetT
 extern bool tliIsPartOfHistory(const parray *timelines, TimeLineID tli);
 
 /* in merge.c */
-extern void do_merge(time_t backup_id);
+extern void do_merge(time_t backup_id, bool no_validate, bool no_sync);
 extern void merge_backups(pgBackup *backup, pgBackup *next_backup);
-extern void merge_chain(parray *parent_chain,
-						pgBackup *full_backup, pgBackup *dest_backup);
+extern void merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup,
+						bool no_validate, bool no_sync);
 
 extern parray *read_database_map(pgBackup *backup);
 
@@ -873,7 +869,7 @@ extern int do_show(const char *instance_name, time_t requested_backup_id, bool s
 /* in delete.c */
 extern void do_delete(time_t backup_id);
 extern void delete_backup_files(pgBackup *backup);
-extern void do_retention(void);
+extern void do_retention(bool no_validate, bool no_sync);
 extern int do_delete_instance(void);
 extern void do_delete_status(InstanceConfig *instance_config, const char *status);
 

src/utils/file.c

@@ -1169,9 +1169,6 @@ int fio_sync(char const* path, fio_location location)
 	{
 		int fd;
 
-		if (merge_no_sync)
-			return 0;
-
 		fd = open(path, O_WRONLY | PG_BINARY, FILE_PERMISSIONS);
 		if (fd < 0)
 			return -1;

src/validate.c

@@ -129,9 +129,6 @@ pgBackupValidate(pgBackup *backup, pgRestoreParams *params)
 //		dbOid_exclude_list = get_dbOid_exclude_list(backup, files, params->partial_db_list,
 //														params->partial_restore_type);
 
-	if (merge_no_validate)
-		goto skip_validation;
-
 	/* setup threads */
 	for (i = 0; i < parray_num(files); i++)
 	{
@@ -183,8 +180,6 @@ pgBackupValidate(pgBackup *backup, pgRestoreParams *params)
 	pfree(threads);
 	pfree(threads_args);
 
-skip_validation:
-
 	/* cleanup */
 	parray_walk(files, pgFileFree);
 	parray_free(files);