On 02/19/2009 12:32 PM, Joe Orton wrote:
> On Wed, Feb 18, 2009 at 09:39:31PM +0100, Ruediger Pluem wrote:
>> On 02/18/2009 11:16 AM, Joe Orton wrote:
>> There is still a nasty issue with the trunk code that can cause you to
>> run out of FD's as the new non blocking core output filter has some trouble
>> setting aside the file buckets to the correct pool (it puts them in the
>> connection pool which is too long living and bad).
>>
>> See thread starting at
>>
>> http://mail-archives.apache.org/mod_mbox/httpd-dev/200901.mbox/%3c495ff1b9.9070...@kippdata.de%3e
>>
>> I admit that I lost momentum on this, but this definitely needs to be fixed.
> 
> Yes, that looks broken.
> 
> Something like the patch below should fix it, though I'm not sure it's 
> really a good idea to be allowing a pathological case of 16 fds consumed 
> per client.  It might present some interesting tuning challenges; 
> hitting fd limits before hitting num(client) limits.

Thanks for the patch and sorry for picking on the details :-)

> 
> Index: core_filters.c
> ===================================================================
> --- core_filters.c    (revision 734690)
> +++ core_filters.c    (working copy)
> @@ -367,6 +367,7 @@
>  
>  #define THRESHOLD_MIN_WRITE 4096
>  #define THRESHOLD_MAX_BUFFER 65536
> +#define THRESHOLD_MAX_FILES 16
>  
>  /* Optional function coming from mod_logio, used for logging of output
>   * traffic
> @@ -380,7 +381,7 @@
>      core_output_filter_ctx_t *ctx = net->out_ctx;
>      apr_bucket_brigade *bb;
>      apr_bucket *bucket, *next;
> -    apr_size_t bytes_in_brigade, non_file_bytes_in_brigade;
> +    apr_size_t bytes_in_brigade, non_file_bytes_in_brigade, 
> num_files_in_brigade;
>  
>      /* Fail quickly if the connection has already been aborted. */
>      if (c->aborted) {
> @@ -466,6 +467,8 @@
>  
>      bytes_in_brigade = 0;
>      non_file_bytes_in_brigade = 0;
> +    num_files_in_brigade = 0;
> +
>      for (bucket = APR_BRIGADE_FIRST(bb); bucket != APR_BRIGADE_SENTINEL(bb);
>           bucket = next) {
>          next = APR_BUCKET_NEXT(bucket);
> @@ -497,13 +500,17 @@
>                  next = APR_BUCKET_NEXT(bucket);
>              }
>              bytes_in_brigade += bucket->length;
> -            if (!APR_BUCKET_IS_FILE(bucket)) {
> +            if (APR_BUCKET_IS_FILE(bucket)) {
> +                num_files_in_brigade++;

Hm, how do you know that the FD in this bucket is different from the one
in the last file bucket(s). It may be possible that we have file buckets
in the brigade that represent different parts of the same file as a filter
might have put something in between tehm in the form of heap buckets or 
something
like that.
Furthermore what about MMAP buckets? Don't they have an open FD as well?

Regards

Rüdiger

Reply via email to