On Tue, Feb 23, 2010 at 3:38 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> The plan I was thinking of was to pass a flag indicating if it's a >> directory to fsync_fname() and open it RD_ONLY if it's a directory and >> RDRW if it's a file. Then ignore any error returns (or at least EBADF >> and EINVAL) iff the flag indicating it was a directory was true. > > Works for me, but let's first try just ignoring EBADF, which is the only > value we saw in the recent buildfarm failures. If we got past the fd<0 > test then EBADF could only indicate a rdonly failure, whereas it's less > clear what EINVAL might cover.
So I'm thinking of something like this. Ignore ESDIR when opening a directory (and return immediately) and ignore EBADF when trying to fsync a directory. -- greg
*** a/src/port/copydir.c --- b/src/port/copydir.c *************** *** 37,43 **** static void copy_file(char *fromfile, char *tofile); ! static void fsync_fname(char *fname); /* --- 37,43 ---- static void copy_file(char *fromfile, char *tofile); ! static void fsync_fname(char *fname, bool isdir); /* *************** *** 121,132 **** copydir(char *fromdir, char *todir, bool recurse) errmsg("could not stat file \"%s\": %m", tofile))); if (S_ISREG(fst.st_mode)) ! fsync_fname(tofile); } FreeDir(xldir); - #ifdef NOTYET - /* * It's important to fsync the destination directory itself as individual * file fsyncs don't guarantee that the directory entry for the file is --- 121,130 ---- errmsg("could not stat file \"%s\": %m", tofile))); if (S_ISREG(fst.st_mode)) ! fsync_fname(tofile, false); } FreeDir(xldir); /* * It's important to fsync the destination directory itself as individual * file fsyncs don't guarantee that the directory entry for the file is *************** *** 135,142 **** copydir(char *fromdir, char *todir, bool recurse) * * However we can't do this just yet, it has portability issues. */ ! fsync_fname(todir); ! #endif } /* --- 133,139 ---- * * However we can't do this just yet, it has portability issues. */ ! fsync_fname(todir, true); } /* *************** *** 216,235 **** copy_file(char *fromfile, char *tofile) /* * fsync a file */ static void ! fsync_fname(char *fname) { ! int fd = BasicOpenFile(fname, ! O_RDWR | PG_BINARY, ! S_IRUSR | S_IWUSR); ! ! if (fd < 0) ereport(ERROR, (errcode_for_file_access(), errmsg("could not open file \"%s\": %m", fname))); ! if (pg_fsync(fd) != 0) ereport(ERROR, (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", fname))); --- 213,256 ---- /* * fsync a file + * + * Try to fsync directories but ignore errors that indicate the OS + * just doesn't allow/require fsyncing directories. */ static void ! fsync_fname(char *fname, bool isdir) { ! int fd; ! int returncode;; ! ! if (!isdir) ! fd = BasicOpenFile(fname, ! O_RDWR | PG_BINARY, ! S_IRUSR | S_IWUSR); ! else ! fd = BasicOpenFile(fname, ! O_RDONLY | PG_BINARY, ! S_IRUSR | S_IWUSR); ! ! /* Some OSs don't allow us to open directories at all */ ! if (fd < 0 && isdir && errno == EISDIR) ! return; ! ! else if (fd < 0) ereport(ERROR, (errcode_for_file_access(), errmsg("could not open file \"%s\": %m", fname))); ! returncode = pg_fsync(fd); ! ! /* Some OSs don't allow us to fsync directories */ ! if (returncode != 0 && isdir && errno == EBADF) ! { ! close(fd); ! return; ! } ! ! if (returncode != 0) ereport(ERROR, (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", fname)));
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers