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

Ivan Zhakov <ivan_at_visualsvn.com> writes:

> Yes, this patch looks easier to review, the only problem that it's
> incomplete. I'm attaching minimal working patch with
> svn_stream_from_file3() against trunk_at_r1680818.

I like this patch better. It puts the flush into 2 extra places, 4 in
total, in FSFS, and the corresponding 4 places in FSX. For 1.8 we would
make the new API private: svn_stream__flush_to_disk_on_close.

Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h (revision 1680818)
+++ subversion/include/svn_io.h (working copy)
@@ -1087,6 +1087,17 @@ svn_stream_from_aprfile2(apr_file_t *file,
                          svn_boolean_t disown,
                          apr_pool_t *pool);
 
+/** Arrange for the APR file underlying @a stream to be flushed to
+ * disk before being closed. Returns SVN_ERR_STREAM_NOT_SUPPORTED for
+ * streams that cannot be flushed. This can be applied to streams
+ * created by svn_stream_from_aprfile2(), svn_stream_open_unique() and
+ * svn_stream_open_writable().
+ *
+ * @since New in 1.9.
+ */
+svn_error_t *
+svn_stream_flush_to_disk_on_close(svn_stream_t *stream);
+
 /** Similar to svn_stream_from_aprfile2(), except that the file will
  * always be disowned.
  *
Index: subversion/libsvn_fs_fs/revprops.c
===================================================================
--- subversion/libsvn_fs_fs/revprops.c (revision 1680818)
+++ subversion/libsvn_fs_fs/revprops.c (working copy)
@@ -660,6 +660,7 @@ write_non_packed_revprop(const char **final_path,
   SVN_ERR(svn_stream_open_unique(&stream, tmp_path,
                                  svn_dirent_dirname(*final_path, pool),
                                  svn_io_file_del_none, pool, pool));
+ SVN_ERR(svn_stream_flush_to_disk_on_close(stream));
   SVN_ERR(svn_hash_write2(proplist, stream, SVN_HASH_TERMINATOR, pool));
   SVN_ERR(svn_stream_close(stream));
 
@@ -875,6 +876,7 @@ repack_stream_open(svn_stream_t **stream,
                                                   pool),
                            APR_WRITE | APR_CREATE, APR_OS_DEFAULT, pool));
   *stream = svn_stream_from_aprfile2(file, FALSE, pool);
+ SVN_ERR(svn_stream_flush_to_disk_on_close(*stream));
 
   return SVN_NO_ERROR;
 }
@@ -1206,6 +1208,7 @@ svn_fs_fs__copy_revprops(const char *pack_file_dir
 
   /* write the pack file content to disk */
   stream = svn_stream_from_aprfile2(pack_file, FALSE, scratch_pool);
+ SVN_ERR(svn_stream_flush_to_disk_on_close(stream));
   SVN_ERR(svn_stream_write(stream, compressed->data, &compressed->len));
   SVN_ERR(svn_stream_close(stream));
 
@@ -1244,6 +1247,7 @@ svn_fs_fs__pack_revprops_shard(const char *pack_fi
   SVN_ERR(svn_io_dir_make(pack_file_dir, APR_OS_DEFAULT, scratch_pool));
   SVN_ERR(svn_stream_open_writable(&manifest_stream, manifest_file_path,
                                    scratch_pool, scratch_pool));
+ SVN_ERR(svn_stream_flush_to_disk_on_close(manifest_stream));
 
   /* revisions to handle. Special case: revision 0 */
   start_rev = (svn_revnum_t) (shard * max_files_per_dir);
Index: subversion/libsvn_fs_x/revprops.c
===================================================================
--- subversion/libsvn_fs_x/revprops.c (revision 1680818)
+++ subversion/libsvn_fs_x/revprops.c (working copy)
@@ -1188,6 +1188,7 @@ write_non_packed_revprop(const char **final_path,
                                                     scratch_pool),
                                  svn_io_file_del_none,
                                  result_pool, scratch_pool));
+ SVN_ERR(svn_stream_flush_to_disk_on_close(stream));
   SVN_ERR(svn_hash_write2(proplist, stream, SVN_HASH_TERMINATOR,
                           scratch_pool));
   SVN_ERR(svn_stream_close(stream));
@@ -1425,6 +1426,7 @@ repack_stream_open(svn_stream_t **stream,
                            APR_WRITE | APR_CREATE, APR_OS_DEFAULT,
                            result_pool));
   *stream = svn_stream_from_aprfile2(file, FALSE, result_pool);
+ SVN_ERR(svn_stream_flush_to_disk_on_close(*stream));
 
   return SVN_NO_ERROR;
 }
@@ -1797,6 +1799,7 @@ svn_fs_x__copy_revprops(const char *pack_file_dir,
 
   /* write the pack file content to disk */
   stream = svn_stream_from_aprfile2(pack_file, FALSE, scratch_pool);
+ SVN_ERR(svn_stream_flush_to_disk_on_close(stream));
   SVN_ERR(svn_stream_write(stream, compressed->data, &compressed->len));
   SVN_ERR(svn_stream_close(stream));
 
@@ -1835,6 +1838,7 @@ svn_fs_x__pack_revprops_shard(const char *pack_fil
   SVN_ERR(svn_io_dir_make(pack_file_dir, APR_OS_DEFAULT, scratch_pool));
   SVN_ERR(svn_stream_open_writable(&manifest_stream, manifest_file_path,
                                    scratch_pool, scratch_pool));
+ SVN_ERR(svn_stream_flush_to_disk_on_close(manifest_stream));
 
   /* revisions to handle. Special case: revision 0 */
   start_rev = (svn_revnum_t) (shard * max_files_per_dir);
Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c (revision 1680818)
+++ subversion/libsvn_subr/stream.c (working copy)
@@ -911,6 +911,29 @@ close_handler_apr(void *baton)
 }
 
 static svn_error_t *
+close_handler_flush(void *baton)
+{
+ struct baton_apr *btn = baton;
+
+ SVN_ERR(svn_io_file_flush_to_disk(btn->file, btn->pool));
+ SVN_ERR(close_handler_apr(baton));
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_stream_flush_to_disk_on_close(svn_stream_t *stream)
+{
+ if (stream->close_fn != close_handler_apr)
+ return svn_error_create(SVN_ERR_STREAM_NOT_SUPPORTED, NULL,
+ _("No closing file to flush"));
+
+ svn_stream_set_close(stream, close_handler_flush);
+
+ return SVN_NO_ERROR;
+}
+
+
+static svn_error_t *
 mark_handler_apr(void *baton, svn_stream_mark_t **mark, apr_pool_t *pool)
 {
   struct baton_apr *btn = baton;

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*