From 7fbb62ce673ec84b905edf68959794b5740d7736 Mon Sep 17 00:00:00 2001 From: Marco Nenciarini Date: Mon, 10 Aug 2026 16:37:58 +0200 Subject: [PATCH v1 2/3] pg_upgrade: write manifest of unchanged relation files After pg_upgrade on a primary, resyncing its standbys today means a full re-clone: there is no shared WAL history across the upgrade boundary for pg_rewind or incremental pg_basebackup to use. pg_upgrade already knows, from gen_db_file_maps(), exactly which relations it is about to transfer unchanged. Record that inventory as pg_upgrade_manifest inside the new cluster's data directory: a header line with the old cluster's own system identifier and shutdown checkpoint, one "db_oid relfilenumber" line per relation transferred unchanged, and a trailing line with the new cluster's own post-restore checkpoint. This deliberately isn't a real backup_manifest. Nothing generic ever reads this file, only a future consumer's own code does, so JSON would only buy code reuse in the writer, not actual interoperability, and forcing the format's own identity fields to describe two different clusters at once, the old one it inventories and the new one it happens to sit in, is exactly the kind of misuse a real backup_manifest shouldn't be asked to carry. pg_upgrade_manifest is excluded from ordinary base backups (basebackup.c's own excludeFiles list, alongside backup_manifest for the same reason): it describes the old cluster, not whatever directory it happens to be sitting in, so it has no business riding along into a future backup of the new primary. Nothing else about pg_upgrade's behavior changes. --- src/backend/backup/basebackup.c | 8 + src/bin/pg_basebackup/t/010_pg_basebackup.pl | 13 +- src/bin/pg_upgrade/controldata.c | 38 ++- src/bin/pg_upgrade/pg_upgrade.c | 11 + src/bin/pg_upgrade/pg_upgrade.h | 21 ++ src/bin/pg_upgrade/relfilenumber.c | 254 +++++++++++++++++++ src/bin/pg_upgrade/t/002_pg_upgrade.pl | 23 ++ 7 files changed, 366 insertions(+), 2 deletions(-) diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index fe5ce23aaba..62f6d913ca3 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -223,6 +223,14 @@ static const struct exclude_list_item excludeFiles[] = */ {"backup_manifest", false}, + /* + * pg_upgrade_manifest describes the *old* cluster's own identity and + * checkpoint (see src/bin/pg_upgrade/relfilenumber.c), not this one's; + * carrying it into a backup of this cluster would leave every future + * backup/standby holding a stale, unrelated record. + */ + {"pg_upgrade_manifest", false}, + {"postmaster.pid", false}, {"postmaster.opts", false}, diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index cfcfdb8b580..97fc8606b9a 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -199,6 +199,17 @@ if ($Config{osname} ne 'darwin') close $file; } +# pg_upgrade_manifest (see src/bin/pg_upgrade/relfilenumber.c) describes +# the *old* cluster's own identity and checkpoint, not this one's, so it +# must never ride along into a backup of this cluster: create a dummy one +# here to confirm the exclusion below actually reaches it, rather than +# relying only on pg_upgrade itself ever having produced one. +{ + open my $file, '>>', "$pgdata/pg_upgrade_manifest" or die $!; + print $file "DONOTCOPY"; + close $file; +} + # Connect to a database to create global/pg_internal.init. If this is removed # the test to ensure global/pg_internal.init is not copied will return a false # positive. @@ -267,7 +278,7 @@ foreach my $dirname ( # These files should not be copied. foreach my $filename ( qw(postgresql.auto.conf.tmp postmaster.opts postmaster.pid tablespace_map current_logfiles.tmp - global/pg_internal.init global/pg_internal.init.123)) + global/pg_internal.init global/pg_internal.init.123 pg_upgrade_manifest)) { ok(!-f "$tempdir/backup/$filename", "$filename not copied"); } diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c index b3bd4ccde83..41f61086548 100644 --- a/src/bin/pg_upgrade/controldata.c +++ b/src/bin/pg_upgrade/controldata.c @@ -63,6 +63,8 @@ get_control_data(ClusterInfo *cluster) bool got_data_checksum_version = false; bool got_cluster_state = false; bool got_default_char_signedness = false; + bool got_sysid = false; + bool got_chkpnt = false; char *lc_collate = NULL; char *lc_ctype = NULL; char *lc_monetary = NULL; @@ -166,6 +168,33 @@ get_control_data(ClusterInfo *cluster) } got_cluster_state = true; } + else if ((p = strstr(bufin, "Database system identifier:")) != NULL) + { + p = strchr(p, ':'); + + if (p == NULL || strlen(p) <= 1) + pg_fatal("%d: controldata retrieval problem", __LINE__); + + p++; /* remove ':' char */ + cluster->controldata.sysid = strtou64(p, NULL, 10); + got_sysid = true; + } + else if ((p = strstr(bufin, "Latest checkpoint location:")) != NULL) + { + uint32 hi; + uint32 lo; + + p = strchr(p, ':'); + + if (p == NULL || strlen(p) <= 1) + pg_fatal("%d: controldata retrieval problem", __LINE__); + + p++; /* remove ':' char */ + if (sscanf(p, "%X/%X", &hi, &lo) != 2) + pg_fatal("%d: controldata retrieval problem", __LINE__); + cluster->controldata.chkpnt_loc = ((uint64) hi) << 32 | lo; + got_chkpnt = true; + } } rc = pclose(output); @@ -531,7 +560,8 @@ get_control_data(ClusterInfo *cluster) !got_large_object || !got_date_is_int || !got_data_checksum_version || (!got_default_char_signedness && - cluster->controldata.cat_ver >= DEFAULT_CHAR_SIGNEDNESS_CAT_VER)) + cluster->controldata.cat_ver >= DEFAULT_CHAR_SIGNEDNESS_CAT_VER) || + !got_sysid || !got_chkpnt) { if (cluster == &old_cluster) pg_log(PG_REPORT, @@ -602,6 +632,12 @@ get_control_data(ClusterInfo *cluster) if (!got_default_char_signedness) pg_log(PG_REPORT, " default char signedness"); + if (!got_sysid) + pg_log(PG_REPORT, " database system identifier"); + + if (!got_chkpnt) + pg_log(PG_REPORT, " latest checkpoint location"); + pg_fatal("Cannot continue without required control information, terminating"); } } diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 7366fd4627c..4251a38cb95 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -254,6 +254,17 @@ main(int argc, char **argv) issue_warnings_and_set_wal_level(); + /* + * The control data captured earlier, in check_cluster_compatibility(), is + * for the freshly-initdb'd cluster, before this run's restore and + * transfer steps. issue_warnings_and_set_wal_level() just did one more + * start/stop cycle to fix wal_level, which produced a later shutdown + * checkpoint. Re-read control data now, after every start/stop pg_upgrade + * does, so the checkpoint recorded below is the final one. + */ + get_control_data(&new_cluster); + append_new_cluster_checkpoint(new_cluster.pgdata); + pg_log(PG_REPORT, "\n" "Upgrade Complete\n" diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index d6e5bca5792..c32dce411f5 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -30,6 +30,24 @@ #define GLOBALS_DUMP_FILE "pg_upgrade_dump_globals.sql" #define DB_DUMP_FILE_MASK "pg_upgrade_dump_%u.custom" +/* + * Written inside the new cluster's PGDATA: a small format of its own (not a + * backup_manifest -- nothing but pg_upgrade_replica ever reads this file) + * listing which relations were transferred unchanged from the old cluster. + * A header line carries the old cluster's own system identifier and + * shutdown checkpoint; one "db_oid relfilenumber" line follows per relation + * transferred unchanged; a trailing line, appended once known, carries the + * new cluster's own post-restore checkpoint. Anything not listed here is + * new content produced by this pg_upgrade run (system catalogs, etc). + * + * This only covers the main fork and, when present, the fsm and vm forks + * (the same suffixes transfer_relfile() ever moves); it says nothing about + * an unlogged relation's init fork, which pg_upgrade never transfers at + * all -- it's created fresh by the new cluster's own DDL replay, whether + * or not the relation's other forks are listed here as kept. + */ +#define UPGRADE_MANIFEST_FILE "pg_upgrade_manifest" + /* * Base directories that include all the files generated internally, from the * root path of the new cluster. The paths are dynamically built as of @@ -234,6 +252,8 @@ typedef struct bool float8_pass_by_value; uint32 data_checksum_version; bool default_char_signedness; + uint64 sysid; + uint64 chkpnt_loc; } ControlData; /* @@ -439,6 +459,7 @@ void transfer_all_new_tablespaces(DbInfoArr *old_db_arr, void transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr, char *old_pgdata, char *new_pgdata, char *old_tablespace, char *new_tablespace); +void append_new_cluster_checkpoint(const char *new_pgdata); /* tablespace.c */ diff --git a/src/bin/pg_upgrade/relfilenumber.c b/src/bin/pg_upgrade/relfilenumber.c index 6c467bdc8a5..18faf831944 100644 --- a/src/bin/pg_upgrade/relfilenumber.c +++ b/src/bin/pg_upgrade/relfilenumber.c @@ -19,6 +19,250 @@ static void transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace, char *new_tablespace); static void transfer_relfile(FileNameMap *map, const char *type_suffix); +static void cleanup_stale_upgrade_manifest(const char *new_pgdata); +static void write_upgrade_manifest(const char *new_pgdata, FileNameMap *maps, + int n_maps, char *old_tablespace); +static void finalize_upgrade_manifest(const char *new_pgdata); + +/* + * append_new_cluster_checkpoint() + * + * Appends a "NEW_CHECKPOINT %X/%08X" line to the manifest with the new + * cluster's own final checkpoint location. + * + * Call only after the new cluster is stopped for the last time, and only + * after a fresh get_control_data() call for new_cluster: an earlier control + * data snapshot (e.g. the one from check_cluster_compatibility()) is from + * before this run's restore/transfer work, so it has the wrong checkpoint. + * + * pg_upgrade always leaves the new cluster cleanly stopped, so this + * checkpoint is real and already on disk. pg_resetwal guarantees it is past + * any LSN the old cluster ever wrote (see copy_xact_xlog_xid()'s "Resetting + * WAL archives" step). This lets a consumer start WAL replay from this + * checkpoint, with no live pg_backup_start()/pg_backup_stop() call needed, + * and still safely catch up on any write made to the new primary before the + * consumer got around to reading this manifest. + */ +void +append_new_cluster_checkpoint(const char *new_pgdata) +{ + char path[MAXPGPATH]; + FILE *f; + + snprintf(path, sizeof(path), "%s/%s", new_pgdata, UPGRADE_MANIFEST_FILE); + + f = fopen(path, "ab"); + if (f == NULL) + pg_fatal("could not open file \"%s\": %m", path); + + fprintf(f, "NEW_CHECKPOINT %X/%08X\n", + (uint32) (new_cluster.controldata.chkpnt_loc >> 32), + (uint32) new_cluster.controldata.chkpnt_loc); + + if (fclose(f) != 0) + pg_fatal("could not write file \"%s\": %m", path); + + fsync_fname(path, false); +} + +/* + * cleanup_stale_upgrade_manifest() + * + * Removes any pg_upgrade_manifest and .part.* fragments left over from an + * earlier, unfinished attempt against this same new_pgdata (see the + * fragment-file scheme in write_upgrade_manifest()'s comment). Call before + * any worker can write a fragment. + */ +static void +cleanup_stale_upgrade_manifest(const char *new_pgdata) +{ + char path[MAXPGPATH]; + char fragment_prefix[MAXPGPATH]; + DIR *dir; + struct dirent *de; + + snprintf(path, sizeof(path), "%s/%s", new_pgdata, UPGRADE_MANIFEST_FILE); + if (unlink(path) != 0 && errno != ENOENT) + pg_fatal("could not remove file \"%s\": %m", path); + + snprintf(fragment_prefix, sizeof(fragment_prefix), "%s.part.", + UPGRADE_MANIFEST_FILE); + + dir = opendir(new_pgdata); + if (dir == NULL) + pg_fatal("could not open directory \"%s\": %m", new_pgdata); + + while (errno = 0, (de = readdir(dir)) != NULL) + { + char fragment_path[MAXPGPATH]; + + if (strncmp(de->d_name, fragment_prefix, strlen(fragment_prefix)) != 0) + continue; + + snprintf(fragment_path, sizeof(fragment_path), "%s/%s", + new_pgdata, de->d_name); + if (unlink(fragment_path) != 0) + pg_fatal("could not remove file \"%s\": %m", fragment_path); + } + if (errno) + pg_fatal("could not read directory \"%s\": %m", new_pgdata); + closedir(dir); +} + +/* + * With --jobs > 1, transfer_all_new_tablespaces() runs one worker + * (process, or thread on Windows) per tablespace, all writing to the + * manifest at the same time. To avoid two workers writing the same file at + * once, write_upgrade_manifest() has each worker append only to its own + * fragment file, named by its pid (GetCurrentThreadId() on Windows, since + * --jobs there means threads, not processes). Once every worker is done, + * finalize_upgrade_manifest() joins all fragments into the real manifest + * and deletes them. cleanup_stale_upgrade_manifest() removes fragments + * left over from an earlier, unfinished run before a new run starts. + */ + +/* + * write_upgrade_manifest() + * + * Appends this call's (db_oid, relfilenumber) pairs to this worker's + * manifest fragment (see the comment above). + * + * old_tablespace picks which entries of "maps" to write, using the same + * check transfer_single_new_db() uses to pick which files to transfer: + * gen_db_file_maps() returns every relation in the database regardless of + * tablespace, and with --jobs > 1 this function is called once per + * (database, tablespace) pair. Without this filter, every worker would + * write the database's whole, unfiltered relation list into its own + * fragment, and the joined manifest would hold one duplicate copy of the + * whole list per tablespace. + */ +static void +write_upgrade_manifest(const char *new_pgdata, FileNameMap *maps, int n_maps, + char *old_tablespace) +{ + char path[MAXPGPATH]; + FILE *f = NULL; + + for (int i = 0; i < n_maps; i++) + { + if (old_tablespace != NULL && + strcmp(maps[i].old_tablespace, old_tablespace) != 0) + continue; + + if (f == NULL) + { + snprintf(path, sizeof(path), "%s/%s.part.%lu", new_pgdata, + UPGRADE_MANIFEST_FILE, +#ifdef WIN32 + (unsigned long) GetCurrentThreadId() +#else + (unsigned long) getpid() +#endif + ); + + f = fopen(path, "ab"); + if (f == NULL) + pg_fatal("could not open file \"%s\": %m", path); + } + + /* + * Only the two integers, nothing else: nspname/relname are SQL + * identifiers and can contain a literal newline, which would break + * this line-based format. + */ + fprintf(f, "%u %u\n", maps[i].db_oid, maps[i].relfilenumber); + } + + if (f != NULL && fclose(f) != 0) + pg_fatal("could not write file \"%s\": %m", path); +} + +/* + * finalize_upgrade_manifest() + * + * Writes the manifest header: old cluster's system identifier and + * checkpoint location. A consumer needs these to check that its local copy + * of the old cluster (e.g. a standby's data directory) was caught up to + * this exact checkpoint before trusting any "unchanged" file listed here. + * Without that check, a replica lagging behind the old primary's last + * checkpoint would keep serving stale data forever, since replay against + * the new cluster only ever moves forward from the new checkpoint. + * + * Then appends every worker's fragment (see the comment above + * write_upgrade_manifest()) into the real pg_upgrade_manifest and deletes + * the fragments. + * + * Call only after every worker has been reaped: fragments are private to + * one worker until this point, so this function and the workers must never + * run at the same time. + * + * append_new_cluster_checkpoint() adds a further NEW_CHECKPOINT line later, + * once the new cluster has been stopped for the last time and its final + * checkpoint is known. + */ +static void +finalize_upgrade_manifest(const char *new_pgdata) +{ + char path[MAXPGPATH]; + char fragment_prefix[MAXPGPATH]; + FILE *f; + DIR *dir; + struct dirent *de; + + snprintf(path, sizeof(path), "%s/%s", new_pgdata, UPGRADE_MANIFEST_FILE); + snprintf(fragment_prefix, sizeof(fragment_prefix), "%s.part.", + UPGRADE_MANIFEST_FILE); + + f = fopen(path, "wb"); + if (f == NULL) + pg_fatal("could not open file \"%s\": %m", path); + + fprintf(f, "PG_UPGRADE_MANIFEST 1 " UINT64_FORMAT " %X/%08X\n", + old_cluster.controldata.sysid, + (uint32) (old_cluster.controldata.chkpnt_loc >> 32), + (uint32) old_cluster.controldata.chkpnt_loc); + + dir = opendir(new_pgdata); + if (dir == NULL) + pg_fatal("could not open directory \"%s\": %m", new_pgdata); + + while (errno = 0, (de = readdir(dir)) != NULL) + { + char fragment_path[MAXPGPATH]; + FILE *frag; + char buf[8192]; + size_t nread; + + if (strncmp(de->d_name, fragment_prefix, strlen(fragment_prefix)) != 0) + continue; + + snprintf(fragment_path, sizeof(fragment_path), "%s/%s", + new_pgdata, de->d_name); + + frag = fopen(fragment_path, "rb"); + if (frag == NULL) + pg_fatal("could not open file \"%s\": %m", fragment_path); + while ((nread = fread(buf, 1, sizeof(buf), frag)) > 0) + { + if (fwrite(buf, 1, nread, f) != nread) + pg_fatal("could not write file \"%s\": %m", path); + } + if (ferror(frag)) + pg_fatal("could not read file \"%s\": %m", fragment_path); + fclose(frag); + + if (unlink(fragment_path) != 0) + pg_fatal("could not remove file \"%s\": %m", fragment_path); + } + if (errno) + pg_fatal("could not read directory \"%s\": %m", new_pgdata); + closedir(dir); + + if (fclose(f) != 0) + pg_fatal("could not write file \"%s\": %m", path); + + fsync_fname(path, false); +} /* * The following set of sync_queue_* functions are used for --swap to reduce @@ -108,6 +352,8 @@ void transfer_all_new_tablespaces(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr, char *old_pgdata, char *new_pgdata) { + cleanup_stale_upgrade_manifest(new_pgdata); + switch (user_opts.transfer_mode) { case TRANSFER_MODE_CLONE: @@ -157,6 +403,13 @@ transfer_all_new_tablespaces(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr, ; } + /* + * Safe only now: every worker that could have called + * write_upgrade_manifest() has been reaped, so there's no longer any + * concurrent writer to race against. + */ + finalize_upgrade_manifest(new_pgdata); + end_progress_output(); check_ok(); } @@ -204,6 +457,7 @@ transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr, mappings = gen_db_file_maps(old_db, new_db, &n_maps, old_pgdata, new_pgdata); + write_upgrade_manifest(new_pgdata, mappings, n_maps, old_tablespace); if (n_maps) { transfer_single_new_db(mappings, n_maps, old_tablespace, new_tablespace); diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index 0a4121fdc4d..f856790163d 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -579,6 +579,29 @@ command_ok( ok( !-d $newnode->data_dir . "/pg_upgrade_output.d", "pg_upgrade_output.d/ removed after pg_upgrade success"); +# pg_upgrade_manifest lists every relation file transferred unchanged, for +# a standby-resync client to consume later; check its format independent +# of any such client, since nothing else in this file exercises it. It's a +# small, purpose-built line format (see relfilenumber.c): a header line with +# the old cluster's own identity and shutdown checkpoint, one "db_oid +# relfilenumber" line per relation transferred unchanged, and a trailing +# NEW_CHECKPOINT line with the new cluster's own post-restore checkpoint. +my $manifest_path = $newnode->data_dir . "/pg_upgrade_manifest"; +ok(-f $manifest_path, "pg_upgrade_manifest exists in the new cluster"); + +my $manifest_contents = slurp_file($manifest_path); +like( + $manifest_contents, + qr/^PG_UPGRADE_MANIFEST 1 \d+ [0-9A-Fa-f]+\/[0-9A-Fa-f]+$/m, + "pg_upgrade_manifest has a version-1 header with the old cluster's identity" +); +like($manifest_contents, qr/^\d+ \d+$/m, + "pg_upgrade_manifest lists at least one unchanged relation"); +like( + $manifest_contents, + qr/^NEW_CHECKPOINT [0-9A-Fa-f]+\/[0-9A-Fa-f]+$/m, + "pg_upgrade_manifest has the new cluster's checkpoint"); + $newnode->start; # Check if there are any logs coming from pg_upgrade, that would only be -- 2.47.3