Re: svn commit: r1447487 - /subversion/trunk/subversion/svn/svn.c

On Tue, Feb 19, 2013 at 12:50 PM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: MARTIN PHILIP [mailto:codematters_at_ntlworld.com] On Behalf Of
>> Philip Martin
>> Sent: dinsdag 19 februari 2013 11:50
>> To: Bert Huijben
>> Cc: dev_at_subversion.apache.org
>> Subject: Re: svn commit: r1447487 - /subversion/trunk/subversion/svn/svn.c
>>
>> "Bert Huijben" <bert_at_qqmail.nl> writes:
>>
>> >> + This blocks every other application from accessing our wc.db at
> the
>> same
>> >> + time as this process. So instead of using the working copy lock
>> functions
>> >> + as designed other processes will already block before being able
> to
>> check
>> >> + that the working copy is locked and without a way to report what
>> blocks
>> >> + it or being able to recover using 'svn cleanup' when a process
> gets
>> stuck
>> >> +
>> >> + BH: I call this a breaking change, but let's discuss that on
> dev_at_s.a.o.
>> >> + first. This behavior should be opt-in, not opt-out until 2.0.
>>
>> The original discussion before the commit:
>>
>> http://svn.haxx.se/dev/archive-2012-02/index.shtml#522
>>
>> The second discussion after the commit:
>>
>> http://svn.haxx.se/dev/archive-2012-10/index.shtml#365
>>
>> >> +
>> >>
>> ##########################################################
>> >> ###############
>> >> + */
>> >> if (!sqlite_exclusive)
>> >> svn_config_set(cfg_config,
>> >> SVN_CONFIG_SECTION_WORKING_COPY,
>> >
>> > If I would use 1.8 to run
>> >
>> > svn diff |more
>> >
>> > And leave that open in a console I would block TortoiseSVN, AnkhSVN,
>> > Subclipse, etc on my entire working copy, while this command doesn't
>> > even obtain a working copy lock.
>> >
>> > And as a developer on both 'svn' and AnkhSVN I often use 'svn diff'
>> > just for reviews.
>>
>> Users that mix the command line with GUIs can switch off exclusive
>> locking. If we switch the default then users that want performance need
>> to switch on exclusive locking. I don't know which group is bigger,
>> users who want performance or users who want to mix clients.
>>
>> > And all those other clients would just hang without a way to
>> > recover... or fail with some sqlite error that they can't convert in
>> > something which they can translate to 'some stupid user exlusively
>> > locked the working copy by touching it with svn'
>> >
>> > We designed working copy locks in our api to take care of the use
>> > cases where a client really needs to block out other clients on a part
>> > of the working copy, but we don't need them any more unless we revert
>> > this default change.
>> >
>> > Maybe it gives a nice performance boost, but I don't think this should
>> > be the new default for 'svn'.
>>
>> Exclusive locking is the only way I know to fix the performance
>> regression on network disks that was introduced in 1.7 and is reduced
>> but still present in 1.8.
>>
>> Subversion has always been slow on network disks but 1.7 was bad enough
>> to make large working copies unusable. Commit is a particular problem
>> because the way it scales is worse than linear with working copy size.
>> Taking a working with 12 copies of Subversion trunk:
>>
>> 1.6
>> status: 34s
>> commit: 32s
>>
>> 1.7
>> status: 45s
>> commit: 10m29s
>>
>> 1.8 shared locking
>> status: 50s
>> commit: 51s
>>
>> 1.8 exclusive locking
>> status: 17s
>> commit: 17s
>>
>> I'm running these tests with a hot OS disk cache on the server but there
>> is additional caching in the NFS layer. The NFS cache appears to have
>> some sort of timeout so repeating a run soon after the first run
>> improves the speed (the server is otherwise idle). The difference can
>> be considerable for the shorter runs, 1.8 exclusive locking 3x faster at
>> about 5s, but is less significant from the longer runs.
>>
>> It's clear 1.8 has made major improvements in commit performance: if I
>> double the working copy size I see commit times that double so your work
>> on commit has probably fixed the scaling issue. However 1.8 without
>> exclusive locking is still slower than 1.6.
>>
>> In your other mail you wrote:
>>
>> > It even breaks the old access baton apis within a single client as they
>> > sometimes need to open the same db multiple times to just detect that
>> they
>> > are the same db.
>>
>> This would only happen if such a client chose to both call the old APIs
>> and to explicitly enable exclusive locking. I don't know why a client
>> would do that. A client that simply continues to use the 1.6 API won't
>> have this problem.
>>
>> > Let's call 1.8, Subversion 2.0 if we start breaking things this way.
>>
>> Performance regressions also break things; 1.7 was bad enough to prevent
>> some users upgrading.
>>
>> If we make shared locking the default can we make exclusive locking an
>> option? Suppose we had
>>
>> [working-copy]
>> exclusive-locking = false
>> exclusive-locking-clients =
>>
>> Where exclusive-locking is a global setting that the user doesn't
>> usually set, and exclusive-locking-clients is list of client names that
>> have chosen to allow exclusive locking to be enabled. The user sets
>>
>> exclusive-locking-clients = svn
>>
>> and the svn client recognises its own name at runtime and overrides
>> the global exclusive-locking setting.
>
> Sounds good.
> We should make it easy for users to choose performance over compatibility,
> while sticking to compatibility ourselves.
>
> We shouldn't break one user group of users in 1.7 and then as response to
> help this group just break another group, which I would assume is much
> bigger. We always recommend using Subversion on local disks.
>
> We should probably make it easy for clients to support this via either some
> cmdline api or via one of the libsvn client functions? (Rev the context
> create function?)
>
> One problem here is that we only create an initial config file in
> $HOME/.subversion/ and don't really show the new options to upgrading users.
> We can't just extend an existing config file using our current apis.
>
> (I expect that the status slowdown compared to 1.7 is caused by the
> additional db query for inherited properties. If we integrate those two in a
> single wc_db call status should be at the same performance as 1.7 and commit
> should show the same result as that is mostly status now. This call is only
> done for directories that contain at least one unversioned node, so usually
> you don't see it on clean checkouts)
>

Probably not the best idea, but just to throw it out there: if
exclusive locking still significantly helps some operations (e.g. some
write operations which will also take a wc-lock for their duration),
perhaps the library / client / 'svn' could make an exclusive
db-connection specifically for only those operations. Or only for
part(s) of the operation. Perhaps even as one of the configuration
choices: exclusive-locking = true | false | sometimes :-). Dunno ...

OTOH if things get "fast enough", also on NFS, without these tricks
... all the better.

-- 
Johan