You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@wandisco.com> on 2015/02/05 01:59:55 UTC

Re: svn commit: r1657451 - in /subversion/branches/reuse-ra-session: ./ subversion/libsvn_client/

On 05.02.2015 01:44, brane@apache.org wrote:
> Author: brane
> Date: Thu Feb  5 00:44:57 2015
> New Revision: 1657451
>
> URL: http://svn.apache.org/r1657451
> Log:
> On the reuse-ra-session branch: Add explicit session reuse in libsvn_client.

Anyone who's interested in the RA session cache, please take time to
review this commit.

All tests pass for me (ra_local, ra_svn and ra_dav) and I re-read the
diffs about five times to check that sessions are being released to
cache only when it's certain that they're valid. But I'd really like to
have a few more pairs of eyes on this, because there's potential for
some nasty bugs being introduced.

-- Brane

Re: svn commit: r1657451 - in /subversion/branches/reuse-ra-session: ./ subversion/libsvn_client/

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 5 February 2015 at 11:33, Branko Čibej <br...@wandisco.com> wrote:
> On 05.02.2015 01:59, Branko Čibej wrote:
>> On 05.02.2015 01:44, brane@apache.org wrote:
>>> Author: brane
>>> Date: Thu Feb  5 00:44:57 2015
>>> New Revision: 1657451
>>>
>>> URL: http://svn.apache.org/r1657451
>>> Log:
>>> On the reuse-ra-session branch: Add explicit session reuse in libsvn_client.
>> Anyone who's interested in the RA session cache, please take time to
>> review this commit.
>>
>> All tests pass for me (ra_local, ra_svn and ra_dav) and I re-read the
>> diffs about five times to check that sessions are being released to
>> cache only when it's certain that they're valid. But I'd really like to
>> have a few more pairs of eyes on this, because there's potential for
>> some nasty bugs being introduced.
>
> To get a feeling for what the RA session cache gains us, here are
> summarized stats from a few test runs:
>
>   * client-test:
>     request:21 open:13 close:3 release:18 reuse:8 cleanup:0
>
>   * merge_tests.py:
>     request:4079 open:3812 close:20 release:3431 reuse:267 cleanup:62
>
>   * externals_tests.py:
>     request:1005 open:597 close:5 release:996 reuse:408 cleanup:4
>
> To get a feeling for what this means: during the externals test run, we
> got about a thousand requests to open a RA session through
> libsvn_client, 40% of which were serviced by the reuse of a previously
> released session. Only 9 sessions were closed due to pool clean-up, 4 of
> which were owned by pools with a lifetime longer than the client
> context's pool.
>
Wow! That more that I expected. Opening RA session could cost up to 5
round-trips:
TCP connect - 1 roundtrip
SSL handshake - 2 roundtrips
Authenctication - 1 roundtrip
Options request - 1 roundtrip

Total 500-1000 ms for overseas connections.

> Our tests are not an ideal showcase for session reuse, because they run
> a zillion short-lived processes; I expect something like TortoiseSVN
> could benefit from the cache far more. Still, the externals-tests
> results seem to show that even our command-line client can benefit from
> session reuse.
>
Yes, GUI clients should benefit significantly.

-- 
Ivan Zhakov

Re: svn commit: r1657451 - in /subversion/branches/reuse-ra-session: ./ subversion/libsvn_client/

Posted by Branko Čibej <br...@wandisco.com>.
On 05.02.2015 01:59, Branko Čibej wrote:
> On 05.02.2015 01:44, brane@apache.org wrote:
>> Author: brane
>> Date: Thu Feb  5 00:44:57 2015
>> New Revision: 1657451
>>
>> URL: http://svn.apache.org/r1657451
>> Log:
>> On the reuse-ra-session branch: Add explicit session reuse in libsvn_client.
> Anyone who's interested in the RA session cache, please take time to
> review this commit.
>
> All tests pass for me (ra_local, ra_svn and ra_dav) and I re-read the
> diffs about five times to check that sessions are being released to
> cache only when it's certain that they're valid. But I'd really like to
> have a few more pairs of eyes on this, because there's potential for
> some nasty bugs being introduced.

To get a feeling for what the RA session cache gains us, here are
summarized stats from a few test runs:

  * client-test:
    request:21 open:13 close:3 release:18 reuse:8 cleanup:0

  * merge_tests.py:
    request:4079 open:3812 close:20 release:3431 reuse:267 cleanup:62

  * externals_tests.py:
    request:1005 open:597 close:5 release:996 reuse:408 cleanup:4

To get a feeling for what this means: during the externals test run, we
got about a thousand requests to open a RA session through
libsvn_client, 40% of which were serviced by the reuse of a previously
released session. Only 9 sessions were closed due to pool clean-up, 4 of
which were owned by pools with a lifetime longer than the client
context's pool.

Our tests are not an ideal showcase for session reuse, because they run
a zillion short-lived processes; I expect something like TortoiseSVN
could benefit from the cache far more. Still, the externals-tests
results seem to show that even our command-line client can benefit from
session reuse.

See the comment in the definition of svn_client__ra_cache_t in
ra_cache.c for an explanation of the various numbers. The following
condition should always be true:

    request == close + release + cleanup.

-- Brane