You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jon Foster <Jo...@cabot.co.uk> on 2010/09/22 14:56:19 UTC
RE: svnsync backwards-compatibility with older servers (Was: Re: svn
commit: r999421)
Hi,
Daniel Shahaf wrote:
> Stefan Sperling wrote on Wed, Sep 22, 2010 at 15:45:41 +0200:
> > On Wed, Sep 22, 2010 at 03:33:20PM +0200, Daniel Shahaf wrote:
> > > Stefan Sperling wrote on Wed, Sep 22, 2010 at 14:31:58 +0200:
> > > > On Wed, Sep 22, 2010 at 01:05:24PM +0200, Daniel Shahaf wrote:
> > > > > Daniel Shahaf wrote on Tue, Sep 21, 2010 at 18:50:56 +0200:
> > > > > > How about combining (1) and (3) --- that is, using the old
> > > > > > locking mode (with its known race condition) but print a
> > > > > > warning that the mirror server should be upgraded? That
> > > > > > seems better than a fatal error for a condition that needs
> > > > > > to be fixed on the server end.
> > > >
> > > > No replies, so I've implemented this in r999868. Let me know
> > > > what you think :).
I spent a while thinking about this. I think this is reasonable.
I think there will be people who use svnsync and have found that
the 1.6 locking has been working well enough for them. I now agree
we should let them keep using the old racy locking until they get
around to upgrading the mirror servers.
(I think "making upgrading to 1.7 easy" is probably the fastest
way to get everyone using fixed locking. And saying "svnsync 1.7
doesn't work against a 1.6 mirror unless you do some complicated
external locking stuff" makes the upgrade much harder).
Like Stefan, I was wondering whether this should be a command-line
option (either --allow-unsafe-locking or --safe-locking-only); I'm
not sure whether that would help or not. Printing the warning
every time might be enough.
> > > Does the corruption issue (considering its severity and
> > > commonness) really warrant an explicit flag?
> >
> > See Jon's comment made earlier about data integrity.
> >
> > But, granted, there isn't any loss of versioned data when the
> > race triggers. The mirror needs to be unwedged by tweaking the
> > svn:sync-* revprops at revision 0. It will then resume operation.
>
> Unwedging the mirror is not bad; if some commit were replayed twice to
> the mirror, /then/ I would be worried. :-)
FYI, it *can* replay commits twice to the mirror.
Real user bug report:
http://svn.haxx.se/users/archive-2010-01/0184.shtml
My analysis of that bug report:
http://svn.haxx.se/users/archive-2010-01/0197.shtml
To recover from that, the sysadmin would have to either delete
and remirror, dump/load, or restore from backup. On a big
repository, that's a lot of downtime.
And from Daniel's earlier mail:
> While here, there is a similar issue in svn_client_revprop_set2(). On
> the branch, it tries to be atomic if possible; but on trunk, it has
> always used a racy implementation. What do you think should be done
> in that case?
This is only used for "svn propedit" and GUI equivalents, right?
I think trying to be atomic if possible (i.e. what you do on the branch)
is the right approach. This function existed before atomic-revprops and
the race is even documented in it's docstring. Fixing the race is a
nice improvement.
The risk of hitting the race in svn_client_revprop_set2() is small, as
it requires two humans to edit a revprop at the same time. The
consequences are minor too - a possibly-incorrect revprop, which can
be easily propedited again.
I wouldn't worry about adding a "fail if not atomic" parameter. For
the weird usecases where people care about atomicity, there's svn_ra.
Kind regards,
Jon
**********************************************************************
This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd.
If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone.
Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232
Co. Registered in England number 02817269
Please contact the sender if you believe you have received this email in error.
**********************************************************************
______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email
______________________________________________________________________
Re: svnsync backwards-compatibility with older servers (Was: Re:
svn commit: r999421)
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 09/22/2010 12:43 PM, Jon Foster wrote:
> Daniel wrote:
>> Okay; then we should document (today) that when svnsync is used with
>> a 1.6 server, then the admin must ensure that two instances never start
>> at about the same time. (unless external locking is used)
>>
>> Not sure where this would go (the site, the book, the mailing lists..)
>
> We should fix the book. That's the canonical reference, and it
> currently tells everyone to use a buggy configuration.
File an issue in the svnbook tracker, and/or send a patch to the
svnbook-dev@red-bean.com list. I'll watch for it.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
RE: svnsync backwards-compatibility with older servers (Was: Re: svn
commit: r999421)
Posted by Jon Foster <Jo...@cabot.co.uk>.
Daniel wrote:
> Okay; then we should document (today) that when svnsync is used with
> a 1.6 server, then the admin must ensure that two instances never
start
> at about the same time. (unless external locking is used)
>
> Not sure where this would go (the site, the book, the mailing lists..)
We should fix the book. That's the canonical reference, and it
currently tells everyone to use a buggy configuration.
Kind regards,
Jon
**********************************************************************
This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd.
If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone.
Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232
Co. Registered in England number 02817269
Please contact the sender if you believe you have received this email in error.
**********************************************************************
______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email
______________________________________________________________________
Re: svnsync backwards-compatibility with older servers (Was: Re:
svn commit: r999421)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
[ snipping tons of context ... ]
Jon Foster wrote on Wed, Sep 22, 2010 at 15:56:19 +0100:
> Daniel Shahaf wrote:
> > Unwedging the mirror is not bad; if some commit were replayed twice to
> > the mirror, /then/ I would be worried. :-)
>
> FYI, it *can* replay commits twice to the mirror.
>
> Real user bug report:
> http://svn.haxx.se/users/archive-2010-01/0184.shtml
>
> My analysis of that bug report:
> http://svn.haxx.se/users/archive-2010-01/0197.shtml
>
> To recover from that, the sysadmin would have to either delete
> and remirror, dump/load, or restore from backup. On a big
> repository, that's a lot of downtime.
>
Okay; then we should document (today) that when svnsync is used with
a 1.6 server, then the admin must ensure that two instances never start
at about the same time. (unless external locking is used)
Not sure where this would go (the site, the book, the mailing lists..)
> And from Daniel's earlier mail:
> > While here, there is a similar issue in svn_client_revprop_set2(). On
> > the branch, it tries to be atomic if possible; but on trunk, it has
> > always used a racy implementation. What do you think should be done
> > in that case?
>
> This is only used for "svn propedit" and GUI equivalents, right?
>
Yes.
> I think trying to be atomic if possible (i.e. what you do on the branch)
> is the right approach. This function existed before atomic-revprops and
> the race is even documented in it's docstring. Fixing the race is a
> nice improvement.
>
> The risk of hitting the race in svn_client_revprop_set2() is small, as
> it requires two humans to edit a revprop at the same time. The
> consequences are minor too - a possibly-incorrect revprop, which can
> be easily propedited again.
>
Indeed.
> I wouldn't worry about adding a "fail if not atomic" parameter. For
> the weird usecases where people care about atomicity, there's svn_ra.