You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Lieven Govaerts <li...@gmail.com> on 2007/08/28 11:25:34 UTC

Re: [serf-dev] Re: [PATCH] serf/ra_serf: add cansend callback to stop writing requests untill authn handshake is finished

Ivan,

thanks for the review. Inline some comments.

On 8/28/07, Ivan Zhakov <ch...@gmail.com> wrote:
>
> On 7/25/07, Lieven Govaerts <lg...@mobsol.be> wrote:
> > Attached two patches implement a new callback 'cansend' in serf which is
> > used in ra_serf to hold sending a bunch of requests before the NTLM
> > authentication handshake is finished.
> >
> > We need this callback in this scenario: consider an apache setup with
> > NTLM authentication MaxRequestsPerChild set to 100. Now use svn to
> > checkout a directory with more than 50 files. Serf will make 100
> > requests (PROPFIND+GET) for all those files and sends them on one of the
> > connections. What happens is that the before the NTLM handshake is
> > finished, the connection will already max out on the number of requests.
..
> >
> I spend some time reviewing your patches and trying to understand how to
> fix this problem. I didn't come to final decision, but I've some thoughts.
>
> Sending first request, complete authentication and then send other
> request isn't bullet proof solution. Just imagine if first request
> isn't require authentication at all, but following ones want it?

The authentication step is only required once per connection, whether
that is from the first request or later, it shouldn't make a
difference. If a 401 is only received after 10 requests, than the next
request added to the pipeline will contain the NTLM  auth phrase. No
other requests are sent until a response is received from the server.
If there were 50 requests still on the pipeline than those will all
get a 401 response and retried after the authentication has finished.
I'm not sure if the patch currently works like this, I'll test it.

> Also sending 100 pipelined requests isn't good idea for me. I think we have to
> implement limit for maximum number of concurrent pipelined requests as other
> clients do. Mozilla Firefox has limit to 4 pipelined requests for example.

Why do you don't think it ain't a good idea? What is the benefit of
introducing this limit? It's not like pipelining requests are adding
extra load to the server, they're just sent more efficiently.

I found this blog post concerning the decision to add pipeling support
to firefox:
http://weblogs.mozillazine.org/asa/archives/2003/11/http11_pipelining.html
It says: '3) We limit the number of requests in a pipeline to minimize
the effects of head-of-line blocking. Mozilla uses a default value of
4, but any value up to the hard-coded limit of 8 is possible.'

Now I think the svn client and Firefox have a different goal here:
Firefox parallelizes requests as much as possible (within certain
limits), because there's a value in downloading each individual image
as fast as possible. In svn we only care for the final result.

> Actually MaxRequestsPerChild limits number of connections per child
> [1]. Number of requests per connection controlled by
> MaxKeepAliveRequests [2].
>
> [1] http://httpd.apache.org/docs/2.0/mod/mpm_common.html#maxrequestsperchild
> [2] http://httpd.apache.org/docs/2.0/mod/core.html#maxkeepaliverequests

serf currently already tries to estimate the MaxKeepAliveRequests
based on the number of received responses before a connection is reset
by the server, to limit the number of requests on the pipeline.

> > Note2: I propose you add 'svn:eol-style' 'native' to the serf source
> > files, makes it a bit easier to patch them on Windows.
> >
> PS: I've changed svn:eol-style to native in all serf sources files.

I noticed, thanks.

Lieven

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [serf-dev] Re: [PATCH] serf/ra_serf: add cansend callback to stop writing requests untill authn handshake is finished

Posted by Greg Stein <gs...@gmail.com>.
Understood that its broken, and that's what irks me... adjusting the  
API to compensate.

*ponder*

On Aug 30, 2007, at 4:17 PM, "Justin Erenkrantz"  
<ju...@erenkrantz.com> wrote:

> On 8/30/07, Greg Stein <gs...@gmail.com> wrote:
>> Note that HTTP specifies auth is per request, not per connection.
>> Holding up a pipeline based on the faulty concept of conn auth is an
>> incorrect design.
>
> Well, yes, but SSPI is fundamentally at odds with HTTP as it does
> authentication at the connection level.  =(
>
>> I could very much agree with a design that incorporates a dynamic
>> scaling of the pipeline fill. i.e. start with a few and quickly ramp
>> to 100 or whatever we believe the max request per child is set to.
>> This still assumes auth is the same across requests, but that is an
>> optimization rather than designing an API that assumes a conn level
>> auth.
>
> Perhaps scaling would be good, but I wonder how that'd effect us in
> the common case where SSPI isn't being used?
>
> At least with Lieven's patches, only SSPI users would stall the
> pipeline - and that's because the underlying protocol is brain-dead.
> Other folks wouldn't have that issue.
>
> *shrug*  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [serf-dev] Re: [PATCH] serf/ra_serf: add cansend callback to stop writing requests untill authn handshake is finished

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On 8/30/07, Greg Stein <gs...@gmail.com> wrote:
> Note that HTTP specifies auth is per request, not per connection.
> Holding up a pipeline based on the faulty concept of conn auth is an
> incorrect design.

Well, yes, but SSPI is fundamentally at odds with HTTP as it does
authentication at the connection level.  =(

> I could very much agree with a design that incorporates a dynamic
> scaling of the pipeline fill. i.e. start with a few and quickly ramp
> to 100 or whatever we believe the max request per child is set to.
> This still assumes auth is the same across requests, but that is an
> optimization rather than designing an API that assumes a conn level
> auth.

Perhaps scaling would be good, but I wonder how that'd effect us in
the common case where SSPI isn't being used?

At least with Lieven's patches, only SSPI users would stall the
pipeline - and that's because the underlying protocol is brain-dead.
Other folks wouldn't have that issue.

*shrug*  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [serf-dev] Re: [PATCH] serf/ra_serf: add cansend callback to stop writing requests untill authn handshake is finished

Posted by Greg Stein <gs...@gmail.com>.
Note that HTTP specifies auth is per request, not per connection.  
Holding up a pipeline based on the faulty concept of conn auth is an  
incorrect design.

I could very much agree with a design that incorporates a dynamic  
scaling of the pipeline fill. i.e. start with a few and quickly ramp  
to 100 or whatever we believe the max request per child is set to.  
This still assumes auth is the same across requests, but that is an  
optimization rather than designing an API that assumes a conn level  
auth.

Cheers,
-g

On Aug 30, 2007, at 11:21 AM, "Ivan Zhakov" <ch...@gmail.com> wrote:

> On 8/28/07, Lieven Govaerts <li...@gmail.com> wrote:
>>
>> Ivan,
>>
>> thanks for the review. Inline some comments.
>>
>> On 8/28/07, Ivan Zhakov <ch...@gmail.com> wrote:
>>>
>>> On 7/25/07, Lieven Govaerts <lg...@mobsol.be> wrote:
>>>> Attached two patches implement a new callback 'cansend' in serf  
>>>> which is
>>>> used in ra_serf to hold sending a bunch of requests before the NTLM
>>>> authentication handshake is finished.
>>>>
>>>> We need this callback in this scenario: consider an apache setup  
>>>> with
>>>> NTLM authentication MaxRequestsPerChild set to 100. Now use svn to
>>>> checkout a directory with more than 50 files. Serf will make 100
>>>> requests (PROPFIND+GET) for all those files and sends them on one  
>>>> of the
>>>> connections. What happens is that the before the NTLM handshake is
>>>> finished, the connection will already max out on the number of  
>>>> requests.
>> ..
>>>>
>>> I spend some time reviewing your patches and trying to understand  
>>> how to
>>> fix this problem. I didn't come to final decision, but I've some  
>>> thoughts.
>>>
>>> Sending first request, complete authentication and then send other
>>> request isn't bullet proof solution. Just imagine if first request
>>> isn't require authentication at all, but following ones want it?
>>
>> The authentication step is only required once per connection, whether
>> that is from the first request or later, it shouldn't make a
>> difference. If a 401 is only received after 10 requests, than the  
>> next
>> request added to the pipeline will contain the NTLM  auth phrase. No
>> other requests are sent until a response is received from the server.
>> If there were 50 requests still on the pipeline than those will all
>> get a 401 response and retried after the authentication has finished.
>> I'm not sure if the patch currently works like this, I'll test it.
> Ok, I agree with you that serf needs some way to stop sending new
> requests while authenticating.
> Now I have some questions concerning implementation: how serf will now
> when to recover sending requests? How we can sure that authenticating
> request will not be blocked by this callback?
>
>>
>>> Also sending 100 pipelined requests isn't good idea for me. I  
>>> think we have to
>>> implement limit for maximum number of concurrent pipelined  
>>> requests as other
>>> clients do. Mozilla Firefox has limit to 4 pipelined requests for  
>>> example.
>>
>> Why do you don't think it ain't a good idea? What is the benefit of
>> introducing this limit? It's not like pipelining requests are adding
>> extra load to the server, they're just sent more efficiently.
>>
>> I found this blog post concerning the decision to add pipeling  
>> support
>> to firefox:
>> http://weblogs.mozillazine.org/asa/archives/2003/11/http11_pipelining.html
>> It says: '3) We limit the number of requests in a pipeline to  
>> minimize
>> the effects of head-of-line blocking. Mozilla uses a default value of
>> 4, but any value up to the hard-coded limit of 8 is possible.'
>>
>> Now I think the svn client and Firefox have a different goal here:
>> Firefox parallelizes requests as much as possible (within certain
>> limits), because there's a value in downloading each individual image
>> as fast as possible. In svn we only care for the final result.
>>
> Because we send 100 request and get 401 for all of them, only after
> that we will start authenticating, right? It's better to don't send
> more than 16 requests at once.
>
>>> Actually MaxRequestsPerChild limits number of connections per child
>>> [1]. Number of requests per connection controlled by
>>> MaxKeepAliveRequests [2].
>>>
>>> [1] http://httpd.apache.org/docs/2.0/mod/mpm_common.html#maxrequestsperchild
>>> [2] http://httpd.apache.org/docs/2.0/mod/core.html#maxkeepaliverequests
>>
>> serf currently already tries to estimate the MaxKeepAliveRequests
>> based on the number of received responses before a connection is  
>> reset
>> by the server, to limit the number of requests on the pipeline.
> Thanks, I didn't know that.
>
> -- 
> Ivan Zhakov
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [serf-dev] Re: [PATCH] serf/ra_serf: add cansend callback to stop writing requests untill authn handshake is finished

Posted by Ivan Zhakov <ch...@gmail.com>.
On 8/28/07, Lieven Govaerts <li...@gmail.com> wrote:
>
> Ivan,
>
> thanks for the review. Inline some comments.
>
> On 8/28/07, Ivan Zhakov <ch...@gmail.com> wrote:
> >
> > On 7/25/07, Lieven Govaerts <lg...@mobsol.be> wrote:
> > > Attached two patches implement a new callback 'cansend' in serf which is
> > > used in ra_serf to hold sending a bunch of requests before the NTLM
> > > authentication handshake is finished.
> > >
> > > We need this callback in this scenario: consider an apache setup with
> > > NTLM authentication MaxRequestsPerChild set to 100. Now use svn to
> > > checkout a directory with more than 50 files. Serf will make 100
> > > requests (PROPFIND+GET) for all those files and sends them on one of the
> > > connections. What happens is that the before the NTLM handshake is
> > > finished, the connection will already max out on the number of requests.
> ..
> > >
> > I spend some time reviewing your patches and trying to understand how to
> > fix this problem. I didn't come to final decision, but I've some thoughts.
> >
> > Sending first request, complete authentication and then send other
> > request isn't bullet proof solution. Just imagine if first request
> > isn't require authentication at all, but following ones want it?
>
> The authentication step is only required once per connection, whether
> that is from the first request or later, it shouldn't make a
> difference. If a 401 is only received after 10 requests, than the next
> request added to the pipeline will contain the NTLM  auth phrase. No
> other requests are sent until a response is received from the server.
> If there were 50 requests still on the pipeline than those will all
> get a 401 response and retried after the authentication has finished.
> I'm not sure if the patch currently works like this, I'll test it.
Ok, I agree with you that serf needs some way to stop sending new
requests while authenticating.
Now I have some questions concerning implementation: how serf will now
when to recover sending requests? How we can sure that authenticating
request will not be blocked by this callback?

>
> > Also sending 100 pipelined requests isn't good idea for me. I think we have to
> > implement limit for maximum number of concurrent pipelined requests as other
> > clients do. Mozilla Firefox has limit to 4 pipelined requests for example.
>
> Why do you don't think it ain't a good idea? What is the benefit of
> introducing this limit? It's not like pipelining requests are adding
> extra load to the server, they're just sent more efficiently.
>
> I found this blog post concerning the decision to add pipeling support
> to firefox:
> http://weblogs.mozillazine.org/asa/archives/2003/11/http11_pipelining.html
> It says: '3) We limit the number of requests in a pipeline to minimize
> the effects of head-of-line blocking. Mozilla uses a default value of
> 4, but any value up to the hard-coded limit of 8 is possible.'
>
> Now I think the svn client and Firefox have a different goal here:
> Firefox parallelizes requests as much as possible (within certain
> limits), because there's a value in downloading each individual image
> as fast as possible. In svn we only care for the final result.
>
Because we send 100 request and get 401 for all of them, only after
that we will start authenticating, right? It's better to don't send
more than 16 requests at once.

> > Actually MaxRequestsPerChild limits number of connections per child
> > [1]. Number of requests per connection controlled by
> > MaxKeepAliveRequests [2].
> >
> > [1] http://httpd.apache.org/docs/2.0/mod/mpm_common.html#maxrequestsperchild
> > [2] http://httpd.apache.org/docs/2.0/mod/core.html#maxkeepaliverequests
>
> serf currently already tries to estimate the MaxKeepAliveRequests
> based on the number of received responses before a connection is reset
> by the server, to limit the number of requests on the pipeline.
Thanks, I didn't know that.

-- 
Ivan Zhakov

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [serf-dev] Re: [PATCH] serf/ra_serf: add cansend callback to stop writing requests untill authn handshake is finished

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On 8/28/07, Lieven Govaerts <li...@gmail.com> wrote:
> serf currently already tries to estimate the MaxKeepAliveRequests
> based on the number of received responses before a connection is reset
> by the server, to limit the number of requests on the pipeline.

Yes.

So, I certainly don't support artificially restricting the pipeline
depth to 4 or 8 - especially when the current default httpd config is
100.  ra_serf is used in a very particular case that doesn't apply to
Firefox: it's talking to only Subversion servers - and in 99% of the
cases, that means there's an Apache sitting on the other end.  Firefox
certainly can't make that same claim - they have to work with all
sorts of random servers that may not support pipelining at all - so,
ra_serf can/should be optimized for our common case: there's an Apache
httpd sitting on the other end.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org