httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject [PATCH] move CachemaxStreamingBuffer from mod_cache to mod_mem_cache
Date Tue, 17 Dec 2002 20:35:34 GMT
This patch does the following:

1. Renames CacheMaxStreamingBuffer to MCacheMaxStreamingBuffer

2. Moves implementation of MCacheMaxStreamingBuffer from mod_cache to
mod_mem_cache

3. Implements default value of MCacheMaxStreamingBuffer (100000 bytes) which is
adjusted downward if the default exceeds the setting of MCacheMaxCacheObjectSize
(I was not too keen on setting default MCacheMaxStreamingBuffer to be equal to
MCacheMaxCacheObjectSize for reasons pointed out by Brian Pane).

The patch is filtered to ignore changes in spacing (cvs diff -u -b). I still
have a bit of cleanup to do in the cache size check area (I am not convinced the
original code worked all that well) and I'm planning on fixing that tomorrow.

Index: cache_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_cache.c,v
retrieving revision 1.2
diff -u -b -r1.2 cache_cache.c
--- cache_cache.c	18 Aug 2002 03:23:06 -0000	1.2
+++ cache_cache.c	17 Dec 2002 21:03:40 -0000
@@ -101,6 +101,7 @@
             ((c->current_size + c->size_entry(entry)) > c->max_size)) {

         ejected = cache_pq_pop(c->pq);
+        /* What if ejected is NULL? */
         priority = c->get_pri(ejected);

         if (c->queue_clock < priority)
Index: mod_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.c,v
retrieving revision 1.71
diff -u -b -r1.71 mod_cache.c
--- mod_cache.c	17 Dec 2002 19:12:38 -0000	1.71
+++ mod_cache.c	17 Dec 2002 21:03:40 -0000
@@ -421,13 +421,13 @@
     cache_request_rec *cache;
     cache_server_conf *conf;
     char *url = r->unparsed_uri;
-    const char *cc_out;
+    const char *cc_out, *cl;
     const char *exps, *lastmods, *dates, *etag;
     apr_time_t exp, date, lastmod, now;
     apr_off_t size;
     cache_info *info;
-
-    apr_bucket *split_point = NULL;
+    char *reason;
+    apr_pool_t *p;

     /* check first whether running this filter has any point or not */
     if(r->no_cache) {
@@ -445,17 +445,8 @@
         ap_set_module_config(r->request_config, &cache_module, cache);
     }

-    /* If we've previously processed and set aside part of this
-     * response, skip the cacheability checks
-     */
-    if (cache->saved_brigade != NULL) {
-        exp = cache->exp;
-        lastmod = cache->lastmod;
-        info = cache->info;
-    }
-    else {
-        char *reason = NULL;
-        apr_pool_t *p = r->pool;
+    reason = NULL;
+    p = r->pool;
         /*
          * Pass Data to Cache
          * ------------------
@@ -611,22 +602,14 @@
             return ap_pass_brigade(f->next, in);
         }
         cache->in_checked = 1;
-    } /* if cache->saved_brigade==NULL */

-    /* Set the content length if known.  We almost certainly do NOT want to
-     * cache streams with unknown content lengths in the in-memory cache.
-     * Streams with unknown content length should be first cached in the
-     * file system. If they are withing acceptable limits, then they can be
-     * moved to the in-memory cache.
+    /* Set the content length if known.
      */
-    {
-        const char* cl;
         cl = apr_table_get(r->headers_out, "Content-Length");
         if (cl) {
             size = apr_atoi64(cl);
         }
         else {
-
             /* if we don't get the content-length, see if we have all the
              * buckets and use their length to calculate the size
              */
@@ -648,108 +631,8 @@
                 }
                 size += e->length;
             }
-
             if (!all_buckets_here) {
-                /* Attempt to set aside a copy of a partial response
-                 * in hopes of caching it once the rest of the response
-                 * is available.  There are special cases in which we
-                 * don't try to set aside the content, though:
-                 *   1. The brigade contains at least one bucket of
-                 *      unknown length, such as a pipe or socket bucket.
-                 *   2. The size of the response exceeds the limit set
-                 *      by the CacheMaxStreamingBuffer  directive.
-                 */
-                if (unresolved_length ||
-                    (cache->saved_size + size >
-                     conf->max_streaming_buffer_size)) {
-
-                    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
-                                 "cache: not caching streamed response for "
-                                 "%s because length %s", url,
-                                 (unresolved_length ?
-                                  "cannot be determined" :
-                                  "> CacheMaxStreamingBuffer"));
-
-                    if (cache->saved_brigade != NULL) {
-                        apr_brigade_destroy(cache->saved_brigade);
-                        cache->saved_brigade = NULL;
-                        cache->saved_size = 0;
-                    }
-                    ap_remove_output_filter(f);
-                    return ap_pass_brigade(f->next, in);
-                }
-
-                /* Add a copy of the new brigade's buckets to the
-                 * saved brigade.  The reason for the copy is so
-                 * that we can output the new buckets immediately,
-                 * rather than having to buffer up the entire
-                 * response before sending anything.
-                 */
-                if (cache->saved_brigade == NULL) {
-                    cache->saved_brigade =
-                        apr_brigade_create(r->pool,
-                                           r->connection->bucket_alloc);
-                    cache->exp = exp;
-                    cache->lastmod = lastmod;
-                    cache->info = info;
-                }
-                APR_BRIGADE_FOREACH(e, in) {
-                    apr_bucket *copy;
-                    rv = apr_bucket_copy(e, &copy);
-                    if (rv == APR_ENOTIMPL) {
-                        const char *str;
-                        apr_size_t len;
-
-                        /* This takes care of uncopyable buckets. */
-                        rv = apr_bucket_read(e, &str, &len, APR_BLOCK_READ);
-                        if ((rv == APR_SUCCESS) &&
-                            (cache->saved_size + len <=
-                                        conf->max_streaming_buffer_size)) {
-                            rv = apr_bucket_copy(e, &copy);
-                        }
-
-                        if ((rv != APR_SUCCESS) ||
-                            (cache->saved_size + len >
-                                        conf->max_streaming_buffer_size)){
-                            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
-                                         "cache: not caching streamed response
for "
-                                         "%s because length %s", url,
-                                          "> CacheMaxStreamingBuffer");
-
-                            if (cache->saved_brigade != NULL) {
-                                apr_brigade_destroy(cache->saved_brigade);
-                                cache->saved_brigade = NULL;
-                                cache->saved_size = 0;
-                            }
-                            ap_remove_output_filter(f);
-                            return ap_pass_brigade(f->next, in);
-                        }
-                    }
-                    APR_BRIGADE_INSERT_TAIL(cache->saved_brigade, copy);
-                }
-                cache->saved_size += size;
-                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
-                             "cache: Response length still unknown, setting "
-                             "aside content for url: %s", url);
-
-                return ap_pass_brigade(f->next, in);
-            }
-            else {
-                /* Now that we've seen an EOS, it's appropriate
-                 * to try caching the response.  If any content
-                 * has been copied into cache->saved_brigade in
-                 * previous passes through this filter, the
-                 * content placed in the cache must be the
-                 * concatenation of the saved brigade and the
-                 * current brigade.
-                 */
-                if (cache->saved_brigade != NULL) {
-                    split_point = APR_BRIGADE_FIRST(in);
-                    APR_BRIGADE_CONCAT(cache->saved_brigade, in);
-                    in = cache->saved_brigade;
-                    size += cache->saved_size;
-                }
-            }
+            size = -1;
         }
     }

@@ -791,11 +674,6 @@
     if (rv != OK) {
         /* Caching layer declined the opportunity to cache the response */
         ap_remove_output_filter(f);
-        if (split_point) {
-            apr_bucket_brigade *already_sent = in;
-            in = apr_brigade_split(in, split_point);
-            apr_brigade_destroy(already_sent);
-        }
         return ap_pass_brigade(f->next, in);
     }

@@ -896,11 +774,7 @@
     if (rv != APR_SUCCESS) {
         ap_remove_output_filter(f);
     }
-    if (split_point) {
-        apr_bucket_brigade *already_sent = in;
-        in = apr_brigade_split(in, split_point);
-        apr_brigade_destroy(already_sent);
-    }
+
     return ap_pass_brigade(f->next, in);
 }

@@ -931,7 +805,6 @@
     ps->no_last_mod_ignore = 0;
     ps->ignorecachecontrol = 0;
     ps->ignorecachecontrol_set = 0 ;
-    ps->max_streaming_buffer_size = 0;
     return ps;
 }

@@ -968,11 +841,6 @@
         (overrides->ignorecachecontrol_set == 0)
         ? base->ignorecachecontrol
         : overrides->ignorecachecontrol;
-    ps->max_streaming_buffer_size  =
-        (overrides->max_streaming_buffer_size == 0)
-        ? base->max_streaming_buffer_size
-        : overrides->max_streaming_buffer_size;
-
     return ps;
 }
 static const char *set_cache_ignore_no_last_mod(cmd_parms *parms, void *dummy,
@@ -1092,24 +960,6 @@
     return NULL;
 }

-static const char *set_max_streaming_buffer(cmd_parms *parms, void *dummy,
-                                            const char *arg)
-{
-    cache_server_conf *conf;
-    apr_off_t val;
-    char *err;
-
-    conf =
-        (cache_server_conf *)ap_get_module_config(parms->server->module_config,
-                                                  &cache_module);
-    val = (apr_off_t)strtol(arg, &err, 10);
-    if (*err != 0) {
-        return "CacheMaxStreamingBuffer value must be a number";
-    }
-    conf->max_streaming_buffer_size = val;
-    return NULL;
-}
-
 static int cache_post_config(apr_pool_t *p, apr_pool_t *plog,
                              apr_pool_t *ptemp, server_rec *s)
 {
@@ -1156,10 +1006,6 @@
     AP_INIT_TAKE1("CacheForceCompletion", set_cache_complete, NULL, RSRC_CONF,
                   "Percentage of download to arrive for the cache to force "
                   "complete transfer"),
-    AP_INIT_TAKE1("CacheMaxStreamingBuffer", set_max_streaming_buffer, NULL,
-                  RSRC_CONF,
-                  "Maximum number of bytes of content to buffer for "
-                  "a streamed response"),
     {NULL}
 };

Index: mod_cache.h
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.h,v
retrieving revision 1.36
diff -u -b -r1.36 mod_cache.h
--- mod_cache.h	17 Nov 2002 01:33:25 -0000	1.36
+++ mod_cache.h	17 Dec 2002 21:03:40 -0000
@@ -179,9 +179,6 @@
     /** ignore client's requests for uncached responses */
     int ignorecachecontrol;
     int ignorecachecontrol_set;
-    /* maximum amount of data to buffer on a streamed response where
-     * we haven't yet seen EOS */
-    apr_off_t max_streaming_buffer_size;
 } cache_server_conf;

 /* cache info information */
Index: mod_mem_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_mem_cache.c,v
retrieving revision 1.88
diff -u -b -r1.88 mod_mem_cache.c
--- mod_mem_cache.c	17 Nov 2002 01:33:25 -0000	1.88
+++ mod_mem_cache.c	17 Dec 2002 21:03:40 -0000
@@ -121,6 +121,9 @@
     apr_size_t max_object_cnt;
     cache_pqueue_set_priority cache_remove_algorithm;

+    /* maximum amount of data to buffer on a streamed response where
+     * we haven't yet seen EOS */
+    apr_off_t max_streaming_buffer_size;
 } mem_cache_conf;
 static mem_cache_conf *sconf;

@@ -128,6 +131,7 @@
 #define DEFAULT_MIN_CACHE_OBJECT_SIZE 0
 #define DEFAULT_MAX_CACHE_OBJECT_SIZE 10000
 #define DEFAULT_MAX_OBJECT_CNT 1009
+#define DEFAULT_MAX_STREAMING_BUFFER_SIZE 100000
 #define CACHEFILE_LEN 20

 /* Forward declarations */
@@ -425,6 +429,7 @@
     sconf->cache_size = 0;
     sconf->cache_cache = NULL;
     sconf->cache_remove_algorithm = memcache_gdsf_algorithm;
+    sconf->max_streaming_buffer_size = DEFAULT_MAX_STREAMING_BUFFER_SIZE;

     return sconf;
 }
@@ -449,12 +454,22 @@
         return DECLINED;
     }

+    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
+         * we know exactly know how much we are caching.
+         */
+        len = sconf->max_streaming_buffer_size;
+    }
+
     /* In principle, we should be able to dispense with the cache_size checks
      * when caching open file descriptors.  However, code in cache_insert() and
      * other places does not make the distinction whether a file's content or
      * descriptor is being cached. For now, just do all the same size checks
      * regardless of what we are caching.
      */
+
     if (len < sconf->min_cache_object_size ||
         len > sconf->max_cache_object_size) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
@@ -463,6 +478,7 @@
                      key);
         return DECLINED;
     }
+
     if (type_e == CACHE_TYPE_FILE) {
         /* CACHE_TYPE_FILE is only valid for local content handled by the
          * default handler. Need a better way to check if the file is
@@ -488,7 +504,6 @@
     /* Safe cast: We tested < sconf->max_cache_object_size above */
     obj->info.len = (apr_size_t)len;

-
     /* Allocate and init mem_cache_object_t */
     mobj = calloc(1, sizeof(*mobj));
     if (!mobj) {
@@ -947,6 +962,7 @@

         /* Content not suitable for fd caching. Cache in-memory instead. */
         mobj->type = CACHE_TYPE_HEAP;
+#if 0
         /* Check to make sure the object will not exceed configured thresholds
*/
         if (mobj->m_len < sconf->min_cache_object_size ||
             mobj->m_len > sconf->max_cache_object_size) {
@@ -956,6 +972,7 @@
             return APR_ENOMEM; /* ?? DECLINED; */
         }
         sconf->cache_size += mobj->m_len;
+#endif
     }

     /*
@@ -977,6 +994,34 @@
         apr_size_t len;

         if (APR_BUCKET_IS_EOS(e)) {
+            if (mobj->m_len > obj->count) {
+                /* Caching a streamed response. Reallocate a buffer of the
+                 * correct size and copy the streamed response into that
+                 * buffer */
+                char *b = malloc(obj->count);
+                if (!b) {
+                    return APR_ENOMEM;
+                }
+                memcpy(b, mobj->m, obj->count);
+                free(mobj->m);
+                mobj->m = b;
+
+                /* Now comes the crufty part... there is no way to tell the
+                 * cache that the size of the object has changed. We need
+                 * to remove the old object, update the size and re-add the
+                 * object, all under protection of the lock.
+                 */
+                if (sconf->lock) {
+                    apr_thread_mutex_lock(sconf->lock);
+                }
+                cache_remove(sconf->cache_cache, obj);
+                mobj->m_len = obj->count;
+                cache_insert(sconf->cache_cache, obj);
+                if (sconf->lock) {
+                    apr_thread_mutex_unlock(sconf->lock);
+                }
+                sconf->cache_size -= (mobj->m_len - obj->count);
+            }
             /* Open for business */
             obj->complete = 1;
             break;
@@ -1022,6 +1067,23 @@
                      "MCacheSize must be greater than MCacheMaxObjectSize");
         return DONE;
     }
+    if (sconf->max_streaming_buffer_size > sconf->max_cache_object_size) {
+        /* Issue a notice only if something other than the default config
+         * is being used */
+        if (sconf->max_streaming_buffer_size !=
DEFAULT_MAX_STREAMING_BUFFER_SIZE &&
+            sconf->max_cache_object_size != DEFAULT_MAX_CACHE_OBJECT_SIZE) {
+            ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s,
+                         "MCacheMaxStreamingBuffer must be less than or equal
to MCacheMaxObjectSize. "
+                         "Resetting MCacheMaxStreamingBuffer to
MCacheMaxObjectSize.");
+        }
+        sconf->max_streaming_buffer_size = sconf->max_cache_object_size;
+    }
+    if (sconf->max_streaming_buffer_size < sconf->min_cache_object_size) {
+        ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
+                     "MCacheMaxStreamingBuffer must be less than or equal to
MCacheMaxObjectSize. "
+                     "Resetting MCacheMaxStreamingBuffer to
MCacheMinObjectSize.");
+        sconf->max_streaming_buffer_size = sconf->min_cache_object_size;
+    }
     ap_mpm_query(AP_MPMQ_IS_THREADED, &threaded_mpm);
     if (threaded_mpm) {
         apr_thread_mutex_create(&sconf->lock, APR_THREAD_MUTEX_DEFAULT, p);
@@ -1108,6 +1170,19 @@
     return NULL;
 }

+static const char *set_max_streaming_buffer(cmd_parms *parms, void *dummy,
+                                            const char *arg)
+{
+    apr_off_t val;
+    char *err;
+    val = (apr_off_t)strtol(arg, &err, 10);
+    if (*err != 0) {
+        return "MCacheMaxStreamingBuffer value must be a number";
+    }
+    sconf->max_streaming_buffer_size = val;
+    return NULL;
+}
+
 static const command_rec cache_cmds[] =
 {
     AP_INIT_TAKE1("MCacheSize", set_max_cache_size, NULL, RSRC_CONF,
@@ -1120,6 +1195,8 @@
      "The maximum size (in bytes) of an object to be placed in the cache"),
     AP_INIT_TAKE1("MCacheRemovalAlgorithm", set_cache_removal_algorithm, NULL,
RSRC_CONF,
      "The algorithm used to remove entries from the cache (default: GDSF)"),
+    AP_INIT_TAKE1("MCacheMaxStreamingBuffer", set_max_streaming_buffer, NULL,
RSRC_CONF,
+     "Maximum number of bytes of content to buffer for a streamed response"),
     {NULL}
 };


Mime
View raw message