Hi Michael, > Saying that, could you adjust the 2PC file? I'd just fix that now > and be done with it..
Here it is, with David's points addressed. The temp file is renamed into place and restoreTwoPhaseData() removes stale ones at startup. Thanks, Shihao
From 6bcc86de9d0d29d28ab69a12ee11cf84800f6edc Mon Sep 17 00:00:00 2001 From: shihao zhong <[email protected]> Date: Sun, 25 Oct 2026 17:42:57 -0400 Subject: [PATCH] Write two-phase state files via a temp file and rename. A checkpoint creates and writes each pg_twophase state file in place, so a crash before the write completes leaves an empty or partial file and the next startup fails with "incorrect size of file". A base backup can copy such a file too. Write a temporary file and rename it into place, and remove stale temporary files at startup. --- src/backend/access/transam/twophase.c | 41 +++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 95aa8be9c53..0960f2a7ef8 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -90,6 +90,7 @@ #include "access/xlogutils.h" #include "catalog/pg_type.h" #include "catalog/storage.h" +#include "common/string.h" #include "funcapi.h" #include "miscadmin.h" #include "pg_trace.h" @@ -1721,6 +1722,7 @@ static void RecreateTwoPhaseFile(TransactionId xid, void *content, int len) { char path[MAXPGPATH]; + char tmppath[MAXPGPATH]; pg_crc32c statefile_crc; int fd; @@ -1731,12 +1733,19 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len) TwoPhaseFilePath(path, xid); - fd = OpenTransientFile(path, + /* + * Write to a temp file and rename it into place, so that neither a crash + * nor a concurrent base backup can leave a partially written state file + * under the final name. + */ + 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; @@ -1748,7 +1757,7 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int 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)) { @@ -1757,7 +1766,7 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int 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(); @@ -1769,13 +1778,20 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int 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))); } /* @@ -1889,6 +1905,19 @@ restoreTwoPhaseData(void) cldir = AllocateDir(TWOPHASE_DIR); while ((clde = ReadDir(cldir, TWOPHASE_DIR)) != NULL) { + /* Left behind by a crash while RecreateTwoPhaseFile() was running */ + if (pg_str_endswith(clde->d_name, ".tmp")) + { + char path[MAXPGPATH]; + + snprintf(path, MAXPGPATH, TWOPHASE_DIR "/%s", clde->d_name); + if (unlink(path) < 0) + ereport(WARNING, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + continue; + } + if (strlen(clde->d_name) == 8 && strspn(clde->d_name, "0123456789ABCDEF") == 8) { -- 2.37.1 (Apple Git-137.1)
From 302086af70c50ff25b3f19a2e787fc6d424e4b00 Mon Sep 17 00:00:00 2001 From: shihao zhong <[email protected]> Date: Sun, 25 Oct 2026 17:42:57 -0400 Subject: [PATCH] Write two-phase state files via a temp file and rename. A checkpoint creates and writes each pg_twophase state file in place, so a crash before the write completes leaves an empty or partial file and the next startup fails with "incorrect size of file". A base backup can copy such a file too. Write a temporary file and rename it into place, and remove stale temporary files at startup. --- src/backend/access/transam/twophase.c | 41 +++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 48e478a4ecb..f5061e6679f 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -90,6 +90,7 @@ #include "access/xlogutils.h" #include "catalog/pg_type.h" #include "catalog/storage.h" +#include "common/string.h" #include "funcapi.h" #include "miscadmin.h" #include "pg_trace.h" @@ -1750,6 +1751,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 +1762,19 @@ 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 neither a crash + * nor a concurrent base backup can leave a partially written state file + * under the final name. + */ + 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 +1786,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 +1795,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 +1807,20 @@ 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))); } /* @@ -1918,6 +1934,19 @@ restoreTwoPhaseData(void) cldir = AllocateDir(TWOPHASE_DIR); while ((clde = ReadDir(cldir, TWOPHASE_DIR)) != NULL) { + /* Left behind by a crash while RecreateTwoPhaseFile() was running */ + if (pg_str_endswith(clde->d_name, ".tmp")) + { + char path[MAXPGPATH]; + + snprintf(path, MAXPGPATH, TWOPHASE_DIR "/%s", clde->d_name); + if (unlink(path) < 0) + ereport(WARNING, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + continue; + } + if (strlen(clde->d_name) == 16 && strspn(clde->d_name, "0123456789ABCDEF") == 16) { -- 2.37.1 (Apple Git-137.1)
