Subject: optimize file transfer in pg_upgrade

From: Nathan Bossart <[email-masked]>
Date: Wed, 6 Nov 2024 16:07:35 -0600

For clusters with many relations, the file transfer step of pg_upgrade can
take the longest.  This step clones, copies, or links the user relation
files from the older cluster to the new cluster, so the amount of time it
takes is closely related to the number of relations.  However, since v15,
we've preserved the relfilenodes during pg_upgrade, which means that all of
these user relation files will have the same name.  Therefore, it can be
much faster to instead move the entire data directory from the old cluster
to the new cluster and to then swap the catalog relation files.

The attached proof-of-concept patches implement this "catalog-swap" mode
for demonstration purposes.  I tested this mode on a cluster with 200
databases, each with 10,000 tables with 1,000 rows and 2 unique constraints
apiece.  Each database also had 10,000 sequences.  The test used 96 jobs.

  pg_upgrade --link --sync-method syncfs  -->  10m 23s (~5m linking)
  pg_upgrade --catalog-swap               -->  5m 32s (~30s linking)

While these results are encouraging, there are a couple of interesting
problems to manage.  First, in order to move the data directory from the
old cluster to the new cluster, we will have first moved the new cluster's
data directory (full of files created by pg_restore) aside.  After the file
transfer stage, this directory will be filled with useless empty files that
should eventually be deleted.  Furthermore, none of these files will have
been synchronized to disk (outside of whatever the kernel has done in the
background), so pg_upgrade's data synchronization step can take a very long
time, even when syncfs() is used (so long that pg_upgrade can take even
longer than before).  After much testing, the best way I've found to deal
with this problem is to introduce a special mode for "initdb --sync-only"
that calls fsync() for everything _except_ the actual data files.  If we
fsync() the new catalog files as we move them into place, and if we assume
that the old catalog files will have been properly synchronized before
upgrading, there's no reason to synchronize them again at the end.

Another interesting problem is that pg_upgrade currently doesn't transfer
the sequence data files.  Since v10, we've restored these via pg_restore.
I believe this was originally done for the introduction of the pg_sequence
catalog, which changed the format of sequence tuples.  In the new
catalog-swap mode I am proposing, this means we need to transfer all the
pg_restore-generated sequence data files.  If there are many sequences, it
can be difficult to determine which transfer mode and synchronization
method will be faster.  Since sequence tuple modifications are very rare, I
think the new catalog-swap mode should just use the sequence data files
from the old cluster whenever possible.

There are a couple of other smaller trade-offs with this approach, too.
First, this new mode complicates rollback if, say, the machine loses power
during file transfer.  IME the vast majority of failures happen before this
step, and it should be relatively simple to generate a script that will
safely perform the required rollback steps, so I don't think this is a
deal-breaker.  Second, this mode leaves around a bunch of files that users
would likely want to clean up at some point.  I think the easiest way to
handle this is to just put all these files in the old cluster's data
directory so that the cleanup script generated by pg_upgrade also takes
care of them.

Thoughts?

-- 
nathan
>From f800010296b1749b57e0fe3dcde010cc2ba41973 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Tue, 5 Nov 2024 15:59:51 -0600
Subject: [PATCH v1 1/8] Export walkdir().

THIS IS A PROOF OF CONCEPT AND IS NOT READY FOR SERIOUS REVIEW.

A follow-up commit will use this function to swap catalog files
between database directories during pg_upgrade.
---
 src/common/file_utils.c         | 5 +----
 src/include/common/file_utils.h | 3 +++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 398fe1c334..3f488bf5ec 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -48,9 +48,6 @@
 #ifdef PG_FLUSH_DATA_WORKS
 static int	pre_sync_fname(const char *fname, bool isdir);
 #endif
-static void walkdir(const char *path,
-					int (*action) (const char *fname, bool isdir),
-					bool process_symlinks);
 
 #ifdef HAVE_SYNCFS
 
@@ -268,7 +265,7 @@ sync_dir_recurse(const char *dir, DataDirSyncMethod sync_method)
  *
  * See also walkdir in fd.c, which is a backend version of this logic.
  */
-static void
+void
 walkdir(const char *path,
 		int (*action) (const char *fname, bool isdir),
 		bool process_symlinks)
diff --git a/src/include/common/file_utils.h b/src/include/common/file_utils.h
index e4339fb7b6..5a9519acfe 100644
--- a/src/include/common/file_utils.h
+++ b/src/include/common/file_utils.h
@@ -39,6 +39,9 @@ extern void sync_pgdata(const char *pg_data, int serverVersion,
 extern void sync_dir_recurse(const char *dir, DataDirSyncMethod sync_method);
 extern int	durable_rename(const char *oldfile, const char *newfile);
 extern int	fsync_parent_path(const char *fname);
+extern void walkdir(const char *path,
+					int (*action) (const char *fname, bool isdir),
+					bool process_symlinks);
 #endif
 
 extern PGFileType get_dirent_type(const char *path,
-- 
2.39.5 (Apple Git-154)

>From 2d6b0d5708f07203ad2ffbd889404094d0c5969c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 6 Nov 2024 13:59:39 -0600
Subject: [PATCH v1 2/8] Add "void *arg" parameter to walkdir() that is passed
 to function.

THIS IS A PROOF OF CONCEPT AND IS NOT READY FOR SERIOUS REVIEW.

This will be used in follow up commits to pass private state to the
functions called by walkdir().
---
 src/bin/pg_basebackup/walmethods.c |  8 +++----
 src/bin/pg_dump/pg_backup_custom.c |  2 +-
 src/bin/pg_dump/pg_backup_tar.c    |  2 +-
 src/bin/pg_dump/pg_dumpall.c       |  2 +-
 src/common/file_utils.c            | 38 +++++++++++++++---------------
 src/include/common/file_utils.h    |  6 ++---
 6 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index 215b24597f..51640cb493 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -251,7 +251,7 @@ dir_open_for_write(WalWriteMethod *wwmethod, const char *pathname,
 	 */
 	if (wwmethod->sync)
 	{
-		if (fsync_fname(tmppath, false) != 0 ||
+		if (fsync_fname(tmppath, false, NULL) != 0 ||
 			fsync_parent_path(tmppath) != 0)
 		{
 			wwmethod->lasterrno = errno;
@@ -486,7 +486,7 @@ dir_close(Walfile *f, WalCloseMethod method)
 			 */
 			if (f->wwmethod->sync)
 			{
-				r = fsync_fname(df->fullpath, false);
+				r = fsync_fname(df->fullpath, false, NULL);
 				if (r == 0)
 					r = fsync_parent_path(df->fullpath);
 			}
@@ -617,7 +617,7 @@ dir_finish(WalWriteMethod *wwmethod)
 		 * Files are fsynced when they are closed, but we need to fsync the
 		 * directory entry here as well.
 		 */
-		if (fsync_fname(dir_data->basedir, true) != 0)
+		if (fsync_fname(dir_data->basedir, true, NULL) != 0)
 		{
 			wwmethod->lasterrno = errno;
 			return false;
@@ -1321,7 +1321,7 @@ tar_finish(WalWriteMethod *wwmethod)
 
 	if (wwmethod->sync)
 	{
-		if (fsync_fname(tar_data->tarfilename, false) != 0 ||
+		if (fsync_fname(tar_data->tarfilename, false, NULL) != 0 ||
 			fsync_parent_path(tar_data->tarfilename) != 0)
 		{
 			wwmethod->lasterrno = errno;
diff --git a/src/bin/pg_dump/pg_backup_custom.c b/src/bin/pg_dump/pg_backup_custom.c
index ecaad7321a..6f750c916c 100644
--- a/src/bin/pg_dump/pg_backup_custom.c
+++ b/src/bin/pg_dump/pg_backup_custom.c
@@ -767,7 +767,7 @@ _CloseArchive(ArchiveHandle *AH)
 
 	/* Sync the output file if one is defined */
 	if (AH->dosync && AH->mode == archModeWrite && AH->fSpec)
-		(void) fsync_fname(AH->fSpec, false);
+		(void) fsync_fname(AH->fSpec, false, NULL);
 
 	AH->FH = NULL;
 }
diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
index 41ee52b1d6..ecba27b623 100644
--- a/src/bin/pg_dump/pg_backup_tar.c
+++ b/src/bin/pg_dump/pg_backup_tar.c
@@ -847,7 +847,7 @@ _CloseArchive(ArchiveHandle *AH)
 
 		/* Sync the output file if one is defined */
 		if (AH->dosync && AH->fSpec)
-			(void) fsync_fname(AH->fSpec, false);
+			(void) fsync_fname(AH->fSpec, false, NULL);
 	}
 
 	AH->FH = NULL;
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index e3ad8fb295..cbb1e3f9e4 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -621,7 +621,7 @@ main(int argc, char *argv[])
 
 		/* sync the resulting file, errors are not fatal */
 		if (dosync)
-			(void) fsync_fname(filename, false);
+			(void) fsync_fname(filename, false, NULL);
 	}
 
 	exit_nicely(0);
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 3f488bf5ec..dc90f35ae1 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -46,7 +46,7 @@
 #define MINIMUM_VERSION_FOR_PG_WAL	100000
 
 #ifdef PG_FLUSH_DATA_WORKS
-static int	pre_sync_fname(const char *fname, bool isdir);
+static int	pre_sync_fname(const char *fname, bool isdir, void *arg);
 #endif
 
 #ifdef HAVE_SYNCFS
@@ -184,10 +184,10 @@ sync_pgdata(const char *pg_data,
 				 * fsync the data directory and its contents.
 				 */
 #ifdef PG_FLUSH_DATA_WORKS
-				walkdir(pg_data, pre_sync_fname, false);
+				walkdir(pg_data, pre_sync_fname, false, NULL);
 				if (xlog_is_symlink)
-					walkdir(pg_wal, pre_sync_fname, false);
-				walkdir(pg_tblspc, pre_sync_fname, true);
+					walkdir(pg_wal, pre_sync_fname, false, NULL);
+				walkdir(pg_tblspc, pre_sync_fname, true, NULL);
 #endif
 
 				/*
@@ -200,10 +200,10 @@ sync_pgdata(const char *pg_data,
 				 * get fsync'd twice. That's not an expected case so we don't
 				 * worry about optimizing it.
 				 */
-				walkdir(pg_data, fsync_fname, false);
+				walkdir(pg_data, fsync_fname, false, NULL);
 				if (xlog_is_symlink)
-					walkdir(pg_wal, fsync_fname, false);
-				walkdir(pg_tblspc, fsync_fname, true);
+					walkdir(pg_wal, fsync_fname, false, NULL);
+				walkdir(pg_tblspc, fsync_fname, true, NULL);
 			}
 			break;
 	}
@@ -242,10 +242,10 @@ sync_dir_recurse(const char *dir, DataDirSyncMethod sync_method)
 				 * fsync the data directory and its contents.
 				 */
 #ifdef PG_FLUSH_DATA_WORKS
-				walkdir(dir, pre_sync_fname, false);
+				walkdir(dir, pre_sync_fname, false, NULL);
 #endif
 
-				walkdir(dir, fsync_fname, false);
+				walkdir(dir, fsync_fname, false, NULL);
 			}
 			break;
 	}
@@ -267,8 +267,8 @@ sync_dir_recurse(const char *dir, DataDirSyncMethod sync_method)
  */
 void
 walkdir(const char *path,
-		int (*action) (const char *fname, bool isdir),
-		bool process_symlinks)
+		int (*action) (const char *fname, bool isdir, void *arg),
+		bool process_symlinks, void *arg)
 {
 	DIR		   *dir;
 	struct dirent *de;
@@ -293,10 +293,10 @@ walkdir(const char *path,
 		switch (get_dirent_type(subpath, de, process_symlinks, PG_LOG_ERROR))
 		{
 			case PGFILETYPE_REG:
-				(*action) (subpath, false);
+				(*action) (subpath, false, arg);
 				break;
 			case PGFILETYPE_DIR:
-				walkdir(subpath, action, false);
+				walkdir(subpath, action, false, arg);
 				break;
 			default:
 
@@ -320,7 +320,7 @@ walkdir(const char *path,
 	 * synced.  Recent versions of ext4 have made the window much wider but
 	 * it's been an issue for ext3 and other filesystems in the past.
 	 */
-	(*action) (path, true);
+	(*action) (path, true, arg);
 }
 
 /*
@@ -332,7 +332,7 @@ walkdir(const char *path,
 #ifdef PG_FLUSH_DATA_WORKS
 
 static int
-pre_sync_fname(const char *fname, bool isdir)
+pre_sync_fname(const char *fname, bool isdir, void *arg)
 {
 	int			fd;
 
@@ -373,7 +373,7 @@ pre_sync_fname(const char *fname, bool isdir)
  * are fatal.
  */
 int
-fsync_fname(const char *fname, bool isdir)
+fsync_fname(const char *fname, bool isdir, void *arg)
 {
 	int			fd;
 	int			flags;
@@ -444,7 +444,7 @@ fsync_parent_path(const char *fname)
 	if (strlen(parentpath) == 0)
 		strlcpy(parentpath, ".", MAXPGPATH);
 
-	if (fsync_fname(parentpath, true) != 0)
+	if (fsync_fname(parentpath, true, NULL) != 0)
 		return -1;
 
 	return 0;
@@ -467,7 +467,7 @@ durable_rename(const char *oldfile, const char *newfile)
 	 * because it's then guaranteed that either source or target file exists
 	 * after a crash.
 	 */
-	if (fsync_fname(oldfile, false) != 0)
+	if (fsync_fname(oldfile, false, NULL) != 0)
 		return -1;
 
 	fd = open(newfile, PG_BINARY | O_RDWR, 0);
@@ -502,7 +502,7 @@ durable_rename(const char *oldfile, const char *newfile)
 	 * To guarantee renaming the file is persistent, fsync the file with its
 	 * new name, and its containing directory.
 	 */
-	if (fsync_fname(newfile, false) != 0)
+	if (fsync_fname(newfile, false, NULL) != 0)
 		return -1;
 
 	if (fsync_parent_path(newfile) != 0)
diff --git a/src/include/common/file_utils.h b/src/include/common/file_utils.h
index 5a9519acfe..c328f56a85 100644
--- a/src/include/common/file_utils.h
+++ b/src/include/common/file_utils.h
@@ -33,15 +33,15 @@ typedef enum DataDirSyncMethod
 struct iovec;					/* avoid including port/pg_iovec.h here */
 
 #ifdef FRONTEND
-extern int	fsync_fname(const char *fname, bool isdir);
+extern int	fsync_fname(const char *fname, bool isdir, void *arg);
 extern void sync_pgdata(const char *pg_data, int serverVersion,
 						DataDirSyncMethod sync_method);
 extern void sync_dir_recurse(const char *dir, DataDirSyncMethod sync_method);
 extern int	durable_rename(const char *oldfile, const char *newfile);
 extern int	fsync_parent_path(const char *fname);
 extern void walkdir(const char *path,
-					int (*action) (const char *fname, bool isdir),
-					bool process_symlinks);
+					int (*action) (const char *fname, bool isdir, void *arg),
+					bool process_symlinks, void *arg);
 #endif
 
 extern PGFileType get_dirent_type(const char *path,
-- 
2.39.5 (Apple Git-154)

>From c248059f7276578f5e8abd00ea9efb007423f94d Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Tue, 5 Nov 2024 16:38:19 -0600
Subject: [PATCH v1 3/8] Introduce catalog-swap mode for pg_upgrade.

THIS IS A PROOF OF CONCEPT AND IS NOT READY FOR SERIOUS REVIEW.

This new mode moves the database directories from the old cluster
to the new cluster and then swaps the pg_restore-generated catalog
files in place.  This can significantly increase the length of the
following data synchronization step (due to the large number of
unsynchronized pg_restore-generated files), but this problem will
be handled in follow-up commits.
---
 src/bin/pg_upgrade/check.c         |   2 +
 src/bin/pg_upgrade/option.c        |   5 +
 src/bin/pg_upgrade/pg_upgrade.h    |   1 +
 src/bin/pg_upgrade/relfilenumber.c | 167 +++++++++++++++++++++++++++++
 src/tools/pgindent/typedefs.list   |   1 +
 5 files changed, 176 insertions(+)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 94164f0472..a4bb365718 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -711,6 +711,8 @@ check_new_cluster(void)
 		case TRANSFER_MODE_LINK:
 			check_hard_link();
 			break;
+		case TRANSFER_MODE_CATALOG_SWAP:
+			break;
 	}
 
 	check_is_install_user(&new_cluster);
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 6f41d63eed..64091a54c4 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -60,6 +60,7 @@ parseCommandLine(int argc, char *argv[])
 		{"copy", no_argument, NULL, 2},
 		{"copy-file-range", no_argument, NULL, 3},
 		{"sync-method", required_argument, NULL, 4},
+		{"catalog-swap", no_argument, NULL, 5},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -212,6 +213,10 @@ parseCommandLine(int argc, char *argv[])
 				user_opts.sync_method = pg_strdup(optarg);
 				break;
 
+			case 5:
+				user_opts.transfer_mode = TRANSFER_MODE_CATALOG_SWAP;
+				break;
+
 			default:
 				fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
 						os_info.progname);
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 53f693c2d4..19cb5a011e 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -256,6 +256,7 @@ typedef enum
 	TRANSFER_MODE_COPY,
 	TRANSFER_MODE_COPY_FILE_RANGE,
 	TRANSFER_MODE_LINK,
+	TRANSFER_MODE_CATALOG_SWAP,
 } transferMode;
 
 /*
diff --git a/src/bin/pg_upgrade/relfilenumber.c b/src/bin/pg_upgrade/relfilenumber.c
index 07baa49a02..9d8fce3c4a 100644
--- a/src/bin/pg_upgrade/relfilenumber.c
+++ b/src/bin/pg_upgrade/relfilenumber.c
@@ -11,11 +11,21 @@
 
 #include <sys/stat.h>
 
+#include "common/file_perm.h"
+#include "common/file_utils.h"
+#include "common/int.h"
+#include "fe_utils/option_utils.h"
 #include "pg_upgrade.h"
 
 static void transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace);
 static void transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_frozenbit);
 
+typedef struct move_catalog_file_context
+{
+	FileNameMap *maps;
+	int			size;
+	char	   *target;
+} move_catalog_file_context;
 
 /*
  * transfer_all_new_tablespaces()
@@ -41,6 +51,9 @@ transfer_all_new_tablespaces(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 		case TRANSFER_MODE_LINK:
 			prep_status_progress("Linking user relation files");
 			break;
+		case TRANSFER_MODE_CATALOG_SWAP:
+			prep_status_progress("Swapping catalog files");
+			break;
 	}
 
 	/*
@@ -127,6 +140,144 @@ transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 	}
 }
 
+static int
+FileNameMapCmp(const void *a, const void *b)
+{
+	return pg_cmp_u32(((const FileNameMap *) a)->relfilenumber,
+					  ((const FileNameMap *) b)->relfilenumber);
+}
+
+static RelFileNumber
+parse_relfilenumber(const char *filename)
+{
+	char	   *endp;
+	unsigned long n;
+
+	if (filename[0] < '1' || filename[0] > '9')
+		return InvalidRelFileNumber;
+
+	errno = 0;
+	n = strtoul(filename, &endp, 10);
+	if (errno || filename == endp || n <= 0 || n > PG_UINT32_MAX)
+		return InvalidRelFileNumber;
+
+	return (RelFileNumber) n;
+}
+
+static int
+move_catalog_file(const char *fname, bool isdir, void *arg)
+{
+	char		dst[MAXPGPATH];
+	const char *filename = last_dir_separator(fname) + 1;
+	RelFileNumber rfn = parse_relfilenumber(filename);
+	move_catalog_file_context *context = (move_catalog_file_context *) arg;
+
+	/*
+	 * XXX: Is this right?  AFAICT we don't really expect there to be
+	 * directories within database directories, so perhaps it would be better
+	 * to either unconditionally rename or to fail.  Further investigation is
+	 * required.
+	 */
+	if (isdir)
+		return 0;
+
+	if (RelFileNumberIsValid(rfn))
+	{
+		FileNameMap key;
+
+		key.relfilenumber = (RelFileNumber) rfn;
+		if (bsearch(&key, context->maps, context->size,
+					sizeof(FileNameMap), FileNameMapCmp))
+			return 0;
+	}
+
+	snprintf(dst, sizeof(dst), "%s/%s", context->target, filename);
+	if (rename(fname, dst) != 0)
+		pg_fatal("could not rename \"%s\" to \"%s\": %m", fname, dst);
+
+	return 0;
+}
+
+/*
+ * XXX: This proof-of-concept patch doesn't yet handle non-default tablespaces.
+ */
+static void
+do_catalog_transfer(FileNameMap *maps, int size, char *old_tablespace)
+{
+	char		old_tblspc[MAXPGPATH];
+	char		new_tblspc[MAXPGPATH];
+	char		old_dat[MAXPGPATH];
+	char		new_dat[MAXPGPATH];
+	char		moved_tblspc[MAXPGPATH];
+	char		moved_dat[MAXPGPATH];
+	char		old_cat[MAXPGPATH];
+	move_catalog_file_context context;
+	DataDirSyncMethod sync_method = DATA_DIR_SYNC_METHOD_FSYNC;
+
+	parse_sync_method(user_opts.sync_method, &sync_method);
+
+	snprintf(old_tblspc, sizeof(old_tblspc), "%s%s",
+			 maps[0].old_tablespace, maps[0].old_tablespace_suffix);
+	snprintf(new_tblspc, sizeof(new_tblspc), "%s%s",
+			 maps[0].new_tablespace, maps[0].new_tablespace_suffix);
+	snprintf(old_dat, sizeof(old_dat), "%s/%u", old_tblspc, maps[0].db_oid);
+	snprintf(new_dat, sizeof(new_dat), "%s/%u", new_tblspc, maps[0].db_oid);
+	snprintf(moved_tblspc, sizeof(moved_tblspc), "%s_moved", old_tblspc);
+	snprintf(moved_dat, sizeof(moved_dat), "%s/%u",
+			 moved_tblspc, maps[0].db_oid);
+	snprintf(old_cat, sizeof(old_cat), "%s/%u_old_cat",
+			 moved_tblspc, maps[0].db_oid);
+
+	qsort(maps, size, sizeof(FileNameMap), FileNameMapCmp);
+
+	/* create dir for stuff that is moved aside */
+	if (pg_mkdir_p(moved_tblspc, pg_dir_create_mode) && errno != EEXIST)
+		pg_fatal("could not create directory \"%s\": %m", moved_tblspc);
+
+	/* move new cluster data dir aside */
+	if (rename(new_dat, moved_dat))
+		pg_fatal("could not rename \"%s\" to \"%s\": %m", new_dat, moved_dat);
+
+	/* move old cluster data dir in place */
+	if (rename(old_dat, new_dat))
+		pg_fatal("could not rename \"%s\" to \"%s\": %m", old_dat, new_dat);
+
+	/* create dir for old catalogs */
+	if (pg_mkdir_p(old_cat, pg_dir_create_mode))
+		pg_fatal("could not create directory \"%s\": %m", old_cat);
+
+	/* move catalogs in new data dir aside */
+	context.maps = maps;
+	context.size = size;
+	context.target = old_cat;
+	walkdir(new_dat, move_catalog_file, false, &context);
+
+	/* move catalogs in moved-aside data dir in place */
+	context.target = new_dat;
+	walkdir(moved_dat, move_catalog_file, false, &context);
+
+	/* no need to sync things individually if we are going to syncfs() later */
+	if (sync_method == DATA_DIR_SYNC_METHOD_SYNCFS)
+		return;
+
+	/* fsync directory entries */
+	if (fsync_fname(moved_dat, true, NULL) != 0)
+		pg_fatal("could not synchronize directory \"%s\": %m", moved_dat);
+	if (fsync_fname(old_cat, true, NULL) != 0)
+		pg_fatal("could not synchronize directory \"%s\": %m", old_cat);
+
+	/*
+	 * XXX: We could instead fsync() these directories once at the end instead
+	 * of once per-database, but it doesn't affect performance meaningfully,
+	 * and this is just a proof-of-concept patch, so I haven't bothered doing
+	 * the required refactoring yet.
+	 */
+	if (fsync_fname(old_tblspc, true, NULL) != 0)
+		pg_fatal("could not synchronize directory \"%s\": %m", old_tblspc);
+	if (fsync_fname(moved_tblspc, true, NULL) != 0)
+		pg_fatal("could not synchronize directory \"%s\": %m", moved_tblspc);
+}
+
 /*
  * transfer_single_new_db()
  *
@@ -145,6 +296,18 @@ transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace)
 		new_cluster.controldata.cat_ver >= VISIBILITY_MAP_FROZEN_BIT_CAT_VER)
 		vm_must_add_frozenbit = true;
 
+	/*
+	 * XXX: In catalog-swap mode, vm_must_add_frozenbit isn't handled yet.  We
+	 * could either disallow using catalog-swap mode if the upgrade involves
+	 * versions older than v9.6, or we could add code to handle rewriting the
+	 * visibility maps in this mode (like the other modes do).
+	 */
+	if (user_opts.transfer_mode == TRANSFER_MODE_CATALOG_SWAP)
+	{
+		do_catalog_transfer(maps, size, old_tablespace);
+		return;
+	}
+
 	for (mapnum = 0; mapnum < size; mapnum++)
 	{
 		if (old_tablespace == NULL ||
@@ -259,6 +422,10 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro
 					pg_log(PG_VERBOSE, "linking \"%s\" to \"%s\"",
 						   old_file, new_file);
 					linkFile(old_file, new_file, map->nspname, map->relname);
+					break;
+				case TRANSFER_MODE_CATALOG_SWAP:
+					pg_fatal("should never happen");
+					break;
 			}
 	}
 }
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 1847bbfa95..58c339af85 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3649,6 +3649,7 @@ mix_data_t
 mixedStruct
 mode_t
 movedb_failure_params
+move_catalog_file_context
 multirange_bsearch_comparison
 multirange_unnest_fctx
 mxact
-- 
2.39.5 (Apple Git-154)

>From 1fd29fa00c777b2c04683394f54261b446e46a61 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Tue, 5 Nov 2024 16:47:42 -0600
Subject: [PATCH v1 4/8] Add --no-sync-data-files flag to initdb.

THIS IS A PROOF OF CONCEPT AND IS NOT READY FOR SERIOUS REVIEW.

This new mode caused 'initdb --sync-only' to synchronize everything
except for the database directories.  It will be used in a
follow-up commit that aims to reduce the duration of the data
synchronization step in pg_upgrade's catalog-swap mode.
---
 src/bin/initdb/initdb.c                     |  9 ++++--
 src/bin/pg_basebackup/pg_basebackup.c       |  2 +-
 src/bin/pg_checksums/pg_checksums.c         |  2 +-
 src/bin/pg_combinebackup/pg_combinebackup.c |  2 +-
 src/bin/pg_rewind/file_ops.c                |  2 +-
 src/common/file_utils.c                     | 35 ++++++++++++++++-----
 src/include/common/file_utils.h             |  3 +-
 7 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 9a91830783..53c6e86a80 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -168,6 +168,7 @@ static bool data_checksums = true;
 static char *xlog_dir = NULL;
 static int	wal_segment_size_mb = (DEFAULT_XLOG_SEG_SIZE) / (1024 * 1024);
 static DataDirSyncMethod sync_method = DATA_DIR_SYNC_METHOD_FSYNC;
+static bool sync_data_files = true;

 /* internal vars */
@@ -3183,6 +3184,7 @@ main(int argc, char *argv[])
 		{"icu-rules", required_argument, NULL, 18},
 		{"sync-method", required_argument, NULL, 19},
 		{"no-data-checksums", no_argument, NULL, 20},
+		{"no-sync-data-files", no_argument, NULL, 21},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -3377,6 +3379,9 @@ main(int argc, char *argv[])
 			case 20:
 				data_checksums = false;
 				break;
+			case 21:
+				sync_data_files = false;
+				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -3428,7 +3433,7 @@ main(int argc, char *argv[])
 
 		fputs(_("syncing data to disk ... "), stdout);
 		fflush(stdout);
-		sync_pgdata(pg_data, PG_VERSION_NUM, sync_method);
+		sync_pgdata(pg_data, PG_VERSION_NUM, sync_method, sync_data_files);
 		check_ok();
 		return 0;
 	}
@@ -3491,7 +3496,7 @@ main(int argc, char *argv[])
 	{
 		fputs(_("syncing data to disk ... "), stdout);
 		fflush(stdout);
-		sync_pgdata(pg_data, PG_VERSION_NUM, sync_method);
+		sync_pgdata(pg_data, PG_VERSION_NUM, sync_method, sync_data_files);
 		check_ok();
 	}
 	else
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index e41a6cfbda..43526e3246 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2310,7 +2310,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
 		}
 		else
 		{
-			(void) sync_pgdata(basedir, serverVersion, sync_method);
+			(void) sync_pgdata(basedir, serverVersion, sync_method, true);
 		}
 	}
 
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index b86bc417c9..06ccaacfda 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -633,7 +633,7 @@ main(int argc, char *argv[])
 		if (do_sync)
 		{
 			pg_log_info("syncing data directory");
-			sync_pgdata(DataDir, PG_VERSION_NUM, sync_method);
+			sync_pgdata(DataDir, PG_VERSION_NUM, sync_method, true);
 		}
 
 		pg_log_info("updating control file");
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index 5f1f62f1db..80a137be4e 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -420,7 +420,7 @@ main(int argc, char *argv[])
 		else
 		{
 			pg_log_debug("recursively fsyncing \"%s\"", opt.output);
-			sync_pgdata(opt.output, version * 10000, opt.sync_method);
+			sync_pgdata(opt.output, version * 10000, opt.sync_method, true);
 		}
 	}
 
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index 67a86bb4c5..ceb1c3ac6d 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -296,7 +296,7 @@ sync_target_dir(void)
 	if (!do_sync || dry_run)
 		return;
 
-	sync_pgdata(datadir_target, PG_VERSION_NUM, sync_method);
+	sync_pgdata(datadir_target, PG_VERSION_NUM, sync_method, true);
 }

diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index dc90f35ae1..65cdf07ae7 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -94,7 +94,8 @@ do_syncfs(const char *path)
 void
 sync_pgdata(const char *pg_data,
 			int serverVersion,
-			DataDirSyncMethod sync_method)
+			DataDirSyncMethod sync_method,
+			bool sync_data_files)
 {
 	bool		xlog_is_symlink;
 	char		pg_wal[MAXPGPATH];
@@ -184,10 +185,11 @@ sync_pgdata(const char *pg_data,
 				 * fsync the data directory and its contents.
 				 */
 #ifdef PG_FLUSH_DATA_WORKS
-				walkdir(pg_data, pre_sync_fname, false, NULL);
+				walkdir(pg_data, pre_sync_fname, false, &sync_data_files);
 				if (xlog_is_symlink)
-					walkdir(pg_wal, pre_sync_fname, false, NULL);
-				walkdir(pg_tblspc, pre_sync_fname, true, NULL);
+					walkdir(pg_wal, pre_sync_fname, false, &sync_data_files);
+				if (sync_data_files)
+					walkdir(pg_tblspc, pre_sync_fname, true, NULL);
 #endif
 
 				/*
@@ -200,10 +202,11 @@ sync_pgdata(const char *pg_data,
 				 * get fsync'd twice. That's not an expected case so we don't
 				 * worry about optimizing it.
 				 */
-				walkdir(pg_data, fsync_fname, false, NULL);
+				walkdir(pg_data, fsync_fname, false, &sync_data_files);
 				if (xlog_is_symlink)
-					walkdir(pg_wal, fsync_fname, false, NULL);
-				walkdir(pg_tblspc, fsync_fname, true, NULL);
+					walkdir(pg_wal, fsync_fname, false, &sync_data_files);
+				if (sync_data_files)
+					walkdir(pg_tblspc, fsync_fname, true, NULL);
 			}
 			break;
 	}
@@ -296,7 +299,23 @@ walkdir(const char *path,
 				(*action) (subpath, false, arg);
 				break;
 			case PGFILETYPE_DIR:
-				walkdir(subpath, action, false, arg);
+
+				/*
+				 * XXX: Checking here for the "sync_data_files" case is quite
+				 * hacky, but it's not clear how to do better.  Another option
+				 * would be to send "de" down to the function, but that would
+				 * introduce a huge number of function pointer calls and
+				 * directory reads that we are trying to avoid.
+				 */
+#ifdef PG_FLUSH_DATA_WORKS
+				if ((action != pre_sync_fname && action != fsync_fname) ||
+#else
+				if (action != fsync_fname ||
+#endif
+					!arg || *((bool *) arg) ||
+					strcmp(de->d_name, "base") != 0)
+					walkdir(subpath, action, false, arg);
+
 				break;
 			default:
 
diff --git a/src/include/common/file_utils.h b/src/include/common/file_utils.h
index c328f56a85..3743caa63e 100644
--- a/src/include/common/file_utils.h
+++ b/src/include/common/file_utils.h
@@ -35,7 +35,8 @@ struct iovec;					/* avoid including port/pg_iovec.h here */
 #ifdef FRONTEND
 extern int	fsync_fname(const char *fname, bool isdir, void *arg);
 extern void sync_pgdata(const char *pg_data, int serverVersion,
-						DataDirSyncMethod sync_method);
+						DataDirSyncMethod sync_method,
+						bool sync_data_files);
 extern void sync_dir_recurse(const char *dir, DataDirSyncMethod sync_method);
 extern int	durable_rename(const char *oldfile, const char *newfile);
 extern int	fsync_parent_path(const char *fname);
-- 
2.39.5 (Apple Git-154)

>From e70eea50e81d8f40fb8db15c06b23305d4b8698f Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 6 Nov 2024 09:52:19 -0600
Subject: [PATCH v1 5/8] Export pre_sync_fname().

THIS IS A PROOF OF CONCEPT AND IS NOT READY FOR SERIOUS REVIEW.

A follow-up commit will use this function to alert the file system
that we want a file's data on disk so that subsequent calls to
fsync() are faster.
---
 src/common/file_utils.c         | 18 +++++-------------
 src/include/common/file_utils.h |  1 +
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 65cdf07ae7..5c201ec6e8 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -45,10 +45,6 @@
  */
 #define MINIMUM_VERSION_FOR_PG_WAL	100000
 
-#ifdef PG_FLUSH_DATA_WORKS
-static int	pre_sync_fname(const char *fname, bool isdir, void *arg);
-#endif
-
 #ifdef HAVE_SYNCFS
 
 /*
@@ -307,11 +303,7 @@ walkdir(const char *path,
 				 * introduce a huge number of function pointer calls and
 				 * directory reads that we are trying to avoid.
 				 */
-#ifdef PG_FLUSH_DATA_WORKS
 				if ((action != pre_sync_fname && action != fsync_fname) ||
-#else
-				if (action != fsync_fname ||
-#endif
 					!arg || *((bool *) arg) ||
 					strcmp(de->d_name, "base") != 0)
 					walkdir(subpath, action, false, arg);
@@ -348,11 +340,12 @@ walkdir(const char *path,
  * Ignores errors trying to open unreadable files, and reports other errors
  * non-fatally.
  */
-#ifdef PG_FLUSH_DATA_WORKS
-
-static int
+int
 pre_sync_fname(const char *fname, bool isdir, void *arg)
 {
+#ifndef PG_FLUSH_DATA_WORKS
+	return 0;
+#else
 	int			fd;
 
 	fd = open(fname, O_RDONLY | PG_BINARY, 0);
@@ -380,9 +373,8 @@ pre_sync_fname(const char *fname, bool isdir, void *arg)
 
 	(void) close(fd);
 	return 0;
-}
-
 #endif							/* PG_FLUSH_DATA_WORKS */
+}
 
 /*
  * fsync_fname -- Try to fsync a file or directory
diff --git a/src/include/common/file_utils.h b/src/include/common/file_utils.h
index 3743caa63e..e7a34d4c4e 100644
--- a/src/include/common/file_utils.h
+++ b/src/include/common/file_utils.h
@@ -43,6 +43,7 @@ extern int	fsync_parent_path(const char *fname);
 extern void walkdir(const char *path,
 					int (*action) (const char *fname, bool isdir, void *arg),
 					bool process_symlinks, void *arg);
+extern int	pre_sync_fname(const char *fname, bool isdir, void *arg);
 #endif
 
 extern PGFileType get_dirent_type(const char *path,
-- 
2.39.5 (Apple Git-154)

>From 5d17fd66e08612574ffe0c39ff7624259319059c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 6 Nov 2024 10:40:43 -0600
Subject: [PATCH v1 6/8] In pg_upgrade's catalog-swap mode, only sync files as
 necessary.

THIS IS A PROOF OF CONCEPT AND IS NOT READY FOR SERIOUS REVIEW.

In this mode, it can be much faster to use "--sync-method fsync",
which now skips synchronizing data files moved from the old cluster
(which we assumed were synchronized before pg_upgrade).
---
 src/bin/pg_upgrade/pg_upgrade.c    |  6 ++--
 src/bin/pg_upgrade/relfilenumber.c | 52 ++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 663235816f..f5946ac89a 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -210,10 +210,12 @@ main(int argc, char **argv)
 	{
 		prep_status("Sync data directory to disk");
 		exec_prog(UTILITY_LOG_FILE, NULL, true, true,
-				  "\"%s/initdb\" --sync-only \"%s\" --sync-method %s",
+				  "\"%s/initdb\" --sync-only \"%s\" --sync-method %s %s",
 				  new_cluster.bindir,
 				  new_cluster.pgdata,
-				  user_opts.sync_method);
+				  user_opts.sync_method,
+				  (user_opts.transfer_mode == TRANSFER_MODE_CATALOG_SWAP) ?
+				  "--no-sync-data-files" : "");
 		check_ok();
 	}
 
diff --git a/src/bin/pg_upgrade/relfilenumber.c b/src/bin/pg_upgrade/relfilenumber.c
index 9d8fce3c4a..dcca4bb2e7 100644
--- a/src/bin/pg_upgrade/relfilenumber.c
+++ b/src/bin/pg_upgrade/relfilenumber.c
@@ -25,8 +25,49 @@ typedef struct move_catalog_file_context
 	FileNameMap *maps;
 	int			size;
 	char	   *target;
+	bool		sync_moved;
 } move_catalog_file_context;
 
+#define SYNC_QUEUE_MAX_LEN (1024)
+
+static char *sync_queue[SYNC_QUEUE_MAX_LEN];
+static bool sync_queue_inited;
+static int	sync_queue_len;
+
+static inline void
+sync_queue_init(void)
+{
+	if (sync_queue_inited)
+		return;
+
+	sync_queue_inited = true;
+	for (int i = 0; i < SYNC_QUEUE_MAX_LEN; i++)
+		sync_queue[i] = palloc(MAXPGPATH);
+}
+
+static inline void
+sync_queue_sync_all(void)
+{
+	if (!sync_queue_inited)
+		return;
+
+	for (int i = 0; i < sync_queue_len; i++)
+		fsync_fname(sync_queue[i], false, NULL);
+	sync_queue_len = 0;
+}
+
+static inline void
+sync_queue_push(const char *fname)
+{
+	sync_queue_init();
+
+	pre_sync_fname(fname, false, NULL);
+
+	strncpy(sync_queue[sync_queue_len++], fname, MAXPGPATH);
+	if (sync_queue_len >= SYNC_QUEUE_MAX_LEN)
+		sync_queue_sync_all();
+}
+
 /*
  * transfer_all_new_tablespaces()
  *
@@ -138,6 +179,8 @@ transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 		/* We allocate something even for n_maps == 0 */
 		pg_free(mappings);
 	}
+
+	sync_queue_sync_all();
 }
 
 static int
@@ -195,6 +238,9 @@ move_catalog_file(const char *fname, bool isdir, void *arg)
 	if (rename(fname, dst) != 0)
 		pg_fatal("could not rename \"%s\" to \"%s\": %m", fname, dst);
 
+	if (context->sync_moved)
+		sync_queue_push(dst);
+
 	return 0;
 }
 
@@ -250,10 +296,12 @@ do_catalog_transfer(FileNameMap *maps, int size, char *old_tablespace)
 	context.maps = maps;
 	context.size = size;
 	context.target = old_cat;
+	context.sync_moved = false;
 	walkdir(new_dat, move_catalog_file, false, &context);
 
 	/* move catalogs in moved-aside data dir in place */
 	context.target = new_dat;
+	context.sync_moved = (sync_method != DATA_DIR_SYNC_METHOD_SYNCFS);
 	walkdir(moved_dat, move_catalog_file, false, &context);
 
 	/* no need to sync things individually if we are going to syncfs() later */
@@ -265,6 +313,8 @@ do_catalog_transfer(FileNameMap *maps, int size, char *old_tablespace)
 		pg_fatal("could not synchronize directory \"%s\": %m", moved_dat);
 	if (fsync_fname(old_cat, true, NULL) != 0)
 		pg_fatal("could not synchronize directory \"%s\": %m", old_cat);
+	if (fsync_fname(new_dat, true, NULL) != 0)
+		pg_fatal("could not synchronize directory \"%s\": %m", new_dat);
 
 	/*
 	 * XXX: We could instead fsync() these directories once at the end instead
@@ -276,6 +326,8 @@ do_catalog_transfer(FileNameMap *maps, int size, char *old_tablespace)
 		pg_fatal("could not synchronize directory \"%s\": %m", old_tblspc);
 	if (fsync_fname(moved_tblspc, true, NULL) != 0)
 		pg_fatal("could not synchronize directory \"%s\": %m", moved_tblspc);
+	if (fsync_fname(new_tblspc, true, NULL) != 0)
+		pg_fatal("could not synchronize directory \"%s\": %m", new_tblspc);
 }
 
 /*
-- 
2.39.5 (Apple Git-154)

>From 234d40f4e45d56f40465037dcf83c8c980edf095 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 6 Nov 2024 10:46:11 -0600
Subject: [PATCH v1 7/8] Add --sequence-data flag to pg_dump.

THIS IS A PROOF OF CONCEPT AND IS NOT READY FOR SERIOUS REVIEW.

This flag can be used to optionally dump the sequence data even
when --schema-only is used.  It is primarily intended for use in a
follow-up commit that will cause sequence data files to be carried
over from the old cluster in pg_upgrade's new catalog-swap mode.
---
 src/bin/pg_dump/pg_dump.c                   | 9 +--------
 src/bin/pg_upgrade/dump.c                   | 2 +-
 src/test/modules/test_pg_dump/t/001_base.pl | 2 +-
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index b2f4eb2c6d..2b57abd305 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -501,6 +501,7 @@ main(int argc, char **argv)
 		{"sync-method", required_argument, NULL, 15},
 		{"filter", required_argument, NULL, 16},
 		{"exclude-extension", required_argument, NULL, 17},
+		{"sequence-data", no_argument, &dopt.sequence_data, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -768,14 +769,6 @@ main(int argc, char **argv)
 	if (dopt.column_inserts && dopt.dump_inserts == 0)
 		dopt.dump_inserts = DUMP_DEFAULT_ROWS_PER_INSERT;
 
-	/*
-	 * Binary upgrade mode implies dumping sequence data even in schema-only
-	 * mode.  This is not exposed as a separate option, but kept separate
-	 * internally for clarity.
-	 */
-	if (dopt.binary_upgrade)
-		dopt.sequence_data = 1;
-
 	if (dopt.dataOnly && dopt.schemaOnly)
 		pg_fatal("options -s/--schema-only and -a/--data-only cannot be used together");
 
diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c
index 8345f55be8..8453722833 100644
--- a/src/bin/pg_upgrade/dump.c
+++ b/src/bin/pg_upgrade/dump.c
@@ -53,7 +53,7 @@ generate_old_dump(void)
 
 		parallel_exec_prog(log_file_name, NULL,
 						   "\"%s/pg_dump\" %s --schema-only --quote-all-identifiers "
-						   "--binary-upgrade --format=custom %s --no-sync --file=\"%s/%s\" %s",
+						   "--binary-upgrade --sequence-data --format=custom %s --no-sync --file=\"%s/%s\" %s",
 						   new_cluster.bindir, cluster_conn_opts(&old_cluster),
 						   log_opts.verbose ? "--verbose" : "",
 						   log_opts.dumpdir,
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index e2579e29cd..46231c93f1 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -48,7 +48,7 @@ my %pgdump_runs = (
 		dump_cmd => [
 			'pg_dump', '--no-sync',
 			"--file=$tempdir/binary_upgrade.sql", '--schema-only',
-			'--binary-upgrade', '--dbname=postgres',
+			'--binary-upgrade', '--sequence-data', '--dbname=postgres',
 		],
 	},
 	clean => {
-- 
2.39.5 (Apple Git-154)

>From 51be6c09256272e1ce0360b5376b4a14cd1d9a61 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 6 Nov 2024 10:53:40 -0600
Subject: [PATCH v1 8/8] Avoid copying sequence files in pg_upgrade's
 catalog-swap mode.

THIS IS A PROOF OF CONCEPT AND IS NOT READY FOR SERIOUS REVIEW.

On clusters with many sequences, this can further reduce the amount
of time required to wire up the data files in the new cluster.  If
the sequence data file format changes, this optimization cannot be
used, but that seems rare enough.
---
 src/bin/pg_upgrade/dump.c | 8 +++++++-
 src/bin/pg_upgrade/info.c | 6 +++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c
index 8453722833..d5a81cc29c 100644
--- a/src/bin/pg_upgrade/dump.c
+++ b/src/bin/pg_upgrade/dump.c
@@ -51,10 +51,16 @@ generate_old_dump(void)
 		snprintf(sql_file_name, sizeof(sql_file_name), DB_DUMP_FILE_MASK, old_db->db_oid);
 		snprintf(log_file_name, sizeof(log_file_name), DB_DUMP_LOG_FILE_MASK, old_db->db_oid);
 
+		/*
+		 * XXX: We need to be sure that the sequence data format hasn't
+		 * changed.
+		 */
 		parallel_exec_prog(log_file_name, NULL,
 						   "\"%s/pg_dump\" %s --schema-only --quote-all-identifiers "
-						   "--binary-upgrade --sequence-data --format=custom %s --no-sync --file=\"%s/%s\" %s",
+						   "--binary-upgrade %s --format=custom %s --no-sync --file=\"%s/%s\" %s",
 						   new_cluster.bindir, cluster_conn_opts(&old_cluster),
+						   (user_opts.transfer_mode == TRANSFER_MODE_CATALOG_SWAP) ?
+						   "" : "--sequence-data",
 						   log_opts.verbose ? "--verbose" : "",
 						   log_opts.dumpdir,
 						   sql_file_name, escaped_connstr.data);
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index f83ded89cb..786d17e32f 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -483,6 +483,8 @@ get_rel_infos_query(void)
 	 * pg_largeobject contains user data that does not appear in pg_dump
 	 * output, so we have to copy that system table.  It's easiest to do that
 	 * by treating it as a user table.
+	 *
+	 * XXX: We need to be sure that the sequence data format hasn't changed.
 	 */
 	appendPQExpBuffer(&query,
 					  "WITH regular_heap (reloid, indtable, toastheap) AS ( "
@@ -490,7 +492,7 @@ get_rel_infos_query(void)
 					  "  FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n "
 					  "         ON c.relnamespace = n.oid "
 					  "  WHERE relkind IN (" CppAsString2(RELKIND_RELATION) ", "
-					  CppAsString2(RELKIND_MATVIEW) ") AND "
+					  CppAsString2(RELKIND_MATVIEW) "%s) AND "
 	/* exclude possible orphaned temp tables */
 					  "    ((n.nspname !~ '^pg_temp_' AND "
 					  "      n.nspname !~ '^pg_toast_temp_' AND "
@@ -499,6 +501,8 @@ get_rel_infos_query(void)
 					  "      c.oid >= %u::pg_catalog.oid) OR "
 					  "     (n.nspname = 'pg_catalog' AND "
 					  "      relname IN ('pg_largeobject') ))), ",
+					  (user_opts.transfer_mode == TRANSFER_MODE_CATALOG_SWAP) ?
+					  ", " CppAsString2(RELKIND_SEQUENCE) : "",
 					  FirstNormalObjectId);
 
 	/*
-- 
2.39.5 (Apple Git-154)

---

From: Greg Sabino Mullane <[email-masked]>
Date: Sun, 17 Nov 2024 13:50:53 -0500

On Wed, Nov 6, 2024 at 5:07 PM Nathan Bossart <[email-masked]>
wrote:

> Therefore, it can be much faster to instead move the entire data directory
> from the old cluster

Thank you for breaking this up so clearly into separate commits. I think it
is a very interesting idea, and anything to speed up pg_upgrade is always
welcome. Some minor thoughts:

> [PATCH v1 3/8] Introduce catalog-swap mode for pg_upgrade.
> .. we don't really expect there to be directories within database
directories,
> so perhaps it would be better to either unconditionally rename or to fail.
Failure seems the best option here, so we can cleanly handle any future
cases in which we decide to put dirs in this directory.

> if (RelFileNumberIsValid(rfn))
> {
>   FileNameMap key;
>
>   key.relfilenumber = (RelFileNumber) rfn;
>   if (bsearch(&key, context->maps, context->size,
>      sizeof(FileNameMap), FileNameMapCmp))
>    return 0;
>  }
>
>  snprintf(dst, sizeof(dst), "%s/%s", context->target, filename);

I'm not quite clear what we are doing here with falling through
for InvalidOid entries, could you explain?

> .. vm_must_add_frozenbit isn't handled yet.  We could either disallow
> using catalog-swap mode if the upgrade involves versions older than v9.6

Yes, this. No need for more code to handle super old versions when other
options exist.

> upgrading, there's no reason to synchronize them again at the end.

 Very cool approach!

Cheers,
Greg

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 19 Nov 2024 12:20:00 -0600

> welcome. Some minor thoughts:

Thank you for reviewing!

> cases in which we decide to put dirs in this directory.

Good point.

> for InvalidOid entries, could you explain?

The idea is that if it looks like a data file that we might want to
transfer (i.e., it starts with a RelFileNumber), we should consult our map
to determine whether to move it.  Otherwise, we want to unconditionally
transfer it so that we always use the files generated during pg_restore in
the new cluster (e.g., PG_VERSION and pg_filenode.map).  In theory, this
should result in the same end state as what --link mode does today (for the
new cluster, at least).

> Yes, this. No need for more code to handle super old versions when other

I'm inclined to agree.

>> upgrading, there's no reason to synchronize them again at the end.

:)

-- 
nathan