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
--

Reply via email to