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 :).
>
> The error message says "external locking"; does the svnbook define this term?
Maybe use "disabled built-in locking", like the svnsync help sync says:
--disable-locking : Disable built-in locking. Use of this option can
corrupt the mirror unless you ensure that no other
instance of svnsync is running concurrently.
> (so the error message could link to the workaround docs)
There are no workaround docs, apart from some scattered mailing list
posts :(
I'd prefer requiring people to pass either --disable-locking or
--use-pre-1.7-style-locking to write to mirrors using pre-1.7 servers.
This may be uncomfortable but will raise awareness of the issue.
The error message should explain that the old servers will expose a
race condition in svnsync's locking mechanism which can cause svnsync
meta data to be corrupted on the mirror, and that for this reason, either
the mirror server should be upgraded, or locking must be disabled entirely
or the racy algorithm must be requested by the user. Users should also be
made aware that they need to take care of preventing svnsync instances from
writing to the mirror concurrently if the mirror doesn't run a 1.7 or
later server.
> > 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?
>
> The docstring on trunk has always promised only a racy implementation.
> I'll just leave that as-is.
But the Book hasn't. That is the problem.
Our users don't read the docstrings, and shouldn't have to.
Stefan
Received on 2010-09-22 14:32:44 CEST