On 06.02.2015 15:00, Ivan Zhakov wrote:
> On 6 February 2015 at 16:50, Branko Čibej <brane_at_wandisco.com> wrote:
>> On 06.02.2015 14:13, ivan_at_apache.org wrote:
>>> Author: ivan
>>> Date: Fri Feb 6 13:13:14 2015
>>> New Revision: 1657797
>>>
>>> URL: http://svn.apache.org/r1657797
>>> Log:
>>> On the reuse-ra-session branch: Avoid svn_ra_reparent() call and network
>>> round-trip for svn:// protocol if possible.
>>>
>>> * subversion/libsvn_client/ra_cache.c
>>> (find_session_by_url): Try to find RA session with exact session URL
>>> match first.
>>>
>>> Modified:
>>> subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c
>>>
>>> Modified: subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c
>>> URL: http://svn.apache.org/viewvc/subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c?rev=1657797&r1=1657796&r2=1657797&view=diff
>>> ==============================================================================
>>> --- subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c (original)
>>> +++ subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c Fri Feb 6 13:13:14 2015
>>> @@ -401,6 +401,24 @@ find_session_by_url(cache_entry_t **cach
>>> {
>>> cache_entry_t *cache_entry;
>>>
>>> + /* Try to find RA session with exact session URL match first because
>>> + * the svn_ra_reparent() for svn:// protocol requires network round-trip.
>>> + */
>>> + APR_RING_FOREACH(cache_entry, &ra_cache->freelist,
>>> + cache_entry_t, freelist)
>>> + {
>>> + const char *session_url;
>>> + SVN_ERR_ASSERT(cache_entry->owner_pool == NULL);
>>> +
>>> + SVN_ERR(svn_ra_get_session_url(cache_entry->session, &session_url,
>>> + scratch_pool));
>>> + if (strcmp(session_url, url) == 0)
>>> + {
>>> + *cache_entry_p = cache_entry;
>>> + return SVN_NO_ERROR;
>>> + }
>>> + }
>> I think this is a mistake. We'll have to do a round-trip anyway to
>> ensure that the session is in a valid state (no connection expired,
>> etc.) before we can safely reuse it. The point of the cache is not to
>> avoid nework round-trips but to avoid expensive connection establishment.
>>
> Please let me disagree with you: we end up with optimizations like we
> currently have in merge.c if obtaining RA session will require
> additional round-trips comparing to passing some RA sessions around.
> Ideally svn_ra_reparent() should not require round trip for svn://,
> but I think we could and workaround/optimization at ra_cache layer for
> now.
>
> Regarding pinging session before reuse: RA session implementions
> should check connection state if it was inactive for a some period
> time of time. But this RA protocol specific and ra_serf already has
> code to reconnect if needed.
Well, right now we have neither automatic reconnection, nor zero-cost
reparent in ra_svn. If we don't want to add svn_ra__ping, then we need
at least the former before we can safely merge this branch. But I have
no idea how to do this.
-- Brane
Received on 2015-02-06 15:16:32 CET