> >>-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.
That is why we allow people to control how many requests can be sent on the same connection. Or, you can just have a limit on the number of file descriptors that you are willing to buffer. And, the pipe_read function should be smart enough that if we don't get any data off of the pipe, for say 30 seconds, then we flush whatever data we currently have. > >>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. You are removing a perfectly valid optimization to stop us from sending a lot of small packets across pipelined responses. Ryan
