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

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

From: David Glasser <glasser_at_mit.edu>
Date: 2006-09-18 19:15:20 CEST

On 9/12/06, David Glasser <glasser@mit.edu> wrote:
> On 9/12/06, Malcolm Rowe <malcolm-svn-dev@farside.org.uk> wrote:
> > I guess the difference between the two approaches is that BDB treats a
> > transaction initially as an unaltered copy of the base revision's root,
> > while FSFS treats it as a new node from the start (which it always will
> > be, of course).
>
> Well, to some degree that seems to be conflating the concepts of "fs
> root" and "node of the root directory". Like, it seems to me that a
> "transaction root" is a different object than the "revision root" it
> was created from, but the root directory node of the repository under
> the transaction root should be the same object as the root directory
> node of the repository under the revision root until it is modified.

I have attached a patch that fixes this.

Since the problem comes down to "bdb doesn't make a txn root noderev
until the root actually changes, and fsfs does", I've added a field to
the FSFS noderev representation that says "I am a fresh txn root
node". (This shouldn't pose any compatibility problems, since it will
only show up in transactions, not committed revisions, and in any case
it *should* be absent in most noderevs.) This value is propagated
into the DAG structure, and svn_fs_fs__dag_get_revision gets the
revision number from its predecessor ID instead of from its own ID if
it is a fresh txn root.

I think this patch feels like a bit of a hack, and I'd appreciate
comments before I commit it. However, I do believe that this
issue is worth solving (but that I shouldn't rewrite the entire FSFS
concept of transaction roots in order to do so).

[[[
Identify unmodified transaction root nodes in FSFS as such, and make
sure that svn_fs_node_created_rev returns their base revision instead
of SVN_INVALID_REVNUM, which is compatible with BDB.
(Resolves issue #2608.)

* subversion/libsvn_fs_fs/fs.h
  (node_revision_t): Add is_fresh_txn_root field.

* subversion/libsvn_fs_fs/fs_fs.c
  (HEADER_FRESHTXNRT): Add constant to represent is_fresh_txn_root.
  (svn_fs_fs__get_node_revision): Read is_fresh_txn_root.
  (write_noderev_txn): Write is_fresh_txn_root.
  (svn_fs_fs__put_node_revision): Take a parameter saying whether
   the written node is a fresh root.
  (create_new_txn_noderev_from_rev): Document that this is only called
   for fresh roots, and change call to svn_fs_fs__put_node_revision to
   pass TRUE for new parameter.
  (svn_fs_fs__create_node, svn_fs_fs__set_entry, rep_write_contents_close,
   svn_fs_fs__create_successor, svn_fs_fs__set_proplist, write_final_rev):
   Pass FALSE for new parameter to svn_fs_fs__put_node_revision.

* subversion/libsvn_fs_fs/fs_fs.h
  (svn_fs_fs__put_node_revision): Take a parameter saying whether
   the written node is a fresh root.

* subversion/libsvn_fs_fs/tree.c
  (update_ancestry): Pass FALSE for new parameter to
   svn_fs_fs__put_node_revision.

* subversion/libsvn_fs_fs/dag.c
  (dag_node_t): Add fresh_root_predecessor_id field.
  (svn_fs_fs__dag_get_node): Initialize fresh_root_predecessor_id based
   on noderev's is_fresh_txn_root and predecessor_id.
  (svn_fs_fs__dag_get_revision): Look up revision in
   fresh_root_predecessor_id if available.

* subversion/tests/cmdline/prop_tests.py
  (test_list): Remove XFail from commit_conflict_dirprops.

* subversion/tests/libsvn_fs/fs-test.c
  (test_funcs): Remove XFAIL from test_node_created_rev.
]]]

> > ... come to think of it, won't the following scenario still be possible
> > (in either FSFS or BDB): create a transaction based on r0 [with r1 as
> > HEAD], add a file to the transaction, modifying the transaction root and
> > making _created_rev() return SVN_INVALID_REVNUM, _then_ propset the root
> > (which is out of date)?
>
> I need to look into that more. I suspect the answer is that it works
> fine because svn_client_commit calls change_dir_prop before opening
> the subdirs instead of after, but I haven't checked that, and that
> just means that the particular use in svn_client_commit happens to
> work, not that the logic in libsvn_repos is sane.

I have not yet looked into this, but I think this is independent of
making svn_fs_node_created_rev work consistently across backends.

--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

Received on Mon Sep 18 19:16:34 2006

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.