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

RE: svn commit: r1680528 - /subversion/branches/fsx-1.10/subversion/libsvn_fs_x/transaction.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Wed, 20 May 2015 13:40:00 +0200

> -----Original Message-----
> From: stefan2_at_apache.org [mailto:stefan2_at_apache.org]
> Sent: woensdag 20 mei 2015 13:33
> To: commits_at_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));
>
Received on 2015-05-20 13:40:13 CEST

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