Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm
Precedence: bulk
From: "Bert Huijben" <bert@qqmail.nl>
To: "'Daniel Shahaf'" <d.s@daniel.shahaf.name>,
	"'Philip Martin'" <philip.martin@wandisco.com>
Cc: =?utf-8?Q?'Branko_=C4=8Cibej'?= <brane@apache.org>,
	<dev@subversion.apache.org>
References: <20151125213120.9C7FE3A02EE@svn01-us-west.apache.org>
 <010901d127d0$81dadc60$85909520$@qqmail.nl> <5656395A.60807@apache.org>
 <011601d127d7$8def30d0$a9cd9270$@qqmail.nl> <87610olvpc.fsf@wandisco.com>
 <20151127075012.GK1899@tarsus.local2>
In-Reply-To: <20151127075012.GK1899@tarsus.local2>
Subject: RE: svn commit: r1716548 -
 /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
Date: Fri, 27 Nov 2015 10:27:39 +0100
Message-ID: <02bd01d128f5$dcfe58d0$96fb0a70$@qqmail.nl>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Thread-Index: 
 AQKdgnenhKXBgV+Z7gXJrJQ9xKlC7QGXwi9DA05vYpcBq+ngrALRo9OcARpg9HGcwudv8A==
Content-Language: nl


> -----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 =C4=8Cibej' =
<brane@apache.org>;
> dev@subversion.apache.org
> Subject: Re: svn commit: r1716548 -
> /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
>=20
> 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.
>=20
> 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.
>=20
> To be clear, I'm not opposed to applying your patches =E2=80=94 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.
>=20
> 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.


=3D=3D=3D=3D=3D=3D=3D
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.


=3D=3D=3D=3D=3D=3D=3D
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