You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Justin Erenkrantz <ju...@erenkrantz.com> on 2004/08/02 19:55:40 UTC
[PATCH] mod_cache fixes: #9
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard <bi...@wstoddard.com>
wrote:
> Too many changes in one patch. Break this up into multiple consumable in 15
> minute patches and I'll review them.
Here's another patch that was hidden in my earlier one. I think 'read' and
'write' are awful terms in the cache context. -- justin
---
Avoid confusion when reading mod_cache code. write_ and read_ often imply
network code; save_ and load_ are more understandable prefixes in this context.
Index: modules/experimental/mod_cache.h
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.h,v
retrieving revision 1.46
diff -u -r1.46 mod_cache.h
--- modules/experimental/mod_cache.h 2 Aug 2004 17:27:08 -0000 1.46
+++ modules/experimental/mod_cache.h 2 Aug 2004 17:54:43 -0000
@@ -178,10 +178,10 @@
struct cache_handle {
cache_object_t *cache_obj;
int (*remove_entity) (cache_handle_t *h);
- apr_status_t (*write_headers)(cache_handle_t *h, request_rec *r,
cache_info *i);
- apr_status_t (*write_body)(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b);
- apr_status_t (*read_headers) (cache_handle_t *h, request_rec *r);
- apr_status_t (*read_body) (cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb);
+ apr_status_t (*save_headers)(cache_handle_t *h, request_rec *r,
cache_info *i);
+ apr_status_t (*save_body)(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b);
+ apr_status_t (*load_headers) (cache_handle_t *h, request_rec *r);
+ apr_status_t (*load_body) (cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb);
apr_table_t *req_hdrs; /* These are the original request headers */
};
@@ -242,11 +242,11 @@
*/
const char* cache_create_key( request_rec*r );
-apr_status_t cache_write_entity_headers(cache_handle_t *h, request_rec *r,
cache_info *info);
-apr_status_t cache_write_entity_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *bb);
+apr_status_t cache_save_entity_headers(cache_handle_t *h, request_rec *r,
cache_info *info);
+apr_status_t cache_save_entity_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *bb);
-apr_status_t cache_read_entity_headers(cache_handle_t *h, request_rec *r);
-apr_status_t cache_read_entity_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb);
+apr_status_t cache_load_entity_headers(cache_handle_t *h, request_rec *r);
+apr_status_t cache_load_entity_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb);
/* hooks */
Index: modules/experimental/mod_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.c,v
retrieving revision 1.86
diff -u -r1.86 mod_cache.c
--- modules/experimental/mod_cache.c 2 Aug 2004 17:39:09 -0000 1.86
+++ modules/experimental/mod_cache.c 2 Aug 2004 17:54:43 -0000
@@ -260,8 +260,8 @@
ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server,
"cache: running CACHE_OUT filter");
- /* cache_read_entity_headers() was called in cache_select_url() */
- cache_read_entity_body(cache->handle, r->pool, bb);
+ /* cache_load_entity_headers() was called in cache_select_url() */
+ cache_load_entity_body(cache->handle, r->pool, bb);
/* This filter is done once it has served up its content */
ap_remove_output_filter(f);
@@ -369,7 +369,7 @@
/* pass the brigades into the cache, then pass them
* up the filter stack
*/
- rv = cache_write_entity_body(cache->handle, r, in);
+ rv = cache_save_entity_body(cache->handle, r, in);
if (rv != APR_SUCCESS) {
ap_remove_output_filter(f);
}
@@ -708,9 +708,9 @@
/*
* Write away header information to cache.
*/
- rv = cache_write_entity_headers(cache->handle, r, info);
+ rv = cache_save_entity_headers(cache->handle, r, info);
if (rv == APR_SUCCESS) {
- rv = cache_write_entity_body(cache->handle, r, in);
+ rv = cache_save_entity_body(cache->handle, r, in);
}
if (rv != APR_SUCCESS) {
ap_remove_output_filter(f);
Index: modules/experimental/mod_disk_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_disk_cache.c,v
retrieving revision 1.53
diff -u -r1.53 mod_disk_cache.c
--- modules/experimental/mod_disk_cache.c 2 Aug 2004 17:32:31 -0000 1.53
+++ modules/experimental/mod_disk_cache.c 2 Aug 2004 17:54:43 -0000
@@ -78,10 +78,10 @@
/* Forward declarations */
static int remove_entity(cache_handle_t *h);
-static apr_status_t write_headers(cache_handle_t *h, request_rec *r,
cache_info *i);
-static apr_status_t write_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b);
-static apr_status_t read_headers(cache_handle_t *h, request_rec *r);
-static apr_status_t read_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb);
+static apr_status_t save_headers(cache_handle_t *h, request_rec *r,
cache_info *i);
+static apr_status_t save_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b);
+static apr_status_t load_headers(cache_handle_t *h, request_rec *r);
+static apr_status_t load_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb);
/*
* Local static functions
@@ -186,7 +186,7 @@
* file for an ap_cache_el, this state information will be read
* and written transparent to clients of this module
*/
-static int file_cache_read_mydata(apr_file_t *fd, cache_info *info,
+static int file_cache_load_mydata(apr_file_t *fd, cache_info *info,
disk_cache_object_t *dobj)
{
apr_status_t rv;
@@ -243,7 +243,7 @@
return APR_SUCCESS;
}
-static int file_cache_write_mydata(apr_file_t *fd , cache_handle_t *h,
request_rec *r)
+static int file_cache_save_mydata(apr_file_t *fd , cache_handle_t *h,
request_rec *r)
{
apr_status_t rc;
char *buf;
@@ -337,10 +337,10 @@
if (rv == APR_SUCCESS) {
/* Populate the cache handle */
h->cache_obj = obj;
- h->read_body = &read_body;
- h->read_headers = &read_headers;
- h->write_body = &write_body;
- h->write_headers = &write_headers;
+ h->load_body = &load_body;
+ h->load_headers = &load_headers;
+ h->save_body = &save_body;
+ h->save_headers = &save_headers;
h->remove_entity = &remove_entity;
ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
@@ -424,17 +424,17 @@
}
/* Read the bytes to setup the cache_info fields */
- rc = file_cache_read_mydata(hfd, info, dobj);
+ rc = file_cache_load_mydata(hfd, info, dobj);
if (rc != APR_SUCCESS) {
/* XXX log message */
return DECLINED;
}
/* Initialize the cache_handle callback functions */
- h->read_body = &read_body;
- h->read_headers = &read_headers;
- h->write_body = &write_body;
- h->write_headers = &write_headers;
+ h->load_body = &load_body;
+ h->load_headers = &load_headers;
+ h->save_body = &save_body;
+ h->save_headers = &save_headers;
h->remove_entity = &remove_entity;
ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
@@ -456,7 +456,7 @@
* @@@: XXX: FIXME: currently the headers are passed thru un-merged.
* Is that okay, or should they be collapsed where possible?
*/
-static apr_status_t read_headers(cache_handle_t *h, request_rec *r)
+static apr_status_t load_headers(cache_handle_t *h, request_rec *r)
{
apr_status_t rv;
char urlbuff[1034];
@@ -517,7 +517,7 @@
return APR_SUCCESS;
}
-static apr_status_t read_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb)
+static apr_status_t load_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;
@@ -531,7 +531,7 @@
return APR_SUCCESS;
}
-static apr_status_t write_headers(cache_handle_t *h, request_rec *r,
cache_info *info)
+static apr_status_t save_headers(cache_handle_t *h, request_rec *r,
cache_info *info)
{
disk_cache_conf *conf = ap_get_module_config(r->server->module_config,
&disk_cache_module);
@@ -572,7 +572,7 @@
hfd = dobj->hfd;
dobj->name = h->cache_obj->key;
- file_cache_write_mydata(dobj->hfd, h, r);
+ file_cache_save_mydata(dobj->hfd, h, r);
if (r->headers_out) {
int i;
@@ -639,7 +639,7 @@
return APR_SUCCESS;
}
-static apr_status_t write_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b)
+static apr_status_t save_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b)
{
apr_bucket *e;
apr_status_t rv;
Index: modules/experimental/mod_mem_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_mem_cache.c,v
retrieving revision 1.110
diff -u -r1.110 mod_mem_cache.c
--- modules/experimental/mod_mem_cache.c 2 Aug 2004 17:18:15 -0000 1.110
+++ modules/experimental/mod_mem_cache.c 2 Aug 2004 17:54:43 -0000
@@ -89,10 +89,10 @@
/* Forward declarations */
static int remove_entity(cache_handle_t *h);
-static apr_status_t write_headers(cache_handle_t *h, request_rec *r,
cache_info *i);
-static apr_status_t write_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b);
-static apr_status_t read_headers(cache_handle_t *h, request_rec *r);
-static apr_status_t read_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb);
+static apr_status_t save_headers(cache_handle_t *h, request_rec *r,
cache_info *i);
+static apr_status_t save_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b);
+static apr_status_t load_headers(cache_handle_t *h, request_rec *r);
+static apr_status_t load_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb);
static void cleanup_cache_object(cache_object_t *obj);
@@ -372,7 +372,7 @@
if (len == -1) {
/* Caching a streaming response. Assume the response is
* less than or equal to max_streaming_buffer_size. We will
- * correct all the cache size counters in write_body once
+ * correct all the cache size counters in save_body once
* we know exactly know how much we are caching.
*/
len = sconf->max_streaming_buffer_size;
@@ -471,10 +471,10 @@
/* Populate the cache handle */
h->cache_obj = obj;
- h->read_body = &read_body;
- h->read_headers = &read_headers;
- h->write_body = &write_body;
- h->write_headers = &write_headers;
+ h->load_body = &load_body;
+ h->load_headers = &load_headers;
+ h->save_body = &save_body;
+ h->save_headers = &save_headers;
h->remove_entity = &remove_entity;
return OK;
@@ -526,13 +526,13 @@
}
/* Initialize the cache_handle */
- h->read_body = &read_body;
- h->read_headers = &read_headers;
- h->write_body = &write_body;
- h->write_headers = &write_headers;
+ h->load_body = &load_body;
+ h->load_headers = &load_headers;
+ h->save_body = &save_body;
+ h->save_headers = &save_headers;
h->remove_entity = &remove_entity;
h->cache_obj = obj;
- h->req_hdrs = NULL; /* Pick these up in read_headers() */
+ h->req_hdrs = NULL; /* Pick these up in load_headers() */
return OK;
}
@@ -665,7 +665,7 @@
return OK;
}
-static apr_status_t read_headers(cache_handle_t *h, request_rec *r)
+static apr_status_t load_headers(cache_handle_t *h, request_rec *r)
{
int rc;
mem_cache_object_t *mobj = (mem_cache_object_t*) h->cache_obj->vobj;
@@ -700,7 +700,7 @@
return rc;
}
-static apr_status_t read_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb)
+static apr_status_t load_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *bb)
{
apr_bucket *b;
mem_cache_object_t *mobj = (mem_cache_object_t*) h->cache_obj->vobj;
@@ -723,7 +723,7 @@
}
-static apr_status_t write_headers(cache_handle_t *h, request_rec *r,
cache_info *info)
+static apr_status_t save_headers(cache_handle_t *h, request_rec *r,
cache_info *info)
{
cache_object_t *obj = h->cache_obj;
mem_cache_object_t *mobj = (mem_cache_object_t*) obj->vobj;
@@ -820,7 +820,7 @@
return APR_SUCCESS;
}
-static apr_status_t write_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b)
+static apr_status_t save_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b)
{
apr_status_t rv;
cache_object_t *obj = h->cache_obj;
Index: modules/experimental/cache_storage.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_storage.c,v
retrieving revision 1.34
diff -u -r1.34 cache_storage.c
--- modules/experimental/cache_storage.c 9 Feb 2004 20:29:18 -0000 1.34
+++ modules/experimental/cache_storage.c 2 Aug 2004 17:54:43 -0000
@@ -65,7 +65,8 @@
* decide whether or not it wants to cache this particular entity.
* If the size is unknown, a size of -1 should be set.
*/
-int cache_create_entity(request_rec *r, const char *types, char *url,
apr_off_t size)
+int cache_create_entity(request_rec *r, const char *types, char *url,
+ apr_off_t size)
{
cache_handle_t *h = apr_pcalloc(r->pool, sizeof(cache_handle_t));
const char *next = types;
@@ -145,18 +146,19 @@
case OK: {
char *vary = NULL;
const char *varyhdr = NULL;
- if (cache_read_entity_headers(h, r) != APR_SUCCESS) {
+ if (cache_load_entity_headers(h, r) != APR_SUCCESS) {
/* TODO: Handle this error */
return DECLINED;
}
/*
* Check Content-Negotiation - Vary
- *
- * At this point we need to make sure that the object we found in
the cache
- * is the same object that would be delivered to the client, when
the
- * effects of content negotiation are taken into effect.
- *
+ *
+ * At this point we need to make sure that the object we found in
+ * the cache is the same object that would be delivered to the
+ * client, when the effects of content negotiation are taken into
+ * effect.
+ *
* In plain english, we want to make sure that a
language-negotiated
* document in one language is not given to a client asking for a
* language negotiated document in a different language by
mistake.
@@ -198,7 +200,8 @@
else {
/* headers do not match, so Vary failed */
ap_log_error(APLOG_MARK, APLOG_INFO, APR_SUCCESS,
r->server,
- "cache_select_url(): Vary header mismatch -
Cached document cannot be used. \n");
+ "cache_select_url(): Vary header mismatch: "
+ "Cached document cannot be used. \n");
apr_table_clear(r->headers_out);
r->status_line = NULL;
cache->handle = NULL;
@@ -222,38 +225,43 @@
return DECLINED;
}
-apr_status_t cache_write_entity_headers(cache_handle_t *h,
+apr_status_t cache_save_entity_headers(cache_handle_t *h,
request_rec *r,
cache_info *info)
{
- return (h->write_headers(h, r, info));
+ return (h->save_headers(h, r, info));
}
-apr_status_t cache_write_entity_body(cache_handle_t *h, request_rec *r,
apr_bucket_brigade *b)
+
+apr_status_t cache_save_entity_body(cache_handle_t *h, request_rec *r,
+ apr_bucket_brigade *b)
{
- return (h->write_body(h, r, b));
+ return (h->save_body(h, r, b));
}
-apr_status_t cache_read_entity_headers(cache_handle_t *h, request_rec *r)
+apr_status_t cache_load_entity_headers(cache_handle_t *h, request_rec *r)
{
apr_status_t rv;
cache_info *info = &(h->cache_obj->info);
/* Build the header table from info in the info struct */
- rv = h->read_headers(h, r);
+ rv = h->load_headers(h, r);
if (rv != APR_SUCCESS) {
return rv;
}
- r->filename = apr_pstrdup(r->pool, info->filename );
+ r->filename = apr_pstrdup(r->pool, info->filename);
return APR_SUCCESS;
}
-apr_status_t cache_read_entity_body(cache_handle_t *h, apr_pool_t *p,
apr_bucket_brigade *b)
+
+apr_status_t cache_load_entity_body(cache_handle_t *h, apr_pool_t *p,
+ apr_bucket_brigade *b)
{
- return (h->read_body(h, p, b));
+ return (h->load_body(h, p, b));
}
-apr_status_t cache_generate_key_default( request_rec *r, apr_pool_t*p,
char**key )
+apr_status_t cache_generate_key_default(request_rec *r, apr_pool_t *p,
+ char **key)
{
if (r->hostname) {
*key = apr_pstrcat(p, r->hostname, r->uri, "?", r->args, NULL);
@@ -264,15 +272,17 @@
return APR_SUCCESS;
}
-APR_IMPLEMENT_EXTERNAL_HOOK_RUN_FIRST(cache, CACHE, int, create_entity,
- (cache_handle_t *h, request_rec *r,
const char *type,
- const char *urlkey, apr_off_t len),
+APR_IMPLEMENT_EXTERNAL_HOOK_RUN_FIRST(cache, CACHE, int, create_entity,
+ (cache_handle_t *h, request_rec *r,
+ const char *type, const char *urlkey,
+ apr_off_t len),
(h, r, type,urlkey,len),DECLINED)
-APR_IMPLEMENT_EXTERNAL_HOOK_RUN_FIRST(cache, CACHE, int, open_entity,
- (cache_handle_t *h, request_rec *r,
const char *type,
- const char *urlkey),(h,r,type,urlkey),
+APR_IMPLEMENT_EXTERNAL_HOOK_RUN_FIRST(cache, CACHE, int, open_entity,
+ (cache_handle_t *h, request_rec *r,
+ const char *type, const char *urlkey),
+ (h,r,type,urlkey),
DECLINED)
-APR_IMPLEMENT_EXTERNAL_HOOK_RUN_ALL(cache, CACHE, int, remove_url,
+APR_IMPLEMENT_EXTERNAL_HOOK_RUN_ALL(cache, CACHE, int, remove_url,
(const char *type, const char *urlkey),
(type,urlkey),OK,DECLINED)
Re: [PATCH] mod_cache fixes: #9
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Monday, August 2, 2004 10:55 AM -0700 Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard
> <bi...@wstoddard.com> wrote:
>
>> Too many changes in one patch. Break this up into multiple consumable in 15
>> minute patches and I'll review them.
>
> Here's another patch that was hidden in my earlier one. I think 'read' and
> 'write' are awful terms in the cache context. -- justin
Some formatting tweaks in cache_storage.c somehow made it in to that message.
Please ignore those for right now. ;-) -- justin
Re: [PATCH] mod_cache fixes: #9
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 2 Aug 2004, Bill Stoddard wrote:
> > How 'bout 2.2 GA for ApacheCon? Seems reasonable to me. ;-) -- justin
>
> +1, sounds like a good target to shoot for.
+1
Re: [PATCH] mod_cache fixes: #9
Posted by Bill Stoddard <bi...@wstoddard.com>.
Justin Erenkrantz wrote:
> --On Monday, August 2, 2004 8:18 PM -0400 Cliff Woolley
> <jw...@virginia.edu> wrote:
>
>> Yeah, it's really good to see the interest in this picking back up. This
>> seems like a really good way to get us motivated to do a 2.2 release
>> Sometime Soon. :)
>
>
> How 'bout 2.2 GA for ApacheCon? Seems reasonable to me. ;-) -- justin
>
+1, sounds like a good target to shoot for.
Bill
Re: [PATCH] mod_cache fixes: #9
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Monday, August 2, 2004 8:18 PM -0400 Cliff Woolley
<jw...@virginia.edu> wrote:
> Yeah, it's really good to see the interest in this picking back up. This
> seems like a really good way to get us motivated to do a 2.2 release
> Sometime Soon. :)
How 'bout 2.2 GA for ApacheCon? Seems reasonable to me. ;-) -- justin
Re: [PATCH] mod_cache fixes: #9
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 2 Aug 2004, Roy T.Fielding wrote:
> Hmm, IIRC, "load"ing a cache means writing to it, not reading from it.
Doh. That does ring a bell in the back of my (usually lousy) memory. :)
> Why not just change them to "cache_write" and "cache_read"?
> Or "store" and "recall"?
store and recall seem reasonable to me.
> Kudos on the other changes -- those are some significant improvements.
Yeah, it's really good to see the interest in this picking back up. This
seems like a really good way to get us motivated to do a 2.2 release
Sometime Soon. :)
--Cliff
Re: [PATCH] mod_cache fixes: #9
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Monday, August 2, 2004 5:11 PM -0700 "Roy T. Fielding"
<fi...@gbiv.com> wrote:
> Hmm, IIRC, "load"ing a cache means writing to it, not reading from it.
> Why not just change them to "cache_write" and "cache_read"?
> Or "store" and "recall"?
store and recall work, too. *shrug* (I share rbb's inability to name
anything, but I do know that 'read_body' and 'write_body' are bad choices.)
> Kudos on the other changes -- those are some significant improvements.
Let's see how many more tricks we have up our sleeves. ;-)
Thanks. -- justin
Re: [PATCH] mod_cache fixes: #9
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Monday, August 2, 2004, at 10:55 AM, Justin Erenkrantz wrote:
> Avoid confusion when reading mod_cache code. write_ and read_ often
> imply
> network code; save_ and load_ are more understandable prefixes in this
> context.
Hmm, IIRC, "load"ing a cache means writing to it, not reading from it.
Why not just change them to "cache_write" and "cache_read"?
Or "store" and "recall"?
Kudos on the other changes -- those are some significant improvements.
....Roy