Ivan Zhakov <ivan_at_visualsvn.com> writes:
> We considered adding FLUSH_TO_DISK flag to svn_stream_from_apr_file(),
> but decided to commit simple fix for now:
> 1. The current FSFS code uses explicit flush and this commit makes
> code consistent with other parts
> 2. Adding flag to svn_stream_from_apr_file() will require revving API
> etc, while this fix should be backported to 1.8.x and 1.9.x
> 3. It's unclear how FLUSH_TO_DISK flag should interact with DISOWN=TRUE.
> Taking in account all above we decided to commit simple fix for now
> and possibly implement FLUSH_TO_DISK flag later applying it to all
> FSFS code where needed.
Is it simple? It's possible that r1680819 is more complicated than
adding flush to the the stream.
Exactly what the API should look like is bit unclear. Is there any need
to support disown=TRUE and flush=TRUE? When would Subversion use it?
If we create svn_stream_from_aprfile3 that takes two booleans then
perhaps we change the return type to allow an error for the impossible
disown=TRUE and flush=TRUE.
All the flushing code goes in libsvn_subr and the code change in FSFS is
really small, something like:
--- subversion/libsvn_fs_fs/revprops.c (revision 1680818)
+++ subversion/libsvn_fs_fs/revprops.c (working copy)
@@ -874,7 +874,7 @@
APR_WRITE | APR_CREATE, APR_OS_DEFAULT, pool));
- *stream = svn_stream_from_aprfile2(file, FALSE, pool);
+ SVN_ERR(svn_stream_from_aprfile3(stream, file, FALSE, TRUE, pool));
@@ -1205,7 +1205,8 @@
SVN_ERR(svn__compress(uncompressed, compressed, compression_level));
/* write the pack file content to disk */
- stream = svn_stream_from_aprfile2(pack_file, FALSE, scratch_pool);
+ SVN_ERR(svn_stream_from_aprfile3(&stream, pack_file, FALSE, TRUE,
SVN_ERR(svn_stream_write(stream, compressed->data, &compressed->len));
A patch like that is possibly easier to review than r1680819.
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Received on 2015-05-22 00:09:24 CEST