On 6 February 2015 at 16:50, Branko Čibej <br...@wandisco.com> wrote: > On 06.02.2015 14:13, i...@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
