[Issue #313] restore pg_control last

Author: kulaginm

Diff for: src/restore.c

@@ -3,7 +3,7 @@
  * restore.c: restore DB cluster and archived WAL.
  *
  * Portions Copyright (c) 2009-2013, NIPPON TELEGRAPH AND TELEPHONE CORPORATION
- * Portions Copyright (c) 2015-2019, Postgres Professional
+ * Portions Copyright (c) 2015-2021, Postgres Professional
  *
  *-------------------------------------------------------------------------
  */
@@ -61,6 +61,8 @@ static void create_recovery_conf(InstanceState *instanceState, time_t backup_id,
 								 pgBackup *backup,
 								 pgRestoreParams *params);
 static void *restore_files(void *arg);
+static size_t restore_file(pgFile *dest_file, const char *to_fullpath, bool already_exists, char *out_buf,
+	pgBackup *dest_backup, parray *parent_chain, bool use_bitmap, IncrRestoreMode incremental_mode, XLogRecPtr shift_lsn);
 static void set_orphan_status(parray *backups, pgBackup *parent_backup);
 
 static void restore_chain(pgBackup *dest_backup, parray *parent_chain,
@@ -710,6 +712,8 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain,
 	parray      *pgdata_files = NULL;
 	parray		*dest_files = NULL;
 	parray		*external_dirs = NULL;
+	pgFile	*dest_pg_control_file = NULL;
+	char	dest_pg_control_fullpath[MAXPGPATH];
 	/* arrays with meta info for multi threaded backup */
 	pthread_t  *threads;
 	restore_files_arg *threads_args;
@@ -965,37 +969,61 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain,
 		dest_bytes = dest_backup->pgdata_bytes;
 
 	pretty_size(dest_bytes, pretty_dest_bytes, lengthof(pretty_dest_bytes));
+
+	/*
+	 * [Issue #313]
+	 * find pg_control file (in already sorted earlier dest_files, see parray_qsort(backup->files...))
+	 * and exclude it from list for future special processing
+	 */
+	{
+		int control_file_elem_index;
+		pgFile search_key;
+		MemSet(&search_key, 0, sizeof(pgFile));
+		/* pgFileCompareRelPathWithExternal uses only .rel_path and .external_dir_num for comparision */
+		search_key.rel_path = XLOG_CONTROL_FILE;
+		search_key.external_dir_num = 0;
+		control_file_elem_index = parray_bsearch_index(dest_files, &search_key, pgFileCompareRelPathWithExternal);
+		if(control_file_elem_index < 0)
+			elog(ERROR, "\"%s\" not found in backup %s", XLOG_CONTROL_FILE, base36enc(dest_backup->start_time));
+		dest_pg_control_file = parray_remove(dest_files, control_file_elem_index);
+
+		join_path_components(dest_pg_control_fullpath, pgdata_path, dest_pg_control_file->rel_path);
+		/* remove dest control file before restoring */
+		if (params->incremental_mode != INCR_NONE)
+			fio_unlink(dest_pg_control_fullpath, FIO_DB_HOST);
+	}
+
 	elog(INFO, "Start restoring backup files. PGDATA size: %s", pretty_dest_bytes);
 	time(&start_time);
 	thread_interrupted = false;
 
 	/* Restore files into target directory */
 	for (i = 0; i < num_threads; i++)
 	{
-		restore_files_arg *arg = &(threads_args[i]);
-
-		arg->dest_files = dest_files;
-		arg->pgdata_files = pgdata_files;
-		arg->dest_backup = dest_backup;
-		arg->dest_external_dirs = external_dirs;
-		arg->parent_chain = parent_chain;
-		arg->dbOid_exclude_list = dbOid_exclude_list;
-		arg->skip_external_dirs = params->skip_external_dirs;
-		arg->to_root = pgdata_path;
-		arg->use_bitmap = use_bitmap;
-		arg->incremental_mode = params->incremental_mode;
-		arg->shift_lsn = params->shift_lsn;
-		threads_args[i].restored_bytes = 0;
-		/* By default there are some error */
-		threads_args[i].ret = 1;
+		threads_args[i] = (restore_files_arg){
+			.dest_files = dest_files,
+			.pgdata_files = pgdata_files,
+			.dest_backup = dest_backup,
+			.dest_external_dirs = external_dirs,
+			.parent_chain = parent_chain,
+			.dbOid_exclude_list = dbOid_exclude_list,
+			.skip_external_dirs = params->skip_external_dirs,
+			.to_root = pgdata_path,
+			.use_bitmap = use_bitmap,
+			.incremental_mode = params->incremental_mode,
+			.shift_lsn = params->shift_lsn,
+			.restored_bytes = 0,
+			/* By default there are some error */
+			.ret = 1,
+		};
 
 		/* Useless message TODO: rewrite */
 		elog(LOG, "Start thread %i", i + 1);
 
-		pthread_create(&threads[i], NULL, restore_files, arg);
+		pthread_create(&threads[i], NULL, restore_files, &(threads_args[i]));
 	}
 
-	/* Wait theads */
+	/* Wait threads */
 	for (i = 0; i < num_threads; i++)
 	{
 		pthread_join(threads[i], NULL);
@@ -1005,6 +1033,15 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain,
 		total_bytes += threads_args[i].restored_bytes;
 	}
 
+	/* [Issue #313] copy pg_control at very end */
+	if (restore_isok)
+	{
+		fio_is_remote(FIO_DB_HOST); /* reopen already closed ssh connection */
+		total_bytes += restore_file(dest_pg_control_file, dest_pg_control_fullpath, false, NULL,
+			dest_backup, parent_chain, use_bitmap, params->incremental_mode, params->shift_lsn);
+		fio_disconnect();
+	}
+
 	time(&end_time);
 	pretty_time_interval(difftime(end_time, start_time),
 						 pretty_time, lengthof(pretty_time));
@@ -1066,10 +1103,15 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain,
 			}
 
 			/* TODO: write test for case: file to be synced is missing */
+			/* MKulagin question: where is fio connection reopened? */
 			if (fio_sync(to_fullpath, FIO_DB_HOST) != 0)
 				elog(ERROR, "Failed to sync file \"%s\": %s", to_fullpath, strerror(errno));
 		}
 
+		/* sync control file */
+		if (fio_sync(dest_pg_control_fullpath, FIO_DB_HOST) != 0)
+			elog(ERROR, "Failed to sync file \"%s\": %s", dest_pg_control_fullpath, strerror(errno));
+
 		time(&end_time);
 		pretty_time_interval(difftime(end_time, start_time),
 							 pretty_time, lengthof(pretty_time));
@@ -1089,6 +1131,8 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain,
 		parray_free(pgdata_files);
 	}
 
+	pgFileFree(dest_pg_control_file);
+
 	for (i = parray_num(parent_chain) - 1; i >= 0; i--)
 	{
 		pgBackup   *backup = (pgBackup *) parray_get(parent_chain, i);
@@ -1107,7 +1151,6 @@ restore_files(void *arg)
 	int         i;
 	uint64      n_files;
 	char        to_fullpath[MAXPGPATH];
-	FILE       *out = NULL;
 	char       *out_buf = pgut_malloc(STDIO_BUFSIZE);
 
 	restore_files_arg *arguments = (restore_files_arg *) arg;
@@ -1117,9 +1160,6 @@ restore_files(void *arg)
 	for (i = 0; i < parray_num(arguments->dest_files); i++)
 	{
 		bool     already_exists = false;
-		PageState      *checksum_map = NULL; /* it should take ~1.5MB at most */
-		datapagemap_t  *lsn_map = NULL;      /* it should take 16kB at most */
-		char           *errmsg = NULL;       /* remote agent error message */
 		pgFile	*dest_file = (pgFile *) parray_get(arguments->dest_files, i);
 
 		/* Directories were created before */
@@ -1193,114 +1233,134 @@ restore_files(void *arg)
 			already_exists = true;
 		}
 
-		/*
-		 * Handle incremental restore case for data files.
-		 * If file is already exists in pgdata, then
-		 * we scan it block by block and get
-		 * array of checksums for every page.
-		 */
-		if (already_exists &&
-			dest_file->is_datafile && !dest_file->is_cfs &&
-			dest_file->n_blocks > 0)
+		arguments->restored_bytes += restore_file(dest_file, to_fullpath, already_exists, out_buf,
+			arguments->dest_backup, arguments->parent_chain, arguments->use_bitmap,
+			arguments->incremental_mode, arguments->shift_lsn);
+	}
+
+	free(out_buf);
+
+	/* ssh connection to longer needed */
+	fio_disconnect();
+
+	/* Data files restoring is successful */
+	arguments->ret = 0;
+
+	return NULL;
+}
+
+/*
+ * Restore one file into $PGDATA.
+ */
+static size_t
+restore_file(pgFile *dest_file, const char *to_fullpath, bool already_exists, char *out_buf,
+	pgBackup *dest_backup, parray *parent_chain, bool use_bitmap, IncrRestoreMode incremental_mode, XLogRecPtr shift_lsn)
+{
+	FILE       *out = NULL;
+	size_t		restored_bytes = 0;
+	PageState      *checksum_map = NULL; /* it should take ~1.5MB at most */
+	datapagemap_t  *lsn_map = NULL;      /* it should take 16kB at most */
+	char           *errmsg = NULL;       /* remote agent error message */
+
+	/*
+	 * Handle incremental restore case for data files.
+	 * If file is already exists in pgdata, then
+	 * we scan it block by block and get
+	 * array of checksums for every page.
+	 */
+	if (already_exists &&
+		dest_file->is_datafile && !dest_file->is_cfs &&
+		dest_file->n_blocks > 0)
+	{
+		if (incremental_mode == INCR_LSN)
 		{
-			if (arguments->incremental_mode == INCR_LSN)
-			{
-				lsn_map = fio_get_lsn_map(to_fullpath, arguments->dest_backup->checksum_version,
-								dest_file->n_blocks, arguments->shift_lsn,
-								dest_file->segno * RELSEG_SIZE, FIO_DB_HOST);
-			}
-			else if (arguments->incremental_mode == INCR_CHECKSUM)
-			{
-				checksum_map = fio_get_checksum_map(to_fullpath, arguments->dest_backup->checksum_version,
-													dest_file->n_blocks, arguments->dest_backup->stop_lsn,
-													dest_file->segno * RELSEG_SIZE, FIO_DB_HOST);
-			}
+			lsn_map = fio_get_lsn_map(to_fullpath, dest_backup->checksum_version,
+							dest_file->n_blocks, shift_lsn,
+							dest_file->segno * RELSEG_SIZE, FIO_DB_HOST);
 		}
+		else if (incremental_mode == INCR_CHECKSUM)
+		{
+			checksum_map = fio_get_checksum_map(to_fullpath, dest_backup->checksum_version,
+												dest_file->n_blocks, dest_backup->stop_lsn,
+												dest_file->segno * RELSEG_SIZE, FIO_DB_HOST);
+		}
+	}
 
-		/*
-		 * Open dest file and truncate it to zero, if destination
-		 * file already exists and dest file size is zero, or
-		 * if file do not exist
-		 */
-		if ((already_exists && dest_file->write_size == 0) || !already_exists)
-			out = fio_fopen(to_fullpath, PG_BINARY_W, FIO_DB_HOST);
-		/*
-		 * If file already exists and dest size is not zero,
-		 * then open it for reading and writing.
-		 */
-		else
-			out = fio_fopen(to_fullpath, PG_BINARY_R "+", FIO_DB_HOST);
-
-		if (out == NULL)
-			elog(ERROR, "Cannot open restore target file \"%s\": %s",
-				 to_fullpath, strerror(errno));
+	/*
+	 * Open dest file and truncate it to zero, if destination
+	 * file already exists and dest file size is zero, or
+	 * if file do not exist
+	 */
+	if ((already_exists && dest_file->write_size == 0) || !already_exists)
+		out = fio_fopen(to_fullpath, PG_BINARY_W, FIO_DB_HOST);
+	/*
+	 * If file already exists and dest size is not zero,
+	 * then open it for reading and writing.
+	 */
+	else
+		out = fio_fopen(to_fullpath, PG_BINARY_R "+", FIO_DB_HOST);
 
-		/* update file permission */
-		if (fio_chmod(to_fullpath, dest_file->mode, FIO_DB_HOST) == -1)
-			elog(ERROR, "Cannot change mode of \"%s\": %s", to_fullpath,
-				 strerror(errno));
+	if (out == NULL)
+		elog(ERROR, "Cannot open restore target file \"%s\": %s",
+			 to_fullpath, strerror(errno));
 
-		if (!dest_file->is_datafile || dest_file->is_cfs)
-			elog(VERBOSE, "Restoring nonedata file: \"%s\"", to_fullpath);
-		else
-			elog(VERBOSE, "Restoring data file: \"%s\"", to_fullpath);
+	/* update file permission */
+	if (fio_chmod(to_fullpath, dest_file->mode, FIO_DB_HOST) == -1)
+		elog(ERROR, "Cannot change mode of \"%s\": %s", to_fullpath,
+			 strerror(errno));
 
-		// If destination file is 0 sized, then just close it and go for the next
-		if (dest_file->write_size == 0)
-			goto done;
+	if (!dest_file->is_datafile || dest_file->is_cfs)
+		elog(VERBOSE, "Restoring nonedata file: \"%s\"", to_fullpath);
+	else
+		elog(VERBOSE, "Restoring data file: \"%s\"", to_fullpath);
 
+	// If destination file is 0 sized, then just close it and go for the next
+	if (dest_file->write_size != 0)
+	{
 		/* Restore destination file */
 		if (dest_file->is_datafile && !dest_file->is_cfs)
 		{
 			/* enable stdio buffering for local destination data file */
-			if (!fio_is_remote_file(out))
+			if (!fio_is_remote_file(out) && out_buf != NULL)
 				setvbuf(out, out_buf, _IOFBF, STDIO_BUFSIZE);
 			/* Destination file is data file */
-			arguments->restored_bytes += restore_data_file(arguments->parent_chain,
+			restored_bytes += restore_data_file(parent_chain,
 														   dest_file, out, to_fullpath,
-														   arguments->use_bitmap, checksum_map,
-														   arguments->shift_lsn, lsn_map, true);
+														   use_bitmap, checksum_map,
+														   shift_lsn, lsn_map, true);
 		}
 		else
 		{
 			/* disable stdio buffering for local destination nonedata file */
 			if (!fio_is_remote_file(out))
 				setvbuf(out, NULL, _IONBF, BUFSIZ);
 			/* Destination file is nonedata file */
-			arguments->restored_bytes += restore_non_data_file(arguments->parent_chain,
-										arguments->dest_backup, dest_file, out, to_fullpath,
+			restored_bytes += restore_non_data_file(parent_chain,
+										dest_backup, dest_file, out, to_fullpath,
 										already_exists);
 		}
+	}
 
-done:
-		/* Writing is asynchronous in case of restore in remote mode, so check the agent status */
-		if (fio_check_error_file(out, &errmsg))
-			elog(ERROR, "Cannot write to the remote file \"%s\": %s", to_fullpath, errmsg);
-
-		/* close file */
-		if (fio_fclose(out) != 0)
-			elog(ERROR, "Cannot close file \"%s\": %s", to_fullpath,
-				 strerror(errno));
-
-		/* free pagemap used for restore optimization */
-		pg_free(dest_file->pagemap.bitmap);
-
-		if (lsn_map)
-			pg_free(lsn_map->bitmap);
+	/* Writing is asynchronous in case of restore in remote mode, so check the agent status */
+	if (fio_check_error_file(out, &errmsg))
+		elog(ERROR, "Cannot write to the remote file \"%s\": %s", to_fullpath, errmsg);
 
-		pg_free(lsn_map);
-		pg_free(checksum_map);
-	}
+	/* close file */
+	if (fio_fclose(out) != 0)
+		elog(ERROR, "Cannot close file \"%s\": %s", to_fullpath,
+			 strerror(errno));
 
-	free(out_buf);
+	if (lsn_map)
+		pg_free(lsn_map->bitmap);
 
-	/* ssh connection to longer needed */
-	fio_disconnect();
+	pg_free(lsn_map);
+	pg_free(checksum_map);
 
-	/* Data files restoring is successful */
-	arguments->ret = 0;
+	/* free pagemap used for restore optimization */
+	pg_free(dest_file->pagemap.bitmap);
+	dest_file->pagemap.bitmap = NULL;
 
-	return NULL;
+	return restored_bytes;
 }
 
 /*

Diff for: src/utils/parray.c

@@ -198,6 +198,13 @@ parray_bsearch(parray *array, const void *key, int(*compare)(const void *, const
 	return bsearch(&key, array->data, array->used, sizeof(void *), compare);
 }
 
+int
+parray_bsearch_index(parray *array, const void *key, int(*compare)(const void *, const void *))
+{
+	void **elem = parray_bsearch(array, key, compare);
+	return elem != NULL ? elem - array->data : -1;
+}
+
 /* checks that parray contains element */
 bool parray_contains(parray *array, void *elem)
 {