On Tue, Feb 23, 2010 at 3:38 PM, Tom Lane <[email protected]> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers