Re: svn_fs_node_created_rev applied to transaction root differs between FSFS and BDB

On 9/11/06, Malcolm Rowe <malcolm-svn-dev@farside.org.uk> wrote:
> > * Who's right, BDB or FSFS? I assume BDB since its result doesn't lead
> > to the dirprop conflict bug.
>
> I wouldn't assume either. Does creating a new transaction conceptually
> cause the transaction's root to be immediately modified? If the answer
> to that question is yes, FSFS is right; if no, BDB is right.

This is close enough to philosophy that I'd like to hear what others think.

> Of course, as soon as you actually change the transaction (other than by
> setting revprops), the transaction root will be guaranteed to be modified,
> and so the function will always return SVN_INVALID_REVNUM for the root
> from then on.

Sure, but this is even before that happens.

> I assume that BDB does actually return SVN_INVALID_REVNUM for the root
> once the transaction is modified?

Yup, and that's even tested (in fs-test.c's test_node_created_rev). I
just added a test for this issue there too (replacing the "repos" test
I made which had nothing to do with the repos layer).

> I'm surprised that our out-of-dated-ness checks are dependent on what
> appears to be very much a corner case - whether an unmodified transaction
> has a modified root. Where's the code that's actually checking this?

Check out change_dir_prop in libsvn_repos/commit.c: it's the
change_dir for the repos commit editor. When the directory is the
root, db->base_rev is the base_rev argument passed to the editor's
open_root. If that's a valid revnum, it finds the node_created_rev of
the path inside the txn, and complains if it's newer than the
base_rev.

Note that created_rev isn't checked for validity: you're always
allowed to commit dirprop changes against a node which has already
been modified in the txn, which makes sense. (It might want explicit
documentation, though, instead of just relying on the fact that
SVN_INVALID_REVNUM is less than all valid revnums.)

It seems like this, as well as delete_entry and open_file in that
editor, are relying on "an invalid created_rev means we've already
decided it's not out of date", which seems reasonable enough to me. So
I do feel like FSFS is at fault

--dave

-- 
David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org