Hi all, I have been reviewing the use of errno in the backend code when %m is used in the logs, and found more places than when I looked at improving the error messages for read() calls which bloat the errno value because of intermediate system calls. As the problem is separate and should be back-patched, I have preferred beginning a new thread.
A couple of places use CloseTransientFile without bothering much that this can overwrite errno. I was tempted in keeping errno saved and kept if set to a non-zero value, but refrained from doing so as some callers may rely on the existing behavior, and the attached shows better the intention. Attached is a patch with everything I have spotted. Any opinions or thoughts in getting this back-patched? Thanks, -- Michael
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 65194db70e..e5571a67d6 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1241,12 +1241,17 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
*/
if (fstat(fd, &stat))
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
if (give_warnings)
+ {
+ errno = save_errno;
ereport(WARNING,
(errcode_for_file_access(),
errmsg("could not stat two-phase state file \"%s\": %m",
path)));
+ }
return NULL;
}
@@ -1274,13 +1279,18 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_READ);
if (read(fd, buf, stat.st_size) != stat.st_size)
{
+ int save_errno = errno;
+
pgstat_report_wait_end();
CloseTransientFile(fd);
if (give_warnings)
+ {
+ errno = save_errno;
ereport(WARNING,
(errcode_for_file_access(),
errmsg("could not read two-phase state file \"%s\": %m",
path)));
+ }
pfree(buf);
return NULL;
}
@@ -1663,16 +1673,22 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_WRITE);
if (write(fd, content, len) != len)
{
+ int save_errno = errno;
+
pgstat_report_wait_end();
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write two-phase state file: %m")));
}
if (write(fd, &statefile_crc, sizeof(pg_crc32c)) != sizeof(pg_crc32c))
{
+ int save_errno = errno;
+
pgstat_report_wait_end();
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write two-phase state file: %m")));
@@ -1686,7 +1702,10 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_SYNC);
if (pg_fsync(fd) != 0)
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not fsync two-phase state file: %m")));
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index adbd6a2126..1a419aa49b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3268,7 +3268,10 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
pgstat_report_wait_start(WAIT_EVENT_WAL_INIT_SYNC);
if (pg_fsync(fd) != 0)
{
+ int save_errno = errno;
+
close(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not fsync file \"%s\": %m", tmppath)));
@@ -11675,8 +11678,10 @@ retry:
if (lseek(readFile, (off_t) readOff, SEEK_SET) < 0)
{
char fname[MAXFNAMELEN];
+ int save_errno = errno;
XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size);
+ errno = save_errno;
ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
(errcode_for_file_access(),
errmsg("could not seek in log segment %s to offset %u: %m",
@@ -11688,9 +11693,11 @@ retry:
if (read(readFile, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
{
char fname[MAXFNAMELEN];
+ int save_errno = errno;
pgstat_report_wait_end();
XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size);
+ errno = save_errno;
ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
(errcode_for_file_access(),
errmsg("could not read from log segment %s, offset %u: %m",
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 52fe55e2af..4ecdc9220f 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -718,9 +718,10 @@ XLogRead(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
{
char path[MAXPGPATH];
+ int save_errno = errno;
XLogFilePath(path, tli, sendSegNo, segsize);
-
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not seek in log segment %s to offset %u: %m",
@@ -741,9 +742,10 @@ XLogRead(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
if (readbytes <= 0)
{
char path[MAXPGPATH];
+ int save_errno = errno;
XLogFilePath(path, tli, sendSegNo, segsize);
-
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read from log segment %s, offset %u, length %lu: %m",
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 5688cbe2e9..3f1eae38a9 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -495,6 +495,8 @@ perform_base_backup(basebackup_options *opt)
fp = AllocateFile(pathbuf, "rb");
if (fp == NULL)
{
+ int save_errno = errno;
+
/*
* Most likely reason for this is that the file was already
* removed by a checkpoint, so check for that to get a better
@@ -502,6 +504,7 @@ perform_base_backup(basebackup_options *opt)
*/
CheckXLogRemoved(segno, tli);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not open file \"%s\": %m", pathbuf)));
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 963878a5d8..ddcff3b643 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -578,7 +578,10 @@ CheckPointReplicationOrigin(void)
/* write magic */
if ((write(tmpfd, &magic, sizeof(magic))) != sizeof(magic))
{
+ int save_errno = errno;
+
CloseTransientFile(tmpfd);
+ errno = save_errno;
ereport(PANIC,
(errcode_for_file_access(),
errmsg("could not write to file \"%s\": %m",
@@ -617,7 +620,10 @@ CheckPointReplicationOrigin(void)
if ((write(tmpfd, &disk_state, sizeof(disk_state))) !=
sizeof(disk_state))
{
+ int save_errno = errno;
+
CloseTransientFile(tmpfd);
+ errno = save_errno;
ereport(PANIC,
(errcode_for_file_access(),
errmsg("could not write to file \"%s\": %m",
@@ -633,7 +639,10 @@ CheckPointReplicationOrigin(void)
FIN_CRC32C(crc);
if ((write(tmpfd, &crc, sizeof(crc))) != sizeof(crc))
{
+ int save_errno = errno;
+
CloseTransientFile(tmpfd);
+ errno = save_errno;
ereport(PANIC,
(errcode_for_file_access(),
errmsg("could not write to file \"%s\": %m",
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 4123cdebcf..b0f82391b4 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1605,7 +1605,10 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
pgstat_report_wait_start(WAIT_EVENT_SNAPBUILD_WRITE);
if ((write(fd, ondisk, needed_length)) != needed_length)
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write to file \"%s\": %m", tmppath)));
@@ -1623,7 +1626,10 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
pgstat_report_wait_start(WAIT_EVENT_SNAPBUILD_SYNC);
if (pg_fsync(fd) != 0)
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not fsync file \"%s\": %m", tmppath)));
@@ -1708,7 +1714,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
pgstat_report_wait_end();
if (readBytes != SnapBuildOnDiskConstantSize)
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read file \"%s\", read %d of %d: %m",
@@ -1736,7 +1745,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
pgstat_report_wait_end();
if (readBytes != sizeof(SnapBuild))
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read file \"%s\", read %d of %d: %m",
@@ -1753,7 +1765,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
pgstat_report_wait_end();
if (readBytes != sz)
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read file \"%s\", read %d of %d: %m",
@@ -1769,7 +1784,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
pgstat_report_wait_end();
if (readBytes != sz)
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read file \"%s\", read %d of %d: %m",
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index e8b76b2f20..5b7116838f 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1378,7 +1378,10 @@ RestoreSlotFromDisk(const char *name)
pgstat_report_wait_start(WAIT_EVENT_REPLICATION_SLOT_RESTORE_SYNC);
if (pg_fsync(fd) != 0)
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(PANIC,
(errcode_for_file_access(),
errmsg("could not fsync file \"%s\": %m",
signature.asc
Description: PGP signature
