Content-Length: 395490 | pFad | http://github.com/postgrespro/postgres/commit/cb867853a17e4055d58a8639ed4a23652e2ce8e0

6D Fix fsync-at-startup code to not treat errors as fatal. · postgrespro/postgres@cb86785 · GitHub
Skip to content

Commit cb86785

Browse files
tglsfdctglsfdc
committed
Fix fsync-at-startup code to not treat errors as fatal.
Commit 2ce439f introduced a rather serious regression, namely that if its scan of the data directory came across any un-fsync-able files, it would fail and thereby prevent database startup. Worse yet, symlinks to such files also caused the problem, which meant that crash restart was guaranteed to fail on certain common installations such as older Debian. After discussion, we agreed that (1) failure to start is worse than any consequence of not fsync'ing is likely to be, therefore treat all errors in this code as nonfatal; (2) we should not chase symlinks other than those that are expected to exist, namely pg_xlog/ and tablespace links under pg_tblspc/. The latter restriction avoids possibly fsync'ing a much larger part of the filesystem than intended, if the user has left random symlinks hanging about in the data directory. This commit takes care of that and also does some code beautification, mainly moving the relevant code into fd.c, which seems a much better place for it than xlog.c, and making sure that the conditional compilation for the pre_sync_fname pass has something to do with whether pg_flush_data works. I also relocated the call site in xlog.c down a few lines; it seems a bit silly to be doing this before ValidateXLOGDirectoryStructure(). The similar logic in initdb.c ought to be made to match this, but that change is noncritical and will be dealt with separately. Back-patch to all active branches, like the prior commit. Abhijit Menon-Sen and Tom Lane
1 parent ee8b93c commit cb86785

File tree

3 files changed

+263
-117
lines changed

3 files changed

+263
-117
lines changed

‎src/backend/access/transam/xlog.c

Lines changed: 12 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -678,8 +678,6 @@ static bool read_backup_label(XLogRecPtr *checkPointLoc,
678678
static void rm_redo_error_callback(void *arg);
679679
static int get_sync_bit(int method);
680680

681-
static void fsync_pgdata(char *datadir);
682-
683681
/*
684682
* Insert an XLOG record having the specified RMID and info bytes,
685683
* with the body of the record being the data chunk(s) described by
@@ -6230,18 +6228,6 @@ StartupXLOG(void)
62306228
(errmsg("database system was interrupted; last known up at %s",
62316229
str_time(ControlFile->time))));
62326230

6233-
/*
6234-
* If we previously crashed, there might be data which we had written,
6235-
* intending to fsync it, but which we had not actually fsync'd yet.
6236-
* Therefore, a power failure in the near future might cause earlier
6237-
* unflushed writes to be lost, even though more recent data written to
6238-
* disk from here on would be persisted. To avoid that, fsync the entire
6239-
* data directory.
6240-
*/
6241-
if (ControlFile->state != DB_SHUTDOWNED &&
6242-
ControlFile->state != DB_SHUTDOWNED_IN_RECOVERY)
6243-
fsync_pgdata(data_directory);
6244-
62456231
/* This is just to allow attaching to startup process with a debugger */
62466232
#ifdef XLOG_REPLAY_DELAY
62476233
if (ControlFile->state != DB_SHUTDOWNED)
@@ -6265,6 +6251,18 @@ StartupXLOG(void)
62656251
*/
62666252
RelationCacheInitFileRemove();
62676253

6254+
/*
6255+
* If we previously crashed, there might be data which we had written,
6256+
* intending to fsync it, but which we had not actually fsync'd yet.
6257+
* Therefore, a power failure in the near future might cause earlier
6258+
* unflushed writes to be lost, even though more recent data written to
6259+
* disk from here on would be persisted. To avoid that, fsync the entire
6260+
* data directory.
6261+
*/
6262+
if (ControlFile->state != DB_SHUTDOWNED &&
6263+
ControlFile->state != DB_SHUTDOWNED_IN_RECOVERY)
6264+
SyncDataDirectory();
6265+
62686266
/*
62696267
* Initialize on the assumption we want to recover to the same timeline
62706268
* that's active according to pg_control.
@@ -10838,31 +10836,3 @@ WakeupRecovery(void)
1083810836
{
1083910837
SetLatch(&XLogCtl->recoveryWakeupLatch);
1084010838
}
10841-
10842-
/*
10843-
* Issue fsync recursively on PGDATA and all its contents.
10844-
*/
10845-
static void
10846-
fsync_pgdata(char *datadir)
10847-
{
10848-
if (!enableFsync)
10849-
return;
10850-
10851-
/*
10852-
* If possible, hint to the kernel that we're soon going to fsync
10853-
* the data directory and its contents.
10854-
*/
10855-
#if defined(HAVE_SYNC_FILE_RANGE) || \
10856-
(defined(USE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED))
10857-
walkdir(datadir, pre_sync_fname);
10858-
#endif
10859-
10860-
/*
10861-
* Now we do the fsync()s in the same order.
10862-
*
10863-
* It's important to fsync the destination directory itself as individual
10864-
* file fsyncs don't guarantee that the directory entry for the file is
10865-
* synced.
10866-
*/
10867-
walkdir(datadir, fsync_fname);
10868-
}

0 commit comments

Comments
 (0)








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/postgrespro/postgres/commit/cb867853a17e4055d58a8639ed4a23652e2ce8e0

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy