diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 0296634d2e2..f670ecc0439 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -7043,6 +7043,38 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' + + data_sync_retry (boolean) + + data_sync_retry configuration parameter + + + + + When set to false, which is the default, PostgreSQL + will raise a PANIC-level error on failure to flush modified data files + to the filesystem. This causes the database server to crash. + + + On some operating systems, the status of data in the kernel's page + cache is unknown after a write-back failure. In some cases it might + have been entirely forgotten, making it unsafe to retry; the second + attempt may be reported as successful, when in fact the data has been + lost. In these circumstances, the only way to avoid data loss is to + recover from the WAL after any failure is reported, preferably + after investigating the root cause of the failure and replacing any + faulty hardware. + + + If set to true, PostgreSQL will instead + report an error but continue to run so that the data flushing + operation can be retried in a later checkpoint. Only set it to true + after investigating the operating system's treatment of buffered data + in case of write-back failure. + + + + diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index ca18d986744..0577c540447 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -976,7 +976,7 @@ logical_end_heap_rewrite(RewriteState state) while ((src = (RewriteMappingFile *) hash_seq_search(&seq_status)) != NULL) { if (FileSync(src->vfd) != 0) - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", src->path))); FileClose(src->vfd); @@ -1194,7 +1194,7 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * doesn't seem worth the trouble. */ if (pg_fsync(fd) != 0) - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", path))); @@ -1291,7 +1291,7 @@ CheckPointLogicalRewriteHeap(void) * but it's currently not deemed worth the effort. */ else if (pg_fsync(fd) != 0) - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", path))); CloseTransientFile(fd); diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index e098cebabf0..217182f8cd6 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -904,7 +904,7 @@ SlruReportIOError(SlruCtl ctl, int pageno, TransactionId xid) path, offset))); break; case SLRU_FSYNC_FAILED: - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not access status of transaction %u", xid), errdetail("Could not fsync file \"%s\": %m.", diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c index 25ac70e8f4f..6f48f199d1f 100644 --- a/src/backend/access/transam/timeline.c +++ b/src/backend/access/transam/timeline.c @@ -402,7 +402,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, } if (pg_fsync(fd) != 0) - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", tmppath))); @@ -478,7 +478,7 @@ writeTimeLineHistoryFile(TimeLineID tli, char *content, int size) } if (pg_fsync(fd) != 0) - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", tmppath))); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 92a3b7954a9..a8a93bca345 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -3230,7 +3230,7 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno, } if (pg_fsync(fd) != 0) - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", tmppath))); diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7a06fe086f6..29f3d95ab68 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -1589,6 +1589,9 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn) * fsync the file before renaming so that even if we crash after this we * have either a fully valid file or nothing. * + * It's safe to just ERROR on fsync() here because we'll retry the whole + * operation including the writes. + * * TODO: Do the fsync() via checkpoints/restartpoints, doing it here has * some noticeable overhead since it's performed synchronously during * decoding? diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 6f35105e867..c255583026f 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -131,6 +131,8 @@ int max_files_per_process = 1000; */ int max_safe_fds = 32; /* default if not changed */ +/* Whether it is safe to continue running after fsync() fails. */ +bool data_sync_retry = false; /* Debugging.... */ @@ -424,7 +426,7 @@ pg_flush_data(int fd, off_t offset, off_t amount) void fsync_fname(const char *fname, bool isdir) { - fsync_fname_ext(fname, isdir, false, ERROR); + fsync_fname_ext(fname, isdir, false, data_sync_elevel(ERROR)); } /* @@ -853,7 +855,8 @@ LruDelete(File file) * to leak the FD than to mess up our internal state. */ if (close(vfdP->fd)) - elog(LOG, "could not close file \"%s\": %m", vfdP->fileName); + elog(vfdP->fdstate & FD_TEMPORARY ? LOG : data_sync_elevel(LOG), + "could not close file \"%s\": %m", vfdP->fileName); vfdP->fd = VFD_CLOSED; --nfile; @@ -1332,7 +1335,14 @@ FileClose(File file) { /* close the file */ if (close(vfdP->fd)) - elog(LOG, "could not close file \"%s\": %m", vfdP->fileName); + { + /* + * We may need to panic on failure to close non-temporary files; + * see LruDelete. + */ + elog(vfdP->fdstate & FD_TEMPORARY ? LOG : data_sync_elevel(LOG), + "could not close file \"%s\": %m", vfdP->fileName); + } --nfile; vfdP->fd = VFD_CLOSED; @@ -2698,6 +2708,9 @@ looks_like_temp_rel_name(const char *name) * harmless cases such as read-only files in the data directory, and that's * not good either. * + * Note that if we previously crashed due to a PANIC on fsync(), we'll be + * rewriting all changes again during recovery. + * * Note we assume we're chdir'd into PGDATA to begin with. */ void @@ -2979,3 +2992,26 @@ fsync_parent_path(const char *fname, int elevel) return 0; } + +/* + * Return the passed-in error level, or PANIC if data_sync_retry is off. + * + * Failure to fsync any data file is cause for immediate panic, unless + * data_sync_retry is enabled. Data may have been written to the operating + * system and removed from our buffer pool already, and if we are running on + * an operating system that forgets dirty data on write-back failure, there + * may be only one copy of the data remaining: in the WAL. A later attempt to + * fsync again might falsely report success. Therefore we must not allow any + * further checkpoints to be attempted. data_sync_retry can in theory be + * enabled on systems known not to drop dirty buffered data on write-back + * failure (with the likely outcome that checkpoints will continue to fail + * until the underlying problem is fixed). + * + * Any code that reports a failure from fsync() or related functions should + * filter the error level with this function. + */ +int +data_sync_elevel(int elevel) +{ + return data_sync_retry ? elevel : PANIC; +} diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c index fcbe9eac993..f28a3e6d066 100644 --- a/src/backend/storage/smgr/md.c +++ b/src/backend/storage/smgr/md.c @@ -979,7 +979,7 @@ mdimmedsync(SMgrRelation reln, ForkNumber forknum) while (v != NULL) { if (FileSync(v->mdfd_vfd) < 0) - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", FilePathName(v->mdfd_vfd)))); @@ -1222,7 +1222,7 @@ mdsync(void) bms_join(new_requests, requests); errno = save_errno; - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", path))); @@ -1396,7 +1396,7 @@ register_dirty_segment(SMgrRelation reln, ForkNumber forknum, MdfdVec *seg) (errmsg("could not forward fsync request because request queue is full"))); if (FileSync(seg->mdfd_vfd) < 0) - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", FilePathName(seg->mdfd_vfd)))); diff --git a/src/backend/utils/cache/relmapper.c b/src/backend/utils/cache/relmapper.c index c151b92dfc3..6d4e179377d 100644 --- a/src/backend/utils/cache/relmapper.c +++ b/src/backend/utils/cache/relmapper.c @@ -792,7 +792,7 @@ write_relmap_file(bool shared, RelMapFile *newmap, * CheckPointRelationMap. */ if (pg_fsync(fd) != 0) - ereport(ERROR, + ereport(data_sync_elevel(ERROR), (errcode_for_file_access(), errmsg("could not fsync relation mapping file \"%s\": %m", mapfilename))); diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 03e2c4033b6..427d807ada1 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -1620,6 +1620,15 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, + { + {"data_sync_retry", PGC_POSTMASTER, ERROR_HANDLING_OPTIONS, + gettext_noop("Whether to continue running after a failure to sync data files."), + }, + &data_sync_retry, + false, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 0dc9566e077..4b62f2241bc 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -602,6 +602,7 @@ #exit_on_error = off # terminate session on any error? #restart_after_crash = on # reinitialize after backend crash? +#data_sync_retry = off # retry or panic on failure to fsync data? #------------------------------------------------------------------------------ diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index c68d9064e78..6c921576564 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -53,6 +53,7 @@ typedef int File; /* GUC parameter */ extern PGDLLIMPORT int max_files_per_process; +extern PGDLLIMPORT bool data_sync_retry; /* * This is private to fd.c, but exported for save/restore_backend_variables() @@ -119,6 +120,7 @@ extern void fsync_fname(const char *fname, bool isdir); extern int durable_rename(const char *oldfile, const char *newfile, int loglevel); extern int durable_link_or_rename(const char *oldfile, const char *newfile, int loglevel); extern void SyncDataDirectory(void); +extern int data_sync_elevel(int elevel); /* Filename components for OpenTemporaryFile */ #define PG_TEMP_FILES_DIR "pgsql_tmp"