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

Reply via email to