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.
--
Ivan Zhakov
Received on 2015-02-06 15:02:40 CET