You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2013/02/18 22:21:16 UTC

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


> -----Original Message-----
> From: rhuijben@apache.org [mailto:rhuijben@apache.org]
> Sent: maandag 18 februari 2013 21:51
> To: commits@subversion.apache.org
> Subject: svn commit: r1447487 - /subversion/trunk/subversion/svn/svn.c
> 
> Author: rhuijben
> Date: Mon Feb 18 20:51:15 2013
> New Revision: 1447487
> 
> URL: http://svn.apache.org/r1447487
> Log:
> * subversion/svn/svn.c
>   (sub_mail): Add comment noting backwards compatibility concern.
> 
> Modified:
>     subversion/trunk/subversion/svn/svn.c
> 
> Modified: subversion/trunk/subversion/svn/svn.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/svn.c?rev=
> 1447487&r1=1447486&r2=1447487&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/svn/svn.c (original)
> +++ subversion/trunk/subversion/svn/svn.c Mon Feb 18 20:51:15 2013
> @@ -2530,6 +2530,17 @@ sub_main(int argc, const char *argv[], a
>                   SVN_CONFIG_SECTION_WORKING_COPY,
>                   SVN_CONFIG_OPTION_SQLITE_EXCLUSIVE,
>                   NULL);
> +  /*
> ##########################################################
> ###############
> +     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@s.a.o.
> +         first. This behavior should be opt-in, not opt-out until 2.0.
> +
> ##########################################################
> ###############
> +   */
>    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.

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'.


But this behavior change certainly explains why my recent performance profiles turned up pretty useless, while in the past they provided good inside in where we can improve the performance in real user performance in clients like AnkhSVN and TortoiseSVN.

	Bert


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

Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, Feb 19, 2013 at 12:50 PM, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: MARTIN PHILIP [mailto:codematters@ntlworld.com] On Behalf Of
>> Philip Martin
>> Sent: dinsdag 19 februari 2013 11:50
>> To: Bert Huijben
>> Cc: dev@subversion.apache.org
>> Subject: Re: svn commit: r1447487 - /subversion/trunk/subversion/svn/svn.c
>>
>> "Bert Huijben" <be...@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@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

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

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: MARTIN PHILIP [mailto:codematters@ntlworld.com] On Behalf Of
> Philip Martin
> Sent: dinsdag 19 februari 2013 11:50
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1447487 - /subversion/trunk/subversion/svn/svn.c
> 
> "Bert Huijben" <be...@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@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)


	Bert
> 
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download


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

Posted by Branko Čibej <br...@wandisco.com>.
On 19.02.2013 11:49, Philip Martin wrote:
> 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. 

I think it's less a question about which group is bigger and more which
group is going to chew our heads off if parallel actions on the same
working copy suddenly start blocking each other. Consider that TSVN by
its own lonesome is already doing this, IIRC, it updates status icons
asynchronously?

-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com


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

Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@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@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.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: exclusive WC locks by default? (was: Re: svn commit: r1447487 - /subversion/trunk/subversion/svn/svn.c)

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Justin Erenkrantz wrote on Mon, Feb 18, 2013 at 22:18:33 -0500:
> +1 - "svn diff" while running "svn commit" (blocking waiting for $EDITOR to
> exit) is my SOP as well.  If we break that by default, I consider it a
> regression.   -- justin

+1

Re: exclusive WC locks by default? (was: Re: svn commit: r1447487 - /subversion/trunk/subversion/svn/svn.c)

Posted by Ben Reser <be...@reser.org>.
On Mon, Feb 18, 2013 at 7:18 PM, Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> +1 - "svn diff" while running "svn commit" (blocking waiting for $EDITOR to
> exit) is my SOP as well.  If we break that by default, I consider it a
> regression.   -- justin

+1

Re: exclusive WC locks by default? (was: Re: svn commit: r1447487 - /subversion/trunk/subversion/svn/svn.c)

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Feb 18, 2013 7:08 PM, "Stefan Sperling" <st...@elego.de> wrote:
>
> On Mon, Feb 18, 2013 at 10:21:16PM +0100, Bert Huijben wrote:
> > 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.
> >
> > And all those other clients would just hang without a way to recover...
>
> I only use the command line but run 'svn' commands concurrently quite
often.
>
> E.g. I run 'svn diff' while 'svn commit' has the log message editor open.
> I also use 'svn log --diff' a lot and sometimes still have it running
while
> committing from the same working copy.
>
> I've disabled exclusive locking in my client configuration for this reason
> and wouldn't mind either if exclusive locking was off by default.

+1 - "svn diff" while running "svn commit" (blocking waiting for $EDITOR to
exit) is my SOP as well.  If we break that by default, I consider it a
regression.   -- justin

exclusive WC locks by default? (was: Re: svn commit: r1447487 - /subversion/trunk/subversion/svn/svn.c)

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Feb 18, 2013 at 10:21:16PM +0100, Bert Huijben wrote:
> 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.
> 
> And all those other clients would just hang without a way to recover... 

I only use the command line but run 'svn' commands concurrently quite often.

E.g. I run 'svn diff' while 'svn commit' has the log message editor open.
I also use 'svn log --diff' a lot and sometimes still have it running while
committing from the same working copy.

I've disabled exclusive locking in my client configuration for this reason
and wouldn't mind either if exclusive locking was off by default.