Josh Pieper wrote:
>In order to improve the data handling properties of FSFS, there are
>times when the permanent rev file and 'current' file need to have
>their data flushed to disk. Currently, there is no method in APR or
>Subversion itself to accomplish this. I am proposing we add a new
>function to libsvn_subr, svn_io_file_flush_to_disk, which performs
>this function.
>
>Should this go into APR instead? If so, what is the proper process to
>get it there, post on the APR dev list and wait?
>
>
Yes, it should go into APR. But there's no need to wait -- quite a few
SVN committers are also APR committers, so it should go smoothly once
the patch for APR is ready. But we still need a fallback in svn_io
because we can't require the absolutely latest version of APR.
>In the meantime, attached is a potential patch which implements this
>function and uses it for FSFS.
>
>-Josh
>
>-----------------------------
>
>Add a new function in svn_io, svn_io_file_flush_to_disk, which flushes
>all data written to a file to disk. On Unix like systems, this
>equates to fsync, for Win32, FlushFileBuffers is used.
>
>* subversion/include/svn_io.h
>* subversion/libsvn_subr/io.h
> (svn_io_file_flush_to_disk): New, flushes all data written to a file
> to disk.
>
>* subversion/libsvn_fs_fs/fs_fs.c
> (write_final_current): Flush the 'current' file to disk before
> closing it.
> (svn_fs_fs__commit): Flush the new permanent rev-file to disk before
> closing it.
>
>Index: subversion/include/svn_io.h
>===================================================================
>--- subversion/include/svn_io.h (revision 9587)
>+++ subversion/include/svn_io.h (working copy)
>@@ -324,6 +324,12 @@
> svn_boolean_t exclusive,
> apr_pool_t *pool);
>
>+/** Flush any unwritten data from @a file to disk. Use @a pool for
>+ * memory allocations.
>+ */
>+svn_error_t *svn_io_file_flush_to_disk (apr_file_t *file,
>+ apr_pool_t *pool);
>+
> /** Copy file @a file from location @a src_path to location @a dest_path.
> * Use @a pool for memory allocations.
> */
>Index: subversion/libsvn_subr/io.c
>===================================================================
>--- subversion/libsvn_subr/io.c (revision 9587)
>+++ subversion/libsvn_subr/io.c (working copy)
>@@ -968,6 +968,50 @@
>
>
>
>+/* Data consistency/coherency operations. */
>+
>+svn_error_t *svn_io_file_flush_to_disk (apr_file_t *file,
>+ apr_pool_t *pool)
>+{
>+ /* First make sure that any user-space buffered data is flushed. */
>+ apr_status_t apr_err;
>+
>+ apr_err = apr_file_flush (file);
>+
>+ if (apr_err)
>+ return svn_error_wrap_apr
>+ (apr_err, "Can't flush file '%s' to disk.", file);
>+
>+ /* Call the operating system specific function to actually force the
>+ data to disk. */
>+ {
>+#ifdef WIN32
>+ HANDLE filehand;
>+
>+ apr_os_file_get (&filehand, file);
>
>
You can move these two lines outside the ifdef:
apr_os_file_t filehand;
apr_if_file_get (&filehand, file)
The less conditional code we have, the better I'll like it.
>+ FlushFileBuffers (filehand);
>
>
How about checking the return value here, too?
Of course, on Windows it would be much better if APR knew how to open
the file with FILE_FLAG_WRITETHROUGH...perhaps another flag to
apr_file_open that would cause apr_file_close to call fsync on Unix, but
open the file in writethrough mode on Windows?
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat May 1 15:58:07 2004