subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bert Huijben" <b...@qqmail.nl>
Subject RE: svn commit: r1716548 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
Date Fri, 27 Nov 2015 09:27:39 GMT


> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> Sent: vrijdag 27 november 2015 08:50
> To: Philip Martin <philip.martin@wandisco.com>
> Cc: Bert Huijben <bert@qqmail.nl>; 'Branko Čibej' <brane@apache.org>;
> dev@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.

	Bert


Mime
View raw message