RE: svn commit: r1502901 - /subversion/trunk/subversion/libsvn_ra_serf/util.c

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan_at_visualsvn.com]
> Sent: maandag 15 juli 2013 14:37
> To: dev_at_subversion.apache.org; Bert Huijben
> Subject: Re: svn commit: r1502901 -
> /subversion/trunk/subversion/libsvn_ra_serf/util.c
>
> On Sun, Jul 14, 2013 at 2:22 AM, <rhuijben_at_apache.org> wrote:
> > Author: rhuijben
> > Date: Sat Jul 13 22:22:51 2013
> > New Revision: 1502901
> >
> > URL: http://svn.apache.org/r1502901
> > Log:
> > Following up on r1502811, make ra_serf capable of reusing ra sessions after
> > a log callback returns a SVN_ERR_CEASE_INVOCATION error, by properly
> resetting
> > the connection in serf.
> >
> > In the location where I apply this change, we would have to read the
> remaining
> > part of the request to make the ra-session reusable, while we already
> know that
> > there are no more outstanding requests on the connection.
> >
> > * subversion/libsvn_ra_serf/util.c
> > (svn_ra_serf__context_run_one): If the context loop returns a
> > SVN_ERR_CEASE_INVOCATION error, reset the connection.
> >
> Hi Bert,
>
> It looks like workaround for issue at wrong layer: I think serf should
> automatically reset connection is this case.
>
> Another question why reset connection only on
> SVN_ERR_CEASE_INVOCATION
> error? What is the reason to leave session not reusable after other
> errors?

I tried to keep the patch local until serf handles this problem for us in a cleaner way.
This is currently the only error that we use in this way* and checking for it specifically allows doing the least amount of work necessary to implement this behavior.

For all other errors we destroy the pool in which the ra session lives at some later point in time.

In theory we should do this for +- every error that exits the response processing, but in our current usage of serf in ra_serf this is very hard to detect. The code should then also make the decision if it is better to read the rest of the response and keep the existing connection, or reset the connection as I do here. For the single request variant it is in the worst case as efficient to reset as starting a new ra session, but in general resetting is much cheaper as it allows reusing the ssl, auth, capability and chunk-detect results.

The current ra_serf error code paths that handles both EOF, AGAIN, etc. (handled by serf) and true errors (breaking the context loop) makes this very hard to implement in a clean way. We probably need a better error handling model here, to do this in a better way for 1.9.

        Bert

* this way = return from a callback to signal not interested in further results
Received on 2013-07-15 16:20:17 CEST