On 06.02.2015 15:00, Ivan Zhakov wrote: > 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.
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
