Ryan Bloom wrote:
>>-1 on buffering across requests, because the performance problems
>>caused by the extra mmap+munmap will offset the gain you're trying
>>to achieve with pipelining.
>>
>>
>
>Wait a second. Now you want to stop buffering to fix a completely
>differeny bug. The idea that we can't keep a file_bucket in the brigade
>across requests is only partially true. Take a closer look at what we
>are doing and why when we convert the file to an mmap. That logic was
>added so that we do not leak file descriptors across requests.
>
>However, there are multiple options to fix that. The first, and easiest
>one is to just have the core_output_filter call apr_file_close() after
>it has sent the file. The second is to migrate the apr_file_t to the
>conn_rec's pool if and only if the file needs to survive the request's
>pool being killed. Because we are only migrating file descriptors in
>the edge case, this shouldn't cause a big enough leak to cause a
>problem.
>
Migrating the apr_file_t to the conn_rec's pool is an appealing
solution, but it's quite dangerous. With that logic added to the
current 8KB threshold, it would be too easy to make an httpd run
out of file descriptors: Send a pipeline of a few hundred requests
for some tiny file (e.g., a small image). They all get setaside
into the conn_rec's pool. Then send a request for something
that takes a long time to process, like a CGI. Run multiple
copies of this against the target httpd at once, and you'll be
able to exhaust the file descriptors for a threaded httpd all
too easily.
>
>
>
>>>You are trying to solve a problem that doesn't exist in the real
>>>
>>>
>world
>
>
>>>IIUIC. Think it through. The problem is that if the page is <1k and
>>>you request that page 8 times on the same connection, then Apache
>>>
>>>
>will
>
>
>>>buffer all of the responses. How often are those conditions going to
>>>happen in the real world?
>>>
>>>
>>>
>>That's not the problem that I care about. The problem that matters
>>is the one that happens in the real world, as a side-effect of the
>>core_output_filter() code trying to be too clever:
>> - Client opens a keepalive connection to httpd
>> - Cclient requests a file smaller than 8KB
>> - core_output_filter(), upon seeing the file bucket followed by EOS,
>> decides to buffer the output because it has less than 8KB total.
>> - There isn't another request ready to be read (read returns EAGAIN)
>> because the client isn't pipelining its connections. So we then
>> do the writev of the file that we've just finished buffering.
>>
>>
>
>But, this case only happens if the headers + the file are less than 8k.
>If the file is >10k, then this problem doesn't actually exist at all.
>As I said above, there are better ways to fix this than removing all
>ability to pipeline responses.
>
We're not removing the ability to pipeline responses.
>>Aside from slowing things down for the user, this hurts the
>>
>>
>scalability
>
>
>>of the httpd (mmap and munmap don't scale well on multiprocessor
>>
>>
>boxes).
>
>
>>What we should be doing in this case is just doing the write
>>
>>
>immediately
>
>
>>upon seeing the EOS, rather than penalizing both the client and the
>>server.
>>
>>
>
>By doing that, you are removing ANY benefit to using pipelined requests
>when serving files. Multiple research projects have all found that
>pipelined requests show a performance benefit. In other words, you are
>fixing a performance problem by removing another performance enhancer.
>
Sorry, but that's completely incorrect. Even if you do the write
for each request's response immediately, you'll still have most
of the benefits of pipelining:
* Lower total packet count, because the TCP connection
handshaking doesn't need to happen on a per-connection
basis
* No network round-trip latency for each request (no extra
TCP connection setup or shutdown per request, and the
data packets, even though they're small, can be sent
immediately because we turn off Nagle's algorithm)
--Brian
