From e767dd37e780ccce99ddcc56466c1733b8f9fcef Mon Sep 17 00:00:00 2001 From: Shubhra Jain Date: Tue, 13 Oct 2026 17:03:32 +0530 Subject: [PATCH v1] pg_upgrade: Prevent premature startup of the new cluster pg_upgrade checks only once, at the beginning, that the new cluster is not running. Nothing prevented the new cluster from being started while pg_upgrade was still modifying it, for example during file transfer, pg_resetwal or the final sync, regardless of transfer mode. A server started in that window can write WAL, consume transaction IDs and modify files that pg_upgrade is still transferring, while pg_upgrade may still report success, leading to silent corruption. To prevent this, pg_upgrade now creates a file named pg_upgrade_in_progress in the new cluster's data directory before it first modifies the cluster, and removes it only after the upgrade has completed. Both the creation and the removal are fsynced. While the file exists, the server refuses to start, except in binary-upgrade mode, which pg_upgrade uses for the servers it starts itself. If pg_upgrade fails or is interrupted, the file is left in place, so that a partially upgraded cluster cannot be started by accident. Author: Shubhra Jain Suggested-by: Nikolay Samokhvalov Discussion: https://postgr.es/m/CAM527d_MAVFE8RVM_Qr8-P4opUSxemY2r=MeEiV+odTu6dw4cw@mail.gmail.com --- doc/src/sgml/ref/pgupgrade.sgml | 12 ++++ src/backend/utils/init/miscinit.c | 28 ++++++++ src/bin/pg_upgrade/pg_upgrade.c | 88 ++++++++++++++++++++++++++ src/bin/pg_upgrade/t/002_pg_upgrade.pl | 15 +++++ src/include/access/xlog_internal.h | 8 +++ 5 files changed, 151 insertions(+) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index e4e8c02e6d..f5aaf6586e 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -957,6 +957,18 @@ psql --username=postgres --file=script.sql postgres + + Once pg_upgrade starts modifying the new + cluster, it creates the file pg_upgrade_in_progress + in the new cluster's data directory, and removes it only after the + upgrade has completed successfully. While this file exists, the new + cluster cannot be started, which prevents an accidental startup from + modifying the new cluster while pg_upgrade + is still working on it. If pg_upgrade fails + or is interrupted, the file is left in place, and the new cluster must + be re-created with initdb before the upgrade + is attempted again. + diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index eddce1ce33..fea1494787 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -30,6 +30,7 @@ #include "access/htup_details.h" #include "access/parallel.h" +#include "access/xlog_internal.h" #include "catalog/pg_authid.h" #include "common/file_perm.h" #include "libpq/libpq.h" @@ -297,6 +298,7 @@ void checkDataDir(void) { struct stat stat_buf; + char upgrade_file[MAXPGPATH]; Assert(DataDir); @@ -380,6 +382,32 @@ checkDataDir(void) /* Check for PG_VERSION */ ValidatePgVersion(DataDir); + + /* + * Refuse to start if pg_upgrade is in the middle of upgrading this data + * directory. pg_upgrade creates this file before it starts modifying the + * new cluster and removes it only once the cluster is safe to start. A + * server started in the meantime could write WAL, consume transaction IDs + * and modify files that pg_upgrade is still transferring, which can lead + * to silent corruption. pg_upgrade starts its own servers in + * binary-upgrade mode, so allow those. + */ + snprintf(upgrade_file, sizeof(upgrade_file), "%s/%s", + DataDir, PG_UPGRADE_IN_PROGRESS_FILE); + if (stat(upgrade_file, &stat_buf) == 0) + { + if (!IsBinaryUpgrade) + ereport(FATAL, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("data directory \"%s\" is being upgraded by pg_upgrade", + DataDir), + errdetail("File \"%s\" exists.", upgrade_file), + errhint("Wait for pg_upgrade to complete. If pg_upgrade failed, the new cluster must be re-created with initdb."))); + } + else if (errno != ENOENT) + ereport(FATAL, + (errcode_for_file_access(), + errmsg("could not stat file \"%s\": %m", upgrade_file))); } /* diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index c0fadb3f31..74fa135c30 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -41,11 +41,15 @@ #include "postgres_fe.h" +#include #include +#include #include "access/multixact.h" +#include "access/xlog_internal.h" #include "catalog/pg_class_d.h" #include "common/file_perm.h" +#include "common/file_utils.h" #include "common/logging.h" #include "common/restricted_token.h" #include "fe_utils/string_utils.h" @@ -67,6 +71,8 @@ static void copy_xact_xlog_xid(void); static void set_frozenxids(void); static void make_outputdirs(char *pgdata); static void setup(char *argv0); +static void create_upgrade_in_progress_file(void); +static void remove_upgrade_in_progress_file(void); static void create_logical_replication_slots(void); static void create_conflict_detection_slot(void); @@ -150,6 +156,12 @@ main(int argc, char **argv) "Performing Upgrade\n" "------------------"); + /* + * From here on, the new cluster gets modified. Prevent it from being + * started by anything other than pg_upgrade until the upgrade completes. + */ + create_upgrade_in_progress_file(); + set_locale_and_encoding(); prepare_new_cluster(); @@ -254,6 +266,9 @@ main(int argc, char **argv) issue_warnings_and_set_wal_level(); + /* The new cluster is now safe to start. */ + remove_upgrade_in_progress_file(); + pg_log(PG_REPORT, "\n" "Upgrade Complete\n" @@ -268,6 +283,79 @@ main(int argc, char **argv) return 0; } +/* + * create_upgrade_in_progress_file() + * + * Create a marker file in the new cluster's data directory. While it exists, + * the server refuses to start unless it runs in binary-upgrade mode, as the + * servers started by pg_upgrade do (see checkDataDir()). The file and its + * directory entry are synced to disk, so the protection survives a crash. + */ +static void +create_upgrade_in_progress_file(void) +{ + char path[MAXPGPATH]; + int fd; + + /* translator: %s is the name of a file */ + prep_status("Creating \"%s\" in new cluster", PG_UPGRADE_IN_PROGRESS_FILE); + + snprintf(path, sizeof(path), "%s/%s", new_cluster.pgdata, + PG_UPGRADE_IN_PROGRESS_FILE); + + fd = open(path, O_WRONLY | O_CREAT | O_EXCL | PG_BINARY, + pg_file_create_mode); + if (fd < 0) + { + if (errno == EEXIST) + pg_fatal("file \"%s\" already exists\n" + "A previous run of pg_upgrade on the new cluster might have failed.\n" + "Re-create the new cluster with initdb and try again.", + path); + pg_fatal("could not create file \"%s\": %m", path); + } + if (close(fd) != 0) + pg_fatal("could not close file \"%s\": %m", path); + + /* Ensure the file and its directory entry are persisted to disk. */ + if (fsync_fname(path, false) != 0) + pg_fatal("could not synchronize file \"%s\": %m", path); + if (fsync_fname(new_cluster.pgdata, true) != 0) + pg_fatal("could not synchronize directory \"%s\": %m", + new_cluster.pgdata); + + check_ok(); +} + +/* + * remove_upgrade_in_progress_file() + * + * Remove the marker file created by create_upgrade_in_progress_file(), once + * the new cluster is safe to start. The removal is synced to disk, so the + * file does not reappear after a crash. + */ +static void +remove_upgrade_in_progress_file(void) +{ + char path[MAXPGPATH]; + + /* translator: %s is the name of a file */ + prep_status("Removing \"%s\" from new cluster", PG_UPGRADE_IN_PROGRESS_FILE); + + snprintf(path, sizeof(path), "%s/%s", new_cluster.pgdata, + PG_UPGRADE_IN_PROGRESS_FILE); + + if (unlink(path) != 0) + pg_fatal("could not remove file \"%s\": %m", path); + + /* Ensure the removal is persisted to disk. */ + if (fsync_fname(new_cluster.pgdata, true) != 0) + pg_fatal("could not synchronize directory \"%s\": %m", + new_cluster.pgdata); + + check_ok(); +} + /* * Create and assign proper permissions to the set of output directories * used to store any data generated internally, filling in log_opts in diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index 9590625fc3..512dab50e5 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -632,6 +632,8 @@ command_ok( 'run of pg_upgrade --check for new instance'); ok(!-d $newnode->data_dir . "/pg_upgrade_output.d", "pg_upgrade_output.d/ removed after pg_upgrade --check success"); +ok(!-e $newnode->data_dir . "/pg_upgrade_in_progress", + "pg_upgrade_in_progress not created by pg_upgrade --check"); # Actual run, pg_upgrade_output.d is removed at the end. command_ok( @@ -649,6 +651,19 @@ command_ok( 'run of pg_upgrade for new instance'); ok( !-d $newnode->data_dir . "/pg_upgrade_output.d", "pg_upgrade_output.d/ removed after pg_upgrade success"); +ok(!-e $newnode->data_dir . "/pg_upgrade_in_progress", + "pg_upgrade_in_progress removed after pg_upgrade success"); + +# The server must refuse to start while pg_upgrade_in_progress exists. +my $upgrade_marker = $newnode->data_dir . "/pg_upgrade_in_progress"; +append_to_file($upgrade_marker, ""); +ok(!$newnode->start(fail_ok => 1), + 'server does not start while pg_upgrade_in_progress exists'); +ok( $newnode->log_contains( + qr/data directory ".*" is being upgraded by pg_upgrade/), + 'server log reports that pg_upgrade is in progress'); +unlink($upgrade_marker) + or die "could not remove \"$upgrade_marker\": $!"; $newnode->start; diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h index bf609c3c70..33359a794c 100644 --- a/src/include/access/xlog_internal.h +++ b/src/include/access/xlog_internal.h @@ -148,6 +148,14 @@ typedef XLogLongPageHeaderData *XLogLongPageHeader; #define XLOGDIR "pg_wal" #define XLOG_CONTROL_FILE "global/pg_control" +/* + * pg_upgrade creates this file in the new cluster's data directory while an + * upgrade is in progress. The server refuses to start while it exists, + * except in binary-upgrade mode. Path is relative to $PGDATA. + */ +#define PG_UPGRADE_IN_PROGRESS_FILE "pg_upgrade_in_progress" + + /* * These macros encapsulate knowledge about the exact layout of XLog file * names, timeline history file names, and archive-status file names. -- 2.43.0