From 47ac2033d460cefbbda2e39732e18de521dc6a36 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 17 Apr 2019 09:51:45 +0900 Subject: [PATCH] Simplify some ERROR paths clearing wait events and transient files Transient files and wait events get normally cleaned up when seeing an exception (be it in the context of a transaction for a backend or another process like the checkpointer), hence there is little point in complicating error code paths to do this work. This shaves a bit of code, and removes some extra handling with errno which needed to be preserved during the cleanup steps done. Reported-by: Masahiko Sawada Author: Michael Paquier Reviewed-by: Tom Lane, Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoDhHYVq5KkXfkaHhmjA-zJYj-e4teiRAJefvXuKJz1tKQ@mail.gmail.com --- src/backend/access/transam/twophase.c | 22 ++++------------------ src/backend/replication/logical/origin.c | 21 ++++++--------------- src/backend/replication/slot.c | 14 -------------- src/backend/storage/file/copydir.c | 1 - 4 files changed, 10 insertions(+), 48 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index f9a4960f8ae..a399c0052d9 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1693,26 +1693,18 @@ 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); - /* if write didn't set errno, assume problem is no disk space */ - errno = save_errno ? save_errno : ENOSPC; + if (errno == 0) + errno = ENOSPC; ereport(ERROR, (errcode_for_file_access(), errmsg("could not write file \"%s\": %m", path))); } if (write(fd, &statefile_crc, sizeof(pg_crc32c)) != sizeof(pg_crc32c)) { - int save_errno = errno; - - pgstat_report_wait_end(); - CloseTransientFile(fd); - /* if write didn't set errno, assume problem is no disk space */ - errno = save_errno ? save_errno : ENOSPC; + if (errno == 0) + errno = ENOSPC; ereport(ERROR, (errcode_for_file_access(), errmsg("could not write file \"%s\": %m", path))); @@ -1725,15 +1717,9 @@ 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 file \"%s\": %m", path))); - } pgstat_report_wait_end(); if (CloseTransientFile(fd) != 0) diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c index 42fd8f5b6b7..ff4d54d6edd 100644 --- a/src/backend/replication/logical/origin.c +++ b/src/backend/replication/logical/origin.c @@ -579,12 +579,9 @@ CheckPointReplicationOrigin(void) errno = 0; if ((write(tmpfd, &magic, sizeof(magic))) != sizeof(magic)) { - int save_errno = errno; - - CloseTransientFile(tmpfd); - /* if write didn't set errno, assume problem is no disk space */ - errno = save_errno ? save_errno : ENOSPC; + if (errno == 0) + errno = ENOSPC; ereport(PANIC, (errcode_for_file_access(), errmsg("could not write to file \"%s\": %m", @@ -624,12 +621,9 @@ CheckPointReplicationOrigin(void) if ((write(tmpfd, &disk_state, sizeof(disk_state))) != sizeof(disk_state)) { - int save_errno = errno; - - CloseTransientFile(tmpfd); - /* if write didn't set errno, assume problem is no disk space */ - errno = save_errno ? save_errno : ENOSPC; + if (errno == 0) + errno = ENOSPC; ereport(PANIC, (errcode_for_file_access(), errmsg("could not write to file \"%s\": %m", @@ -646,12 +640,9 @@ CheckPointReplicationOrigin(void) errno = 0; if ((write(tmpfd, &crc, sizeof(crc))) != sizeof(crc)) { - int save_errno = errno; - - CloseTransientFile(tmpfd); - /* if write didn't set errno, assume problem is no disk space */ - errno = save_errno ? save_errno : ENOSPC; + if (errno == 0) + errno = ENOSPC; ereport(PANIC, (errcode_for_file_access(), errmsg("could not write to file \"%s\": %m", diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index 006446b663e..057c5d7ab2e 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -1398,16 +1398,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", path))); - } pgstat_report_wait_end(); /* Also sync the parent directory */ @@ -1421,10 +1415,6 @@ RestoreSlotFromDisk(const char *name) pgstat_report_wait_end(); if (readBytes != ReplicationSlotOnDiskConstantSize) { - int saved_errno = errno; - - CloseTransientFile(fd); - errno = saved_errno; if (readBytes < 0) ereport(PANIC, (errcode_for_file_access(), @@ -1466,10 +1456,6 @@ RestoreSlotFromDisk(const char *name) pgstat_report_wait_end(); if (readBytes != cp.length) { - int saved_errno = errno; - - CloseTransientFile(fd); - errno = saved_errno; if (readBytes < 0) ereport(PANIC, (errcode_for_file_access(), diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c index 342d078a8ff..30f6200a86f 100644 --- a/src/backend/storage/file/copydir.c +++ b/src/backend/storage/file/copydir.c @@ -199,7 +199,6 @@ copy_file(char *fromfile, char *tofile) pgstat_report_wait_start(WAIT_EVENT_COPY_FILE_WRITE); if ((int) write(dstfd, buffer, nbytes) != nbytes) { - pgstat_report_wait_end(); /* if write didn't set errno, assume problem is no disk space */ if (errno == 0) errno = ENOSPC;