Author: philip
Date: Mon Mar 20 19:13:28 2023
New Revision: 1908595
URL: http://svn.apache.org/viewvc?rev=1908595&view=rev
Log:
Reduce spurious conflicts during commit. Two txns that are not meant
to conflict, a file content change and a directory property change,
can generate a spurious conflict when the txns are built in parallel.
When doing this from a working copy, simply retrying the commit would
allow it to complete, no update was necessary, indicating that the
commit was spurious.
Spurious conflicts are still possible, but are now less likely.
* subversion/libsvn_fs_fs/tree.c
(merge): Handle ancestor directory property change.
* subversion/libsvn_fs_x/tree.c
(merge): Handle ancestor directory property change.
* subversion/tests/libsvn_fs/fs-test.c
(unordered_txn_dirprops, dir_prop_merge): Now only fails on BDB.
Modified:
subversion/trunk/subversion/libsvn_fs_fs/tree.c
subversion/trunk/subversion/libsvn_fs_x/tree.c
subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1908595&r1=1908594&r2=1908595&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Mon Mar 20 19:13:28 2023
@@ -1977,7 +1977,30 @@ merge(svn_stringbuf_t *conflict_p,
different contents. */
SVN_ERR(svn_fs_fs__prop_rep_equal(&same, fs, src_nr, anc_nr, pool));
if (! same)
- return conflict_err(conflict_p, target_path);
+ {
+ apr_hash_t *proplist;
+
+ /* There is a prop difference between source and ancestor, if
+ there is no property difference between target and ancestor
+ then this txn didn't change props and we can simply update
+ target to match source.
+
+ Commit calls merge in a loop until it manages to get the
+ write lock with source=head. Copying the properties like
+ this will only work on the first iteration as later
+ iterations will see tgt.prop!=anc.prop and won't know that
+ the txn did not change properties. This means we
+ successfully handle a race between this txn and a
+ propchange txn while building this txn, but we don't handle
+ a race that occurs after the first iteration of the merge
+ loop -- we will raise an unwanted conflict. */
+ SVN_ERR(svn_fs_fs__prop_rep_equal(&same, fs, tgt_nr, anc_nr, pool));
+ if (! same)
+ return conflict_err(conflict_p, target_path);
+
+ SVN_ERR(svn_fs_fs__dag_get_proplist(&proplist, source, pool));
+ SVN_ERR(svn_fs_fs__dag_set_proplist(target, proplist, pool));
+ }
/* The directory entries got changed in the repository but the directory
properties did not. */
Modified: subversion/trunk/subversion/libsvn_fs_x/tree.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/tree.c?rev=1908595&r1=1908594&r2=1908595&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/tree.c Mon Mar 20 19:13:28 2023
@@ -890,7 +890,32 @@ merge(svn_stringbuf_t *conflict_p,
different contents. */
SVN_ERR(svn_fs_x__prop_rep_equal(&same, fs, src_nr, anc_nr, TRUE, pool));
if (! same)
- return conflict_err(conflict_p, target_path);
+ {
+ apr_hash_t *proplist;
+
+ /* There is a prop difference between source and ancestor, if
+ there is no property difference between target and ancestor
+ then this txn didn't change props and we can simply update
+ target to match source.
+
+ Commit calls merge in a loop until it manages to get the
+ write lock with source=head. Copying the properties like
+ this will only work on the first iteration as later
+ iterations will see tgt.prop!=anc.prop and won't know that
+ the txn did not change properties. This means we
+ successfully handle a race between this txn and a
+ propchange txn while building this txn, but we don't handle
+ a race that occurs after the first iteration of the merge
+ loop -- we will raise an unwanted conflict. */
+ SVN_ERR(svn_fs_x__prop_rep_equal(&same, fs, tgt_nr, anc_nr,
+ TRUE, pool));
+ if (! same)
+ return conflict_err(conflict_p, target_path);
+
+ SVN_ERR(svn_fs_x__dag_get_proplist(&proplist, source, pool, pool));
+ SVN_ERR(svn_fs_x__dag_set_proplist(target, proplist, pool));
+ }
+
/* The directory entries got changed in the repository but the directory
properties did not. */
Modified: subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/fs-test.c?rev=1908595&r1=1908594&r2=1908595&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Mon Mar 20 19:13:28
2023
@@ -4990,10 +4990,17 @@ unordered_txn_dirprops(const svn_test_op
/* Commit the second one first. */
SVN_ERR(test_commit_txn(&new_rev, txn2, NULL, pool));
- /* Then commit the first -- but expect a conflict due to the
- propchanges made by the other txn. */
- SVN_ERR(test_commit_txn(¬_rev, txn, "/A/B", pool));
- SVN_ERR(svn_fs_abort_txn(txn, pool));
+ if (is_bdb)
+ {
+ /* Then commit the first -- but expect a conflict due to the
+ propchanges made by the other txn. */
+ SVN_ERR(test_commit_txn(¬_rev, txn, "/A/B", pool));
+ SVN_ERR(svn_fs_abort_txn(txn, pool));
+ }
+ else
+ {
+ SVN_ERR(test_commit_txn(&new_rev, txn, NULL, pool));
+ }
/* Now, let's try those in reverse. Open two transactions */
SVN_ERR(svn_fs_begin_txn(&txn, fs, new_rev, pool));
@@ -5641,16 +5648,16 @@ dir_prop_merge(const svn_test_opts_t *op
{
SVN_ERR(test_commit_txn(&head_rev, top_txn, "/A", pool));
SVN_ERR(svn_fs_abort_txn(top_txn, pool));
+
+ SVN_ERR(test_commit_txn(&head_rev, sub_txn, "/A/D", pool));
+ SVN_ERR(svn_fs_abort_txn(sub_txn, pool));
}
else
{
SVN_ERR(test_commit_txn(&head_rev, top_txn, NULL, pool));
+ SVN_ERR(test_commit_txn(&head_rev, sub_txn, NULL, pool));
}
- /* The inverted case is not that trivial to handle. Hence, conflict.
- Depending on the checking order, the reported conflict path differs. */
- SVN_ERR(test_commit_txn(&head_rev, sub_txn, is_bdb ? "/A/D" : "/A", pool));
- SVN_ERR(svn_fs_abort_txn(sub_txn, pool));
return SVN_NO_ERROR;
}
