[PGPRO-5421] rewrite --wal-file-path logic and tests

kulaginm · kulaginm · commit a9f3e806cc96 · 2021-11-16T02:36:19.000+03:00
diff --git a/.travis.yml b/.travis.yml
@@ -46,7 +46,6 @@ env:
 #    - PG_VERSION=13 PG_BRANCH=REL_13_STABLE PTRACK_PATCH_PG_BRANCH=REL_13_STABLE MODE=replica
 #    - PG_VERSION=13 PG_BRANCH=REL_13_STABLE PTRACK_PATCH_PG_BRANCH=off MODE=retention
 #    - PG_VERSION=13 PG_BRANCH=REL_13_STABLE PTRACK_PATCH_PG_BRANCH=REL_13_STABLE MODE=restore
-    - PG_VERSION=15 PG_BRANCH=master
     - PG_VERSION=14 PG_BRANCH=REL_14_STABLE MODE=archive
     - PG_VERSION=13 PG_BRANCH=REL_13_STABLE MODE=archive
     - PG_VERSION=12 PG_BRANCH=REL_12_STABLE MODE=archive
diff --git a/src/archive.c b/src/archive.c
@@ -3,7 +3,7 @@
  * archive.c: -  pg_probackup specific archive commands for archive backups.
  *
  *
- * Portions Copyright (c) 2018-2019, Postgres Professional
+ * Portions Copyright (c) 2018-2021, Postgres Professional
  *
  *-------------------------------------------------------------------------
  */
@@ -113,18 +113,13 @@ static parray *setup_push_filelist(const char *archive_status_dir,
  * Where archlog_path is $BACKUP_PATH/wal/instance_name
  */
 void
-do_archive_push(InstanceState *instanceState, InstanceConfig *instance, char *wal_file_path,
+do_archive_push(InstanceState *instanceState, InstanceConfig *instance, char *pg_xlog_dir,
 				char *wal_file_name, int batch_size, bool overwrite,
 				bool no_sync, bool no_ready_rename)
 {
 	uint64		i;
-	char		current_dir[MAXPGPATH];
-	/* directory with wal files to be archived (usually instance pgdata/pg_wal) */
-	char		pg_xlog_dir[MAXPGPATH];
-	/* usually instance pgdata/pg_wal/archive_status */
-	char		archive_status_dir[MAXPGPATH];
-	char		xlog_wal_path[MAXPGPATH];
-	uint64		system_id;
+	/* usually instance pgdata/pg_wal/archive_status, empty if no_ready_rename or batch_size == 1 */
+	char		archive_status_dir[MAXPGPATH] = "";
 	bool		is_compress = false;
 
 	/* arrays with meta info for multi threaded backup */
@@ -144,50 +139,8 @@ do_archive_push(InstanceState *instanceState, InstanceConfig *instance, char *wa
 	parray     *batch_files = NULL;
 	int         n_threads;
 
-	if (wal_file_name == NULL)
-		elog(ERROR, "Required parameter is not specified: --wal-file-name %%f");
-
-	join_path_components(xlog_wal_path, PG_XLOG_DIR, wal_file_name);
-
-	if (wal_file_path == NULL)
-	{
-		elog(INFO, "Optional parameter is not specified: --wal_file_path %%p "
-					"Setting wal-file-path by default");
-		wal_file_path = xlog_wal_path;
-	}
-
-	if (strcmp(wal_file_path, xlog_wal_path) != 0)
-	{
-		elog(INFO, "wal_file_path is setted by user %s", wal_file_path);
-
-		join_path_components(pg_xlog_dir, instance->pgdata, XLOGDIR);
-	}
-	else
-	{
-		/* Create 'archlog_path' directory. Do nothing if it already exists. */
-		//fio_mkdir(instance->arclog_path, DIR_PERMISSION, FIO_BACKUP_HOST);
-
-		if (!getcwd(current_dir, sizeof(current_dir)))
-			elog(ERROR, "getcwd() error");
-
-		/* verify that archive-push --instance parameter is valid */
-		system_id = get_system_identifier(current_dir, FIO_DB_HOST);
-
-		if (instance->pgdata == NULL)
-			elog(ERROR, "Cannot read pg_probackup.conf for this instance");
-
-		if (system_id != instance->system_identifier)
-			elog(ERROR, "Refuse to push WAL segment %s into archive. Instance parameters mismatch."
-						"Instance '%s' should have SYSTEM_ID = " UINT64_FORMAT " instead of " UINT64_FORMAT,
-					wal_file_name, instanceState->instance_name, instance->system_identifier, system_id);
-
-		if (instance->compress_alg == PGLZ_COMPRESS)
-			elog(ERROR, "Cannot use pglz for WAL compression");
-
-		join_path_components(pg_xlog_dir, current_dir, XLOGDIR);
-	}
-
-	join_path_components(archive_status_dir, pg_xlog_dir, "archive_status");
+	if (!no_ready_rename || batch_size > 1)
+		join_path_components(archive_status_dir, pg_xlog_dir, "archive_status");
 
 #ifdef HAVE_LIBZ
 	if (instance->compress_alg == ZLIB_COMPRESS)
@@ -226,12 +179,13 @@ do_archive_push(InstanceState *instanceState, InstanceConfig *instance, char *wa
 		{
 			int rc;
 			WALSegno *xlogfile = (WALSegno *) parray_get(batch_files, i);
+			bool first_wal = strcmp(xlogfile->name, wal_file_name) == 0;
 
-			rc = push_file(xlogfile, archive_status_dir,
+			rc = push_file(xlogfile, first_wal ? NULL : archive_status_dir,
 						   pg_xlog_dir, instanceState->instance_wal_subdir_path,
 						   overwrite, no_sync,
 						   instance->archive_timeout,
-						   no_ready_rename || (strcmp(xlogfile->name, wal_file_name) == 0) ? true : false,
+						   no_ready_rename || first_wal,
 						   is_compress && IsXLogFileName(xlogfile->name) ? true : false,
 						   instance->compress_level);
 			if (rc == 0)
@@ -255,7 +209,7 @@ do_archive_push(InstanceState *instanceState, InstanceConfig *instance, char *wa
 		arg->first_filename = wal_file_name;
 		arg->archive_dir = instanceState->instance_wal_subdir_path;
 		arg->pg_xlog_dir = pg_xlog_dir;
-		arg->archive_status_dir = archive_status_dir;
+		arg->archive_status_dir = (!no_ready_rename || batch_size > 1) ? archive_status_dir : NULL;
 		arg->overwrite = overwrite;
 		arg->compress = is_compress;
 		arg->no_sync = no_sync;
@@ -298,7 +252,7 @@ do_archive_push(InstanceState *instanceState, InstanceConfig *instance, char *wa
 
 	/* Note, that we are leaking memory here,
 	 * because pushing into archive is a very
-	 * time-sensetive operation, so we skip freeing stuff.
+	 * time-sensitive operation, so we skip freeing stuff.
 	 */
 
 push_done:
@@ -378,9 +332,6 @@ push_file(WALSegno *xlogfile, const char *archive_status_dir,
 		  int compress_level)
 {
 	int     rc;
-	char	wal_file_dummy[MAXPGPATH];
-
-	join_path_components(wal_file_dummy, archive_status_dir, xlogfile->name);
 
 	elog(LOG, "pushing file \"%s\"", xlogfile->name);
 
@@ -397,11 +348,13 @@ push_file(WALSegno *xlogfile, const char *archive_status_dir,
 #endif
 
 	/* take '--no-ready-rename' flag into account */
-	if (!no_ready_rename)
+	if (!no_ready_rename && archive_status_dir != NULL)
 	{
+		char	wal_file_dummy[MAXPGPATH];
 		char	wal_file_ready[MAXPGPATH];
 		char	wal_file_done[MAXPGPATH];
 
+		join_path_components(wal_file_dummy, archive_status_dir, xlogfile->name);
 		snprintf(wal_file_ready, MAXPGPATH, "%s.%s", wal_file_dummy, "ready");
 		snprintf(wal_file_done, MAXPGPATH, "%s.%s", wal_file_dummy, "done");
 
diff --git a/src/pg_probackup.c b/src/pg_probackup.c
@@ -35,7 +35,7 @@
  * which includes info about pgdata directory and connection.
  *
  * Portions Copyright (c) 2009-2013, NIPPON TELEGRAPH AND TELEPHONE CORPORATION
- * Portions Copyright (c) 2015-2019, Postgres Professional
+ * Portions Copyright (c) 2015-2021, Postgres Professional
  *
  *-------------------------------------------------------------------------
  */
@@ -151,6 +151,7 @@ static char *wal_file_path;
 static char *wal_file_name;
 static bool file_overwrite = false;
 static bool no_ready_rename = false;
+static char archive_push_xlog_dir[MAXPGPATH] = "";
 
 /* archive get options */
 static char *prefetch_dir;
@@ -788,14 +789,95 @@ main(int argc, char *argv[])
 		current.stream = stream_wal = true;
 		if (instance_config.external_dir_str)
 			elog(ERROR, "external directories not supported fom \"%s\" command", get_subcmd_name(backup_subcmd));
-		// TODO проверить instance_config.conn_opt
+		// TODO check instance_config.conn_opt
 	}
 
 	/* sanity */
 	if (backup_subcmd == VALIDATE_CMD && restore_params->no_validate)
 		elog(ERROR, "You cannot specify \"--no-validate\" option with the \"%s\" command",
 			get_subcmd_name(backup_subcmd));
 
+	if (backup_subcmd == ARCHIVE_PUSH_CMD)
+	{
+		/* Check archive-push parameters and construct archive_push_xlog_dir
+		 *
+		 * There are 3 cases:
+		 * 1. no --wal-file-path specified -- use cwd, ./PG_XLOG_DIR for wal files
+		 * (and ./PG_XLOG_DIR/archive_status for .done files inside do_archive_push())
+		 * in this case we can use batches and threads
+		 * 2. --wal-file-path is specified and it is the same dir as stored in pg_probackup.conf (instance_config.pgdata)
+		 * in this case we can use this path, as well as batches and thread
+		 * 3. --wal-file-path is specified and it is different from instance_config.pgdata
+		 * disable optimizations and work with user specified path
+		 */
+		bool	check_system_id = true;
+		uint64	system_id;
+
+		if (wal_file_name == NULL)
+			elog(ERROR, "Required parameter is not specified: --wal-file-name %%f");
+
+		if (instance_config.pgdata == NULL)
+			elog(ERROR, "Cannot read pg_probackup.conf for this instance");
+
+		/* TODO may be remove in preference of checking inside compress_init()? */
+		if (instance_config.compress_alg == PGLZ_COMPRESS)
+                        elog(ERROR, "Cannot use pglz for WAL compression");
+
+		if (wal_file_path == NULL)
+		{
+			/* 1st case */
+			char	current_dir[MAXPGPATH];
+			if (!getcwd(current_dir, sizeof(current_dir)))
+				elog(ERROR, "getcwd() error");
+
+			system_id = get_system_identifier(current_dir, FIO_DB_HOST);
+			join_path_components(archive_push_xlog_dir, current_dir, XLOGDIR);
+		}
+		else
+		{
+			/*
+			 * Usually we get something like
+			 *   wal_file_path = "pg_wal/0000000100000000000000A1"
+			 *   wal_file_name = "0000000100000000000000A1"
+			 *   instance_config.pgdata = "/pgdata/.../node/data"
+			 * We need to strip wal_file_name from wal_file_path, add XLOGDIR to instance_config.pgdata
+			 * and compare this directories.
+			 * Note, that pg_wal can be symlink (see test_waldir_outside_pgdata_archiving)
+			 */
+			char	*stripped_wal_file_path = pgut_str_strip_trailing_filename(wal_file_path, wal_file_name);
+			join_path_components(archive_push_xlog_dir, instance_config.pgdata, XLOGDIR);
+			if (fio_is_same_file(stripped_wal_file_path, archive_push_xlog_dir, true, FIO_DB_HOST))
+			{
+				/* 2nd case */
+				system_id = get_system_identifier(instance_config.pgdata, FIO_DB_HOST);
+				/* archive_push_xlog_dir already have right value */
+			}
+			else
+			{
+				/* 3rd case */
+				check_system_id = false;
+				if (strlen(stripped_wal_file_path) < MAXPGPATH)
+					strncpy(archive_push_xlog_dir, stripped_wal_file_path, MAXPGPATH);
+				else
+					elog(ERROR, "Value specified to --wal_file_path is too long");
+
+				if (batch_size > 1 || num_threads > 1 || !no_ready_rename)
+				{
+					elog(WARNING, "Supplied --wal_file_path is outside pgdata, force safe values for options: --batch-size=1 -j 1 --no-ready-rename");
+					batch_size = 1;
+					num_threads = 1;
+					no_ready_rename = true;
+				}
+			}
+			pfree(stripped_wal_file_path);
+		}
+
+		if (check_system_id && system_id != instance_config.system_identifier)
+			elog(ERROR, "Refuse to push WAL segment %s into archive. Instance parameters mismatch."
+						"Instance '%s' should have SYSTEM_ID = " UINT64_FORMAT " instead of " UINT64_FORMAT,
+					wal_file_name, instanceState->instance_name, instance_config.system_identifier, system_id);
+	}
+
 #if PG_VERSION_NUM >= 100000
 	if (temp_slot && perm_slot)
 		elog(ERROR, "You cannot specify \"--perm-slot\" option with the \"--temp-slot\" option");
@@ -819,7 +901,7 @@ main(int argc, char *argv[])
 	switch (backup_subcmd)
 	{
 		case ARCHIVE_PUSH_CMD:
-			do_archive_push(instanceState, &instance_config, wal_file_path, wal_file_name,
+			do_archive_push(instanceState, &instance_config, archive_push_xlog_dir, wal_file_name,
 							batch_size, file_overwrite, no_sync, no_ready_rename);
 			break;
 		case ARCHIVE_GET_CMD:
diff --git a/src/pg_probackup.h b/src/pg_probackup.h
@@ -889,7 +889,7 @@ extern int do_init(CatalogState *catalogState);
 extern int do_add_instance(InstanceState *instanceState, InstanceConfig *instance);
 
 /* in archive.c */
-extern void do_archive_push(InstanceState *instanceState, InstanceConfig *instance, char *wal_file_path,
+extern void do_archive_push(InstanceState *instanceState, InstanceConfig *instance, char *pg_xlog_dir,
 						   char *wal_file_name, int batch_size, bool overwrite,
 						   bool no_sync, bool no_ready_rename);
 extern void do_archive_get(InstanceState *instanceState, InstanceConfig *instance, const char *prefetch_dir_arg, char *wal_file_path,
diff --git a/src/utils/file.c b/src/utils/file.c
@@ -1141,6 +1141,33 @@ fio_stat(char const* path, struct stat* st, bool follow_symlink, fio_location lo
 	}
 }
 
+/*
+ * Compare, that filename1 and filename2 is the same file
+ * in windows compare only filenames
+ */
+bool
+fio_is_same_file(char const* filename1, char const* filename2, bool follow_symlink, fio_location location)
+{
+#ifndef WIN32
+	struct stat	stat1, stat2;
+
+	if (fio_stat(filename1, &stat1, follow_symlink, location) < 0)
+		elog(ERROR, "Can't stat file \"%s\": %s", filename1, strerror(errno));
+
+	if (fio_stat(filename2, &stat2, follow_symlink, location) < 0)
+		elog(ERROR, "Can't stat file \"%s\": %s", filename2, strerror(errno));
+
+	return stat1.st_ino == stat2.st_ino && stat1.st_dev == stat2.st_dev;
+#else
+	char	*abs_name1 = make_absolute_path(filename1);
+	char	*abs_name2 = make_absolute_path(filename2);
+	bool	result = strcmp(abs_name1, abs_name2) == 0;
+	free(abs_name2);
+	free(abs_name1);
+	return result;
+#endif
+}
+
 /*
  * Read value of a symbolic link
  * this is a wrapper about readlink() syscall
diff --git a/src/utils/file.h b/src/utils/file.h
@@ -129,6 +129,7 @@ extern int     fio_mkdir(char const* path, int mode, fio_location location);
 extern int     fio_chmod(char const* path, int mode, fio_location location);
 extern int     fio_access(char const* path, int mode, fio_location location);
 extern int     fio_stat(char const* path, struct stat* st, bool follow_symlinks, fio_location location);
+extern bool    fio_is_same_file(char const* filename1, char const* filename2, bool follow_symlink, fio_location location);
 extern ssize_t fio_readlink(const char *path, char *value, size_t valsiz, fio_location location);
 extern DIR*    fio_opendir(char const* path, fio_location location);
 extern struct dirent * fio_readdir(DIR *dirp);
diff --git a/src/utils/pgut.c b/src/utils/pgut.c
@@ -977,6 +977,22 @@ pgut_strndup(const char *str, size_t n)
 	return ret;
 }
 
+/*
+ * Allocates new string, that contains part of filepath string minus trailing filename string
+ * If trailing filename string not found, returns copy of filepath.
+ * Result must be free by caller.
+ */
+char *
+pgut_str_strip_trailing_filename(const char *filepath, const char *filename)
+{
+	size_t	fp_len = strlen(filepath);
+	size_t	fn_len = strlen(filename);
+	if (strncmp(filepath + fp_len - fn_len, filename, fn_len) == 0)
+		return pgut_strndup(filepath, fp_len - fn_len);
+	else
+		return pgut_strndup(filepath, fp_len);
+}
+
 FILE *
 pgut_fopen(const char *path, const char *mode, bool missing_ok)
 {
diff --git a/src/utils/pgut.h b/src/utils/pgut.h
@@ -63,6 +63,7 @@ extern void *pgut_malloc0(size_t size);
 extern void *pgut_realloc(void *p, size_t size);
 extern char *pgut_strdup(const char *str);
 extern char *pgut_strndup(const char *str, size_t n);
+extern char *pgut_str_strip_trailing_filename(const char *filepath, const char *filename);
 
 #define pgut_new(type)			((type *) pgut_malloc(sizeof(type)))
 #define pgut_new0(type)			((type *) pgut_malloc0(sizeof(type)))
diff --git a/tests/archive.py b/tests/archive.py
