On 19 Nov 2011, at 3:56 PM, f_los_ch wrote:
Thanks for the reply and your patch - it worked!
I could not longer reproduce diffs for cached/uncached files. The log
with dumped buckets according to my previous patch is again available
at: http://paste2.org/p/1785342
Now, when the location is spotted, maybe I can also try to isolate the
bug even more via further debugging for developing some production-use
patch. In the hope that really the bug was in there and the continuous
flushing did not masquerade the underlying issue..?
But that it is working for now is a real relief.
Thanks for confirming it works. Would it be possible to try this
alternative patch? Looking at the brigade inside h, it doesn't seem to
be doing anything useful in this case, and was a hangup from older
code. This new patch takes the intermediate brigade out completely,
and should in theory use less memory and be slightly faster: What I
need to do is find out why we haven't discovered this before.
Index: modules/cache/mod_cache_disk.c
===================================================================
--- modules/cache/mod_cache_disk.c (revision 1203646)
+++ modules/cache/mod_cache_disk.c (working copy)
@@ -1075,9 +1075,6 @@
disk_cache_dir_conf *dconf = ap_get_module_config(r-
>per_dir_config, &cache_disk_module);
int seen_eos = 0;
- if (!dobj->bb) {
- dobj->bb = apr_brigade_create(r->pool, r->connection-
>bucket_alloc);
- }
if (!dobj->offset) {
dobj->offset = dconf->readsize;
}
@@ -1107,7 +1104,6 @@
seen_eos = 1;
dobj->done = 1;
APR_BUCKET_REMOVE(e);
- APR_BRIGADE_CONCAT(out, dobj->bb);
APR_BRIGADE_INSERT_TAIL(out, e);
break;
}
@@ -1115,7 +1111,6 @@
/* honour flush buckets, we'll get called again */
if (APR_BUCKET_IS_FLUSH(e)) {
APR_BUCKET_REMOVE(e);
- APR_BRIGADE_CONCAT(out, dobj->bb);
APR_BRIGADE_INSERT_TAIL(out, e);
break;
}
@@ -1123,21 +1118,20 @@
/* metadata buckets are preserved as is */
if (APR_BUCKET_IS_METADATA(e)) {
APR_BUCKET_REMOVE(e);
- APR_BRIGADE_INSERT_TAIL(dobj->bb, e);
+ APR_BRIGADE_INSERT_TAIL(out, e);
continue;
}
/* read the bucket, write to the cache */
rv = apr_bucket_read(e, &str, &length, APR_BLOCK_READ);
APR_BUCKET_REMOVE(e);
- APR_BRIGADE_INSERT_TAIL(dobj->bb, e);
+ APR_BRIGADE_INSERT_TAIL(out, e);
if (rv != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"cache_disk: Error when reading bucket for URL
%s",
h->cache_obj->key);
/* Remove the intermediate cache file and return non-
APR_SUCCESS */
apr_pool_destroy(dobj->data.pool);
- APR_BRIGADE_CONCAT(out, dobj->bb);
return rv;
}
@@ -1156,7 +1150,6 @@
APR_BUFFERED | APR_EXCL, dobj-
>data.pool);
if (rv != APR_SUCCESS) {
apr_pool_destroy(dobj->data.pool);
- APR_BRIGADE_CONCAT(out, dobj->bb);
return rv;
}
dobj->file_size = 0;
@@ -1164,7 +1157,6 @@
dobj->data.tempfd);
if (rv != APR_SUCCESS) {
apr_pool_destroy(dobj->data.pool);
- APR_BRIGADE_CONCAT(out, dobj->bb);
return rv;
}
dobj->disk_info.device = finfo.device;
@@ -1180,7 +1172,6 @@
h->cache_obj->key);
/* Remove the intermediate cache file and return non-
APR_SUCCESS */
apr_pool_destroy(dobj->data.pool);
- APR_BRIGADE_CONCAT(out, dobj->bb);
return rv;
}
dobj->file_size += written;
@@ -1191,7 +1182,6 @@
h->cache_obj->key, dobj->file_size, dconf->maxfs);
/* Remove the intermediate cache file and return non-
APR_SUCCESS */
apr_pool_destroy(dobj->data.pool);
- APR_BRIGADE_CONCAT(out, dobj->bb);
return APR_EGENERAL;
}
@@ -1203,12 +1193,10 @@
dobj->offset -= length;
if (dobj->offset <= 0) {
dobj->offset = 0;
- APR_BRIGADE_CONCAT(out, dobj->bb);
break;
}
if ((dconf->readtime && apr_time_now() > dobj->timeout)) {
dobj->timeout = 0;
- APR_BRIGADE_CONCAT(out, dobj->bb);
break;
}
Index: modules/cache/mod_cache_disk.h
===================================================================
--- modules/cache/mod_cache_disk.h (revision 1203646)
+++ modules/cache/mod_cache_disk.h (working copy)
@@ -49,7 +49,6 @@
const char *key; /* On-disk prefix; URI with Vary
bits (if present) */
apr_off_t file_size; /* File size of the cached data
file */
disk_cache_info_t disk_info; /* Header information. */
- apr_bucket_brigade *bb; /* Set aside brigade */
apr_table_t *headers_in; /* Input headers to save */
apr_table_t *headers_out; /* Output headers to save */
apr_off_t offset; /* Max size to set aside */
Regards,
Graham
--
