Continuing the discussion from [0], there are still a number of fsync()
calls in client programs that are unchecked or where errors are treated
non-fatally.
Digging around through the call stack, I think changing fsync_fname() to
just call exit(1) on errors instead of proceeding would address most of
that.
This affects (at least) initdb, pg_basebackup, pg_checksums, pg_dump,
pg_dumpall, and pg_rewind.
Thoughts?
[0]:
https://www.postgresql.org/message-id/flat/9b49fe44-8f3e-eca9-5914-29e9e99030bf%402ndquadrant.com
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From dfe2140d34d3c50b2908556c4627d77d009e487f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 11 Feb 2020 09:07:59 +0100
Subject: [PATCH] Change client-side fsync_fname() to report errors fatally
Given all we have learned about fsync() error handling in the last few
years, reporting an fsync() error non-fatally is not useful,
unless you don't care much about the file, in which case you probably
don't need to use fsync() in the first place.
Change fsync_fname() to exit(1) on fsync() errors other than those
that we specifically chose to ignore.
This affects initdb, pg_basebackup, pg_checksums, pg_dump, pg_dumpall,
and pg_rewind.
---
src/common/file_utils.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 413fe4eeb1..078980a954 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -51,8 +51,6 @@ static void walkdir(const char *path,
* fsyncing, and might not have privileges to write at all.
*
* serverVersion indicates the version of the server to be fsync'd.
- *
- * Errors are reported but not considered fatal.
*/
void
fsync_pgdata(const char *pg_data,
@@ -250,8 +248,8 @@ pre_sync_fname(const char *fname, bool isdir)
* fsync_fname -- Try to fsync a file or directory
*
* Ignores errors trying to open unreadable files, or trying to fsync
- * directories on systems where that isn't allowed/required. Reports
- * other errors non-fatally.
+ * directories on systems where that isn't allowed/required. All other errors
+ * are fatal.
*/
int
fsync_fname(const char *fname, bool isdir)
@@ -294,9 +292,9 @@ fsync_fname(const char *fname, bool isdir)
*/
if (returncode != 0 && !(isdir && (errno == EBADF || errno == EINVAL)))
{
- pg_log_error("could not fsync file \"%s\": %m", fname);
+ pg_log_fatal("could not fsync file \"%s\": %m", fname);
(void) close(fd);
- return -1;
+ exit(EXIT_FAILURE);
}
(void) close(fd);
--
2.25.0
