Unfortunately, Malcolm's patch still doesn't fix STOP. Using strace,
it looks like the problem occurs when you open("<repository>/db/
revprops", O_RDONLY). The fsync immediately after that operation fails.
Thanks for your help so far; I'll continue using my version of the
patch until I hear from you again.
I'm not a subscriber here, so please cc replies to mike (at) motomike
(dot) net.
-Mike Ashmore
On Feb 8, 2006, at 12:47 PM, Malcolm Rowe wrote:
> On Mon, Feb 06, 2006 at 04:49:53PM -0500, Mike Ashmore wrote:
>> According to my man pages, POSIX doesn't define a behavior for fsync
>> () on a file descriptor that was opened O_RDONLY. Unfortunately,
>> STOP's authors chose to make fsync fail on a read-only file
>> descriptor, while apparently every other implementor in the galaxy
>> chose to succeed (and presumably silently no-op).
>>
>> Anyway, to get everything working, I had to tweak libsvn_subr/io.c
>> just a bit. The patch is attached. And if there's a better way to
>> find out if a file descriptor is read-only, please let me know.
>>
>
> That patch also falls foul of POSIX, I think: write(fd, ..., 0) is
> supposed to return immediately if "the file is a regular file" - no
> mention of whether it's open for writing or not. (Though yes, Linux
> does check first).
>
> The real question is why we're fsync()'ing a file that's not being
> written to. As far as I can see, we only do it in one place, in
> libsvn_fs_fs/fs_fs.c:svn_fs_fs__move_into_place(). We rename a file,
> then open the new name (read-only) and fsync() it, then (on Linux
> only)
> open the directory and fsync() that, the latter to sync the directory
> contents, necessary for at least ext2, and maybe ext3.
>
> Unfortunately, we can't open the directory read-write on Linux, since
> the open will then fail. There's no reason we can't open the target
> file read-write though, which is what the attached patch does.
>
> Could you try the attached and see if it fixes the problem - STOP may
> not care about fsync()'s of directories, since that's a common usage
> pattern on Linux.
>
> (Even if it doesn't, does anyone else see any objection to my
> committing
> it? It seems strange that we'd try to fsync() an unwritable file -
> even if it does work).
>
> Regards,
> Malcolm
> Index: subversion/libsvn_fs_fs/fs_fs.c
> ===================================================================
> --- subversion/libsvn_fs_fs/fs_fs.c (revision 18392)
> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> @@ -3580,25 +3580,25 @@ svn_fs_fs__move_into_place (const char *
>
> /* Move the file into place. */
> err = svn_io_file_rename (old_filename, new_filename, pool);
> if (err && APR_STATUS_IS_EXDEV (err->apr_err))
> {
> apr_file_t *file;
>
> /* Can't rename across devices; fall back to copying. */
> svn_error_clear (err);
> SVN_ERR (svn_io_copy_file (old_filename, new_filename, TRUE,
> pool));
>
> /* Flush the target of the copy to disk. */
> - SVN_ERR (svn_io_file_open (&file, new_filename, APR_READ,
> + SVN_ERR (svn_io_file_open (&file, new_filename, APR_WRITE,
> APR_OS_DEFAULT, pool));
> SVN_ERR (svn_io_file_flush_to_disk (file, pool));
> SVN_ERR (svn_io_file_close (file, pool));
> }
>
> #ifdef __linux__
> {
> /* Linux has the unusual feature that fsync() on a file is not
> enough to ensure that a file's directory entries have been
> flushed to disk; you have to fsync the directory as well.
> On other operating systems, we'd only be asking for trouble
> by trying to open and fsync a directory. */
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Feb 8 22:36:08 2006