Hi David, Thanks for the patch. I reproduced all three cases on master with an injection point, and v1 fixes them.
I found one more file with the same problem. A checkpoint rewrites pg_twophase files in place with O_TRUNC. If the backup copies one right after the truncate, it gets an empty file. The restored cluster then fails with FATAL: incorrect size of file "pg_twophase/000000000000029C": 0 bytes v1 cannot help here, since the open file really is empty. The attached patch writes to a temp file and renames it into place. Also, pg_verifybackup reports all of these broken backups as fine. Thanks, Shihao
From c64c652044150782ca1ab6226eac040e09466782 Mon Sep 17 00:00:00 2001 From: shihao zhong <[email protected]> Date: Sun, 25 Oct 2026 00:13:36 -0400 Subject: [PATCH] Write two-phase state files via a temp file and rename. A checkpoint truncates and rewrites pg_twophase files in place, so a concurrent base backup can copy an empty or partial file, and the restored cluster then fails to start. --- src/backend/access/transam/twophase.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 48e478a4ecb..bcd16454197 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1750,6 +1750,7 @@ static void RecreateTwoPhaseFile(FullTransactionId fxid, const void *content, size_t len) { char path[MAXPGPATH]; + char tmppath[MAXPGPATH]; pg_crc32c statefile_crc; int fd; @@ -1760,12 +1761,18 @@ RecreateTwoPhaseFile(FullTransactionId fxid, const void *content, size_t len) TwoPhaseFilePath(path, fxid); - fd = OpenTransientFile(path, + /* + * Write to a temp file and rename it into place, so that a concurrent + * base backup never copies a partially written file. + */ + snprintf(tmppath, sizeof(tmppath), "%s.tmp", path); + + fd = OpenTransientFile(tmppath, O_CREAT | O_TRUNC | O_WRONLY | PG_BINARY); if (fd < 0) ereport(ERROR, (errcode_for_file_access(), - errmsg("could not recreate file \"%s\": %m", path))); + errmsg("could not recreate file \"%s\": %m", tmppath))); /* Write content and CRC */ errno = 0; @@ -1777,7 +1784,7 @@ RecreateTwoPhaseFile(FullTransactionId fxid, const void *content, size_t len) errno = ENOSPC; ereport(ERROR, (errcode_for_file_access(), - errmsg("could not write file \"%s\": %m", path))); + errmsg("could not write file \"%s\": %m", tmppath))); } if (write(fd, &statefile_crc, sizeof(pg_crc32c)) != sizeof(pg_crc32c)) { @@ -1786,7 +1793,7 @@ RecreateTwoPhaseFile(FullTransactionId fxid, const void *content, size_t len) errno = ENOSPC; ereport(ERROR, (errcode_for_file_access(), - errmsg("could not write file \"%s\": %m", path))); + errmsg("could not write file \"%s\": %m", tmppath))); } pgstat_report_wait_end(); @@ -1798,13 +1805,21 @@ RecreateTwoPhaseFile(FullTransactionId fxid, const void *content, size_t len) if (pg_fsync(fd) != 0) ereport(ERROR, (errcode_for_file_access(), - errmsg("could not fsync file \"%s\": %m", path))); + errmsg("could not fsync file \"%s\": %m", tmppath))); pgstat_report_wait_end(); if (CloseTransientFile(fd) != 0) ereport(ERROR, (errcode_for_file_access(), - errmsg("could not close file \"%s\": %m", path))); + errmsg("could not close file \"%s\": %m", tmppath))); + + + /* CheckPointTwoPhase() fsyncs the directory afterwards */ + if (rename(tmppath, path) != 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not rename file \"%s\" to \"%s\": %m", + tmppath, path))); } /* -- 2.37.1 (Apple Git-137.1)
