subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bert Huijben" <b...@qqmail.nl>
Subject RE: svn commit: r1680528 - /subversion/branches/fsx-1.10/subversion/libsvn_fs_x/transaction.c
Date Wed, 20 May 2015 11:40:00 GMT


> -----Original Message-----
> From: stefan2@apache.org [mailto:stefan2@apache.org]
> Sent: woensdag 20 mei 2015 13:33
> To: commits@subversion.apache.org
> Subject: svn commit: r1680528 - /subversion/branches/fsx-
> 1.10/subversion/libsvn_fs_x/transaction.c
> 
> Author: stefan2
> Date: Wed May 20 11:32:42 2015
> New Revision: 1680528
> 
> URL: http://svn.apache.org/r1680528
> Log:
> On the fsx-1.10 branch:
> Don't FSYNC changes to FSX txn properties.  FSYNC future revprops
> only after their final contents has been written.  This is how we
> already treat the revision file data.
> 
> * subversion/libsvn_fs_x/transaction.c
>   (set_txn_proplist): Don't FSYNC any txn prop change.
>   (commit_body): Flush the revprop contents here.
> 
> Modified:
>     subversion/branches/fsx-1.10/subversion/libsvn_fs_x/transaction.c
> 
> Modified: subversion/branches/fsx-1.10/subversion/libsvn_fs_x/transaction.c
> URL: http://svn.apache.org/viewvc/subversion/branches/fsx-
> 1.10/subversion/libsvn_fs_x/transaction.c?rev=1680528&r1=1680527&r2=1680
> 528&view=diff
> ================================================================
> ==============
> --- subversion/branches/fsx-1.10/subversion/libsvn_fs_x/transaction.c (original)
> +++ subversion/branches/fsx-1.10/subversion/libsvn_fs_x/transaction.c Wed
> May 20 11:32:42 2015
> @@ -1344,23 +1344,27 @@ set_txn_proplist(svn_fs_t *fs,
>                   svn_boolean_t final,
>                   apr_pool_t *scratch_pool)
>  {
> -  svn_stringbuf_t *buf;
>    svn_stream_t *stream;
> +  const char *temp_path;
> 
> -  /* Write out the new file contents to BUF. */
> -  buf = svn_stringbuf_create_ensure(1024, scratch_pool);
> -  stream = svn_stream_from_stringbuf(buf, scratch_pool);
> +  /* Write the new contents into a temporary file. */
> +  SVN_ERR(svn_stream_open_unique(&stream, &temp_path,
> +                                 svn_fs_x__path_txn_dir(fs, txn_id,
> +                                                        scratch_pool),
> +                                 svn_io_file_del_none,
> +                                 scratch_pool, scratch_pool));
>    SVN_ERR(svn_hash_write2(props, stream, SVN_HASH_TERMINATOR,
> scratch_pool));
>    SVN_ERR(svn_stream_close(stream));
> 
> -  /* Open the transaction properties file and write new contents to it. */
> -  SVN_ERR(svn_io_write_atomic((final
> +  /* Replace the old file with the new one. */
> +  SVN_ERR(svn_io_file_rename(temp_path,
> +                             (final
>                                 ? svn_fs_x__path_txn_props_final(fs, txn_id,
>                                                                  scratch_pool)
>                                 : svn_fs_x__path_txn_props(fs, txn_id,
>                                                            scratch_pool)),
> -                              buf->data, buf->len,
> -                              NULL /* copy_perms_path */, scratch_pool));
> +                              scratch_pool));
> +
>    return SVN_NO_ERROR;
>  }
> 
> @@ -3316,7 +3320,7 @@ commit_body(void *baton,
>    const char *revprop_filename, *final_revprop;
>    svn_fs_x__id_t root_id, new_root_id;
>    svn_revnum_t old_rev, new_rev;
> -  apr_file_t *proto_file;
> +  apr_file_t *proto_file, *revprop_file;
>    void *proto_file_lockcookie;
>    apr_off_t initial_offset, changed_path_offset;
>    svn_fs_x__txn_id_t txn_id = svn_fs_x__txn_get_id(cb->txn);
> @@ -3437,6 +3441,12 @@ commit_body(void *baton,
>    /* Move the revprops file into place. */
>    SVN_ERR_ASSERT(! svn_fs_x__is_packed_revprop(cb->fs, new_rev));
>    SVN_ERR(write_final_revprop(&revprop_filename, cb->txn, txn_id, subpool));
> +
> +  SVN_ERR(svn_io_file_open(&revprop_file, revprop_filename, APR_READ,
> +                           APR_OS_DEFAULT, subpool));
> +  SVN_ERR(svn_io_file_flush_to_disk(revprop_file, subpool));
> +  SVN_ERR(svn_io_file_close(revprop_file, subpool));

Perhaps this works on some platforms, but it really depends on which layer the information
is cached before flushing.

[[
  (commit_body): Flush the revprop contents here.
]]
Opening a file, flushing it and closing it isn't guaranteed to flush the information written
when the file was opened earlier on.

I would be surprised if this actually performed a hardware flush on Windows, as generally
things are attached to handles there... And if nothing is written to the handle, nor can be
written, there is not much to flush to lower layers.

On top of that close, directly followed by open is generally slow on Windows, because virusscanners
will access the file between these operations. Closing the file just once should perform much
better and will avoid retry loops.

	Bert

> +
>    final_revprop = svn_fs_x__path_revprops(cb->fs, new_rev, subpool);
>    SVN_ERR(svn_fs_x__move_into_place(revprop_filename, final_revprop,
>                                      old_rev_filename, subpool));
> 



Mime
View raw message