[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

From: Philip Martin <philip.martin_at_wandisco.com>
Date: Thu, 21 May 2015 23:09:08 +0100

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:

Index: subversion/libsvn_fs_fs/revprops.c
===================================================================
--- subversion/libsvn_fs_fs/revprops.c (revision 1680818)
+++ subversion/libsvn_fs_fs/revprops.c (working copy)
@@ -874,7 +874,7 @@
                                                   new_filename->data,
                                                   pool),
                            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));
 
   return SVN_NO_ERROR;
 }
@@ -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,
+ scratch_pool));
   SVN_ERR(svn_stream_write(stream, compressed->data, &compressed->len));
   SVN_ERR(svn_stream_close(stream));
 
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

This is an archived mail posted to the Subversion Dev mailing list.