Hi Tom, Unfortunately, due to some personal life business, it took until for me to feel comfortable pushing the fix for https://www.postgresql.org/message-id/zezdj1h61ryrm...@msg.df7cb.de (FileFallocate() erroring out with EINTR due to running on tmpfs).
Do you want me to hold off before beta2 is wrapped? I did a bunch of CI runs with the patch and patch + test infra, and they all passed. Greetings, Andres Freund
>From 6e67692231de22b6e347d330aacad72a6fdbd89a Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Mon, 19 Jun 2023 10:26:30 -0700 Subject: [PATCH v2] fd.c: Retry after EINTR in more places Starting with 4d330a61bb1 we can use posix_fallocate() to extend files. Unfortunately in some situation, e.g. on tmpfs filesystems, EINTR may be returned. See also 4518c798b2b. To fix, add a retry path to FileFallocate(). In contrast to 4518c798b2b the amount we extend by is limited and the extending may happen at a high frequency, so disabling signals does not appear to be the correct path here. Also add retry paths to other file operations currently lacking them (around fdatasync(), fsync(), ftruncate(), posix_fadvise(), sync_file_range(), truncate()) - they are all documented or have been observed to return EINTR. Even though most of these functions used in the back branches, it does not seem worth the risk to backpatch - outside of the new-to-16 case of posix_fallocate() I am not aware of problem reports due to the lack of retries. Reported-by: Christoph Berg <m...@debian.org> Discussion: https://postgr.es/m/zezdj1h61ryrm...@msg.df7cb.de Backpatch: - --- src/backend/storage/file/fd.c | 73 +++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 12 deletions(-) diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 173476789c7..db39186f058 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -415,10 +415,18 @@ pg_fsync(int fd) int pg_fsync_no_writethrough(int fd) { - if (enableFsync) - return fsync(fd); - else + int rc; + + if (!enableFsync) return 0; + +retry: + rc = fsync(fd); + + if (rc == -1 && errno == EINTR) + goto retry; + + return rc; } /* @@ -448,10 +456,18 @@ pg_fsync_writethrough(int fd) int pg_fdatasync(int fd) { - if (enableFsync) - return fdatasync(fd); - else + int rc; + + if (!enableFsync) return 0; + +retry: + rc = fdatasync(fd); + + if (rc == -1 && errno == EINTR) + goto retry; + + return rc; } /* @@ -483,6 +499,7 @@ pg_flush_data(int fd, off_t offset, off_t nbytes) if (not_implemented_by_kernel) return; +retry: /* * sync_file_range(SYNC_FILE_RANGE_WRITE), currently linux specific, * tells the OS that writeback for the specified blocks should be @@ -498,6 +515,9 @@ pg_flush_data(int fd, off_t offset, off_t nbytes) { int elevel; + if (rc == EINTR) + goto retry; + /* * For systems that don't have an implementation of * sync_file_range() such as Windows WSL, generate only one @@ -629,32 +649,54 @@ pg_flush_data(int fd, off_t offset, off_t nbytes) #endif } +/* + * Truncate an open file to a given length. + */ +static int +pg_ftruncate(int fd, off_t length) +{ + int ret; + +retry: + ret = ftruncate(fd, length); + + if (ret == -1 && errno == EINTR) + goto retry; + + return ret; +} + /* * Truncate a file to a given length by name. */ int pg_truncate(const char *path, off_t length) { + int ret; #ifdef WIN32 int save_errno; - int ret; int fd; fd = OpenTransientFile(path, O_RDWR | PG_BINARY); if (fd >= 0) { - ret = ftruncate(fd, length); + ret = pg_ftruncate(fd, length); save_errno = errno; CloseTransientFile(fd); errno = save_errno; } else ret = -1; +#else + +retry: + ret = truncate(path, length); + + if (ret == -1 && errno == EINTR) + goto retry; +#endif return ret; -#else - return truncate(path, length); -#endif } /* @@ -2001,11 +2043,15 @@ FilePrefetch(File file, off_t offset, off_t amount, uint32 wait_event_info) if (returnCode < 0) return returnCode; +retry: pgstat_report_wait_start(wait_event_info); returnCode = posix_fadvise(VfdCache[file].fd, offset, amount, POSIX_FADV_WILLNEED); pgstat_report_wait_end(); + if (returnCode == EINTR) + goto retry; + return returnCode; #else Assert(FileIsValid(file)); @@ -2281,12 +2327,15 @@ FileFallocate(File file, off_t offset, off_t amount, uint32 wait_event_info) if (returnCode < 0) return -1; +retry: pgstat_report_wait_start(wait_event_info); returnCode = posix_fallocate(VfdCache[file].fd, offset, amount); pgstat_report_wait_end(); if (returnCode == 0) return 0; + else if (returnCode == EINTR) + goto retry; /* for compatibility with %m printing etc */ errno = returnCode; @@ -2334,7 +2383,7 @@ FileTruncate(File file, off_t offset, uint32 wait_event_info) return returnCode; pgstat_report_wait_start(wait_event_info); - returnCode = ftruncate(VfdCache[file].fd, offset); + returnCode = pg_ftruncate(VfdCache[file].fd, offset); pgstat_report_wait_end(); if (returnCode == 0 && VfdCache[file].fileSize > offset) -- 2.38.0
