httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: svn commit: r1021546 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml include/ap_mmn.h modules/cache/cache_util.h modules/cache/mod_cache.c modules/cache/mod_cache.h modules/cache/mod_disk_cache.c
Date Tue, 12 Oct 2010 09:30:11 GMT
On 12 Oct 2010, at 8:16 AM, Ruediger Pluem wrote:

>> Modified: httpd/httpd/trunk/modules/cache/mod_disk_cache.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_disk_cache.c?rev=1021546&r1=1021545&r2=1021546&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/trunk/modules/cache/mod_disk_cache.c (original)
>> +++ httpd/httpd/trunk/modules/cache/mod_disk_cache.c Mon Oct 11  
>> 23:32:56 2010
>> @@ -860,16 +860,14 @@ static apr_status_t recall_headers(cache
>>
>> static apr_status_t recall_body(cache_handle_t *h, apr_pool_t *p,  
>> apr_bucket_brigade *bb)
>> {
>> -    apr_bucket *e;
>>     disk_cache_object_t *dobj = (disk_cache_object_t*) h->cache_obj- 
>> >vobj;
>>
>>     if (dobj->data.fd) {
>> -        apr_brigade_insert_file(bb, dobj->data.fd, 0, dobj- 
>> >file_size, p);
>> +        apr_bucket *e = apr_bucket_file_create(dobj->data.fd, 0,
>> +                dobj->file_size, p, bb->bucket_alloc);
>> +        APR_BRIGADE_INSERT_HEAD(bb, e);
>>     }
>>
>> -    e = apr_bucket_eos_create(bb->bucket_alloc);
>> -    APR_BRIGADE_INSERT_TAIL(bb, e);
>> -
>>     return APR_SUCCESS;
>> }
>
>
> What is the purpose of these changes? What makes the need for  
> apr_brigade_insert_file
> go away and why don't we add an eos any longer?

The changes to cache_out_filter() didn't come out cleanly in the  
patch, it would have made it clearer if it had.

In the past, cache_out_filter() made the bogus assumption that it  
would always receive an empty brigade from cache_[quick_]handler(),  
and that it was always safe to just add the file, followed by an eos  
to the end of this empty brigade.

Now, in the failure case, we already have buckets in the brigade  
representing the error, which we need to replace with our stale cached  
file, so our assumption that the brigade is empty is no longer valid.

What we do now, is have the cache_out_filter() delete existing buckets  
properly until it finds eos, and then asks recall_body() to prepend  
the body to the front of the brigade, instead of appending it to at  
the end and creating it's own eos, as before.

Because an eos bucket is already present in the output brigade, it is  
no longer valid to add a second eos bucket inside recall_body(), and  
for this reason, it was removed.

Looking deeper at the code for apr_brigade_insert_file(), it looks  
like it does more than just add a single file bucket, but rather adds  
multiple smaller file buckets if the file exceeds a certain size. What  
we may need to do is switch back to using apr_brigade_insert_file(),  
but then prepend the brigade returned to the brigade destined for the  
client.

Regards,
Graham
--


Mime
View raw message