At 2014-09-29 11:54:10 +0200, and...@2ndquadrant.com wrote:
>
> On 2014-09-29 14:09:01 +0530, Abhijit Menon-Sen wrote:
> >
> > I just noticed that initdb -S ("Safely write all database files to disk
> > and exit") does (only) the following in perform_fsync:
> >
> > pre_sync_fname(pdir, true);
> > walkdir(pg_data, pre_sync_fname);
> >
> > fsync_fname(pdir, true);
> > walkdir(pg_data, fsync_fname);
> >
> > walkdir() reads the directory and calls itself recursively for S_ISDIR
> > entries, or calls the function for S_ISREG entries… which means it
> > doesn't follow links.
> >
> > Which means it doesn't fsync the contents of tablespaces.
>
> Which means at least pg_upgrade is unsafe right
> now... c.f. 630cd14426dc1daf85163ad417f3a224eb4ac7b0.
Here's a proposed patch to initdb to make initdb -S fsync everything
under pg_tblspc. It introduces a new function that calls walkdir on
every entry under pg_tblspc. This is only one approach: I could have
also changed walkdir to follow links, but that would have required a
bunch of #ifdefs for Windows (because it doesn't have symlinks), and I
guessed a separate function with two calls might be easier to patch into
back branches. I've tested this patch under various conditions on Linux,
but it could use some testing on Windows.
-- Abhijit
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index dc1f1df..4fdc9eb 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -218,6 +218,7 @@ static char **filter_lines_with_token(char **lines, const char *token);
static char **readfile(const char *path);
static void writefile(char *path, char **lines);
static void walkdir(char *path, void (*action) (char *fname, bool isdir));
+static void walktblspc_links(char *path, void (*action) (char *fname, bool isdir));
static void pre_sync_fname(char *fname, bool isdir);
static void fsync_fname(char *fname, bool isdir);
static FILE *popen_check(const char *command, const char *mode);
@@ -588,6 +589,53 @@ walkdir(char *path, void (*action) (char *fname, bool isdir))
}
/*
+ * walktblspc_links: call walkdir on each entry under the given
+ * pg_tblspc directory, or do nothing if pg_tblspc doesn't exist.
+ */
+static void
+walktblspc_links(char *path, void (*action) (char *fname, bool isdir))
+{
+ DIR *dir;
+ struct dirent *direntry;
+ char subpath[MAXPGPATH];
+
+ dir = opendir(path);
+ if (dir == NULL)
+ {
+ if (errno == ENOENT)
+ return;
+ fprintf(stderr, _("%s: could not open directory \"%s\": %s\n"),
+ progname, path, strerror(errno));
+ exit_nicely();
+ }
+
+ while (errno = 0, (direntry = readdir(dir)) != NULL)
+ {
+ if (strcmp(direntry->d_name, ".") == 0 ||
+ strcmp(direntry->d_name, "..") == 0)
+ continue;
+
+ snprintf(subpath, MAXPGPATH, "%s/%s", path, direntry->d_name);
+
+ walkdir(subpath, action);
+ }
+
+ if (errno)
+ {
+ fprintf(stderr, _("%s: could not read directory \"%s\": %s\n"),
+ progname, path, strerror(errno));
+ exit_nicely();
+ }
+
+ if (closedir(dir))
+ {
+ fprintf(stderr, _("%s: could not close directory \"%s\": %s\n"),
+ progname, path, strerror(errno));
+ exit_nicely();
+ }
+}
+
+/*
* Hint to the OS that it should get ready to fsync() this file.
*/
static void
@@ -2377,6 +2425,7 @@ static void
perform_fsync(void)
{
char pdir[MAXPGPATH];
+ char pg_tblspc[MAXPGPATH];
fputs(_("syncing data to disk ... "), stdout);
fflush(stdout);
@@ -2398,6 +2447,11 @@ perform_fsync(void)
/* then recursively through the directory */
walkdir(pg_data, pre_sync_fname);
+ /* now do the same thing for everything under pg_tblspc */
+
+ snprintf(pg_tblspc, MAXPGPATH, "%s/pg_tblspc", pg_data);
+ walktblspc_links(pg_tblspc, pre_sync_fname);
+
/*
* Now, do the fsync()s in the same order.
*/
@@ -2408,6 +2462,8 @@ perform_fsync(void)
/* then recursively through the directory */
walkdir(pg_data, fsync_fname);
+ walktblspc_links(pg_tblspc, fsync_fname);
+
check_ok();
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
