sworda / serf

Automatically exported from code.google.com/p/serf
Apache License 2.0
0 stars 0 forks source link

PLEASE DO ENABLE RFC896(Nagle) #148

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Any projects that adds extra http header using serf would suffer from that.
Unless you have extra room for improvement of headers_buckets.c so that it does 
serialize http request headers, You shouldn't disable RFC896.

Original issue reported on code.google.com by jojel...@gmail.com on 4 Jul 2014 at 9:43

Attachments:

GoogleCodeExporter commented 9 years ago
Hi,

enabling Nagle has a negative performance impact, so we don't want to do that.

Can you give some more info on the issue you're seeing? Which version of serf 
are you using?

I gather from your description that you're seeing that HTTP requests are split 
over multiple TCP packets? Or is this HTTPS related? Both cases are optimised 
to send requests in the minimum amount of TCP packets/SSL records as possible.

This is how it works in serf trunk (and in any recent version of serf):
- in write_to_connection: read_iovec on the stream bucket (which is an 
aggregate)
- the first element in the stream bucket is a request bucket, call read_iovec 
on that
- serf_request_read_iovec serialises the request line and adds it and the 
headers bucket to a new aggregate bucket
- it will then call read_iovec on the headers bucket, which iterates over all 
headers and adds them one by one to the iovec array.
- the iovec array now contains the whole request.
- serf passes this iovec array to the socket_writev call which will full up the 
TCP buffer with as much data as it can.

I'm interested to know where you see room for improvement.

Lieven

Original comment by lieven.govaerts@gmail.com on 5 Jul 2014 at 6:28

GoogleCodeExporter commented 9 years ago
svn --version

* ra_serf : Module for accessing a repository via WebDAV protocol using serf.
  - using serf 1.3.5
  - handles 'http' scheme
  - handles 'https' scheme

In cygwin, execute following command.

svnrdump dump --incremental http://serf.googlecode.com/svn/trunk/ 
serf-read-only >/dev/null

Then you can use wireshark to observe that http header is being sent to 
destination which is not aggregated(one) tcp segment but line by line whenever 
serf_bucket_headers_setn is called. It's okay if it is fixed in trunk.

But i'd prefer enable nagle algorithm, the resolution of serf against negative 
performance impact when enabled nagle doesn't work as expected. even if it is 
expected working as you mentioned.

Original comment by jojel...@gmail.com on 5 Jul 2014 at 12:20

GoogleCodeExporter commented 9 years ago
Above command doesn't execute. Here is command instead of above
svnrdump dump --incremental http://serf.googlecode.com/svn/trunk/ >/dev/null

Original comment by jojel...@gmail.com on 5 Jul 2014 at 12:24

GoogleCodeExporter commented 9 years ago
I don't see this problem with the native windows code.

I'm guessing you are hitting a limitation in the emulation layer of Cygwin. (Or 
with the Cygwin packages you are using).

Googling for 'writev' on Cygwin shows a few users asking how to perform 
writev() with Cygwin... My guess is they just didn't implemented it in the 
Cygwin layer, while the platform has no problem supporting it.

Original comment by b...@qqmail.nl on 5 Jul 2014 at 1:35

GoogleCodeExporter commented 9 years ago
I've tried this on Mac OS X Mavericks and don't see a problem, all requests are 
sent in one TCP packet instead of (what I suppose you're seeing) being split 
over multiple packets.

This might be a problem in Cygwin as Bert suggests, I don't know. 

What I do know is that enabling Nagle will have a negative impact on serf. The 
reason is that: 
a) serf will write a full request in one writev step so it provides enough 
bytes to the socket layer to warrant sending a packet,
b) serf uses HTTP pipelining, so if the application provides many requests to 
serf and serf will send them one by one without waiting for a HTTP response. In 
this case there's no reason why serf should wait one or multiple roundtrip 
times for an ACK before sending the next TCP packet.

Original comment by lieven.govaerts@gmail.com on 5 Jul 2014 at 1:58

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
apr_socket_writev just calls apr_socket_write for n times if writev is not 
implemented.
And cygwin hasn't implemented writev.
I'll follow-up further discussion to cygwin mailing list. thank you all for 
kind explanation.

Original comment by jojel...@gmail.com on 6 Jul 2014 at 5:27

GoogleCodeExporter commented 9 years ago
It's Cygwin/APR issue. Closing ticket as Won't fix.

Original comment by chemodax@gmail.com on 7 Jul 2014 at 9:00