Hey fsfs experts -- cmpilato fears that fsfs may have this same logic
bug, regarding the recording of changes. Maybe you can take a look.
On Feb 9, 2005, at 2:44 PM, sussman@tigris.org wrote:
> Author: sussman
> Date: Wed Feb 9 14:44:47 2005
> New Revision: 12956
>
> Modified:
> branches/locking/subversion/libsvn_fs_base/tree.c
> Log:
> Locking branch: obscure BDB changes-table bugfix, done with cmpilato.
>
> Normal clients don't stimulate this bug, but the new lock-checking
> behavior in svn_fs_commit_txn() stimulated it in changes-test #5. The
> locking branch should now pass regression tests again.
>
> In particular, changes-test #5 was deleting a child of a dir, then
> deleting the parent dir, all in one transaction. When
> svn_fs_commit_txn() started looping over over the list of txn changes
> (to enforce any locks), it was getting back a mutable-id for the
> deleted parent dir... a mutable-id of a node that was now gone!
>
> * subversion/libsvn_fs_base/tree.c
> (txn_body_delete): don't write a mutable node-id into the changes
> list.
> Instead, write the immutable predecessor id.
>
>
> Modified: branches/locking/subversion/libsvn_fs_base/tree.c
> Url:
> http://svn.collab.net/viewcvs/svn/branches/locking/subversion/
> libsvn_fs_base/tree.c?view=diff&rev=12956&p1=branches/locking/
> subversion/libsvn_fs_base/tree.c&r1=12955&p2=branches/locking/
> subversion/libsvn_fs_base/tree.c&r2=12956
> =======================================================================
> =======
> --- branches/locking/subversion/libsvn_fs_base/tree.c (original)
> +++ branches/locking/subversion/libsvn_fs_base/tree.c Wed Feb 9
> 14:44:47 2005
> @@ -2926,6 +2926,7 @@
> const char *path = args->path;
> parent_path_t *parent_path;
> const char *txn_id = root->txn;
> + const svn_fs_id_t *change_id;
>
> if (! root->is_txn_root)
> return not_txn (root);
> @@ -2956,9 +2957,26 @@
> txn_id, trail));
>
> /* Make a record of this modification in the changes table. */
> - SVN_ERR (add_change (root->fs, txn_id, path,
> - svn_fs_base__dag_get_id (parent_path->node),
> - svn_fs_path_change_delete, 0, 0, trail));
> +
> + /* The goal: avoid writing a 'changes' entry of a mutable node-id.
> + If it's mutable, we try to use its predecessor-id instead, which
> + is guaranteed to be immutable. In the unusual case that there is
> + no predecessor id (e.g. we created and deleted the object all in
> + the same transaction), then we don't want to record any
> + 'changes' information at all -- so we send a 'reset' command. */
> +
> + if (svn_fs_base__dag_check_mutable (parent_path->node, txn_id))
> + SVN_ERR (svn_fs_base__dag_get_predecessor_id (&change_id,
> + parent_path->node,
> + trail));
> + else
> + change_id = svn_fs_base__dag_get_id (parent_path->node);
> +
> + SVN_ERR (add_change (root->fs, txn_id, path, change_id,
> + change_id ? svn_fs_path_change_delete :
> + svn_fs_path_change_reset,
> + 0, 0, trail));
> +
>
> return SVN_NO_ERROR;
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Feb 9 21:56:19 2005