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

RE: svn commit: r1716548 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Fri, 27 Nov 2015 10:27:39 +0100

> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s_at_daniel.shahaf.name]
> Sent: vrijdag 27 november 2015 08:50
> To: Philip Martin <philip.martin_at_wandisco.com>
> Cc: Bert Huijben <bert_at_qqmail.nl>; 'Branko Čibej' <brane_at_apache.org>;
> dev_at_subversion.apache.org
> Subject: Re: svn commit: r1716548 -
> /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
> Philip Martin wrote on Thu, Nov 26, 2015 at 13:46:39 +0000:
> > I suppose one way to fix this would be to ensure that every BDB revision
> > generates a new node-revision-id.
> I wouldn't call this a fix; I think it is a workaround. A "fix" would
> be to figure out why bdb reports the wrong revision number, not to make
> the place it wrongly looks the value up in contain the right value.
> To be clear, I'm not opposed to applying your patches — I do think they
> are an improvement. I just want it to be clear: re-using the noderev
> wasn't wrong; some other code is wrong, and we don't know which code
> exactly that is.
> Can the problem happen on nodes other than the root? I haven't tried
> it, but I wonder if a open_root/open_directory/close_directory/close_edit
> editor drive might lead to an instance of this problem on the directory
> that was opened and closed without modification.

I'm not sure what the real bug is here:

* For 1.10 I added some verifications on incoming revision numbers in mod_dav_svn. These expect that the base revision numbers passed to editor functions are always lower than the revision against we commit. I still think this is a good check.
(But I'm not 100% sure if we correctly find the revision we commit against)
Before our svn-HTTPv2 we had a similar base revision check in a different code path... But we skip this code path for our streamlined v2 protocol.

* To handle this we open a transaction and reload it a few times via its name. Once created it returns one revision. Once reloaded it returns a different -older- revision. I think *this behavior* is broken. We should either directly return the older revision... or always return the newer revision.

-> This could be as simple as storing it in the transaction name, by appending something like "/r123" to the name if we detect this case.

And when researching this issue I found that the root of the repository still has the same 'last-changed-*' values in this case of a no-op revision.

With 1.9 we optimized some code in the repos layer under the assumption that the repos-root 'always changes' in each and every revision. I think this still works ok, but I'm not 100% sure.

And all of this brings more interesting cases to the discussion on what changes are... So some of this should probably be added to Julian's document on what path changes are.

I'm pretty sure this 'problem' adds other nice features where dump+load may slightly alter behavior... even though the repository is still 100% the same in textual+property changes.

Received on 2015-11-27 10:28:07 CET

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