You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by st...@apache.org on 2002/12/18 16:07:22 UTC

cvs commit: httpd-2.0/modules/experimental cache_cache.c mod_cache.c mod_cache.h mod_mem_cache.c

stoddard    2002/12/18 07:07:21

  Modified:    .        CHANGES
               modules/experimental cache_cache.c mod_cache.c mod_cache.h
                        mod_mem_cache.c
  Log:
  Rename CacheMaxStreamingBuffer to MCacheMaxStreamingBuffer. Move
  implementation of MCacheMaxStreamingBuffer from mod_cache to
  mod_mem_cache. MCacheMaxStreamingBuffer now defaults to the
  lesser of 100,000 bytes or MCacheMaxCacheObjectSize. This should
  eliminate the need for explicitly coding MCacheMaxStreamingBuffer
  in most configurations. [Bill Stoddard]
  
  Revision  Changes    Path
  1.1014    +6 -0      httpd-2.0/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/CHANGES,v
  retrieving revision 1.1013
  retrieving revision 1.1014
  diff -u -r1.1013 -r1.1014
  --- CHANGES	17 Dec 2002 20:08:18 -0000	1.1013
  +++ CHANGES	18 Dec 2002 15:07:20 -0000	1.1014
  @@ -1,6 +1,12 @@
   Changes with Apache 2.1.0-dev
   
     [Remove entries to the current 2.0 section below, when backported]
  +  *) Rename CacheMaxStreamingBuffer to MCacheMaxStreamingBuffer. Move
  +     implementation of MCacheMaxStreamingBuffer from mod_cache to
  +     mod_mem_cache. MCacheMaxStreamingBuffer now defaults to the
  +     lesser of 100,000 bytes or MCacheMaxCacheObjectSize. This should 
  +     eliminate the need for explicitly coding MCacheMaxStreamingBuffer
  +     in most configurations. [Bill Stoddard]
   
     *) mod_cache: Fix PR 15113, a core dump in cache_in_filter when
        a redirect occurs. The code was passing a format string and
  
  
  
  1.3       +1 -0      httpd-2.0/modules/experimental/cache_cache.c
  
  Index: cache_cache.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_cache.c,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- cache_cache.c	18 Aug 2002 03:23:06 -0000	1.2
  +++ cache_cache.c	18 Dec 2002 15:07:21 -0000	1.3
  @@ -101,6 +101,7 @@
               ((c->current_size + c->size_entry(entry)) > c->max_size)) {
   
           ejected = cache_pq_pop(c->pq);
  +        /* FIX: If ejected is NULL, we'll segfault here */
           priority = c->get_pri(ejected);
   
           if (c->queue_clock < priority)
  
  
  
  1.72      +177 -331  httpd-2.0/modules/experimental/mod_cache.c
  
  Index: mod_cache.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.c,v
  retrieving revision 1.71
  retrieving revision 1.72
  diff -u -r1.71 -r1.72
  --- mod_cache.c	17 Dec 2002 19:12:38 -0000	1.71
  +++ mod_cache.c	18 Dec 2002 15:07:21 -0000	1.72
  @@ -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,311 +445,194 @@
           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
  +    reason = NULL;
  +    p = r->pool;
  +    /*
  +     * Pass Data to Cache
  +     * ------------------
  +     * This section passes the brigades into the cache modules, but only
  +     * if the setup section (see below) is complete.
        */
  -    if (cache->saved_brigade != NULL) {
  -        exp = cache->exp;
  -        lastmod = cache->lastmod;
  -        info = cache->info;
  -    }
  -    else {
  -        char *reason = NULL;
  -        apr_pool_t *p = r->pool;
  -        /*
  -         * Pass Data to Cache
  -         * ------------------
  -         * This section passes the brigades into the cache modules, but only
  -         * if the setup section (see below) is complete.
  -         */
  -
  -        /* have we already run the cachability check and set up the
  -         * cached file handle? 
  -         */
  -        if (cache->in_checked) {
  -            /* pass the brigades into the cache, then pass them
  -             * up the filter stack
  -             */
  -            rv = cache_write_entity_body(cache->handle, r, in);
  -            if (rv != APR_SUCCESS) {
  -                ap_remove_output_filter(f);
  -            }
  -            return ap_pass_brigade(f->next, in);
  -        }
   
  -        /*
  -         * Setup Data in Cache
  -         * -------------------
  -         * This section opens the cache entity and sets various caching
  -         * parameters, and decides whether this URL should be cached at
  -         * all. This section is* run before the above section.
  -         */
  -        info = apr_pcalloc(r->pool, sizeof(cache_info));
  -
  -        /* read expiry date; if a bad date, then leave it so the client can
  -         * read it 
  -         */
  -        exps = apr_table_get(r->headers_out, "Expires");
  -        if (exps != NULL) {
  -            if (APR_DATE_BAD == (exp = apr_date_parse_http(exps))) {
  -                exps = NULL;
  -            }
  -        }
  -        else {
  -            exp = APR_DATE_BAD;
  +    /* have we already run the cachability check and set up the
  +     * cached file handle? 
  +     */
  +    if (cache->in_checked) {
  +        /* pass the brigades into the cache, then pass them
  +         * up the filter stack
  +         */
  +        rv = cache_write_entity_body(cache->handle, r, in);
  +        if (rv != APR_SUCCESS) {
  +            ap_remove_output_filter(f);
           }
  +        return ap_pass_brigade(f->next, in);
  +    }
   
  -        /* read the last-modified date; if the date is bad, then delete it */
  -        lastmods = apr_table_get(r->headers_out, "Last-Modified");
  -        if (lastmods != NULL) {
  -            if (APR_DATE_BAD == (lastmod = apr_date_parse_http(lastmods))) {
  -                lastmods = NULL;
  -            }
  -        }
  -        else {
  -            lastmod = APR_DATE_BAD;
  -        }
  +    /*
  +     * Setup Data in Cache
  +     * -------------------
  +     * This section opens the cache entity and sets various caching
  +     * parameters, and decides whether this URL should be cached at
  +     * all. This section is* run before the above section.
  +     */
  +    info = apr_pcalloc(r->pool, sizeof(cache_info));
   
  -        conf = (cache_server_conf *) ap_get_module_config(r->server->module_config, &cache_module);
  -        /* read the etag and cache-control from the entity */
  -        etag = apr_table_get(r->headers_out, "Etag");
  -        cc_out = apr_table_get(r->headers_out, "Cache-Control");
  -
  -        /*
  -         * what responses should we not cache?
  -         *
  -         * At this point we decide based on the response headers whether it
  -         * is appropriate _NOT_ to cache the data from the server. There are
  -         * a whole lot of conditions that prevent us from caching this data.
  -         * They are tested here one by one to be clear and unambiguous. 
  -         */
  -        if (r->status != HTTP_OK && r->status != HTTP_NON_AUTHORITATIVE
  -            && r->status != HTTP_MULTIPLE_CHOICES
  -            && r->status != HTTP_MOVED_PERMANENTLY
  -            && r->status != HTTP_NOT_MODIFIED) {
  -            /* RFC2616 13.4 we are allowed to cache 200, 203, 206, 300, 301 or 410
  -             * We don't cache 206, because we don't (yet) cache partial responses.
  -             * We include 304 Not Modified here too as this is the origin server
  -             * telling us to serve the cached copy.
  -             */
  -              reason = apr_psprintf(p, "Response status %d", r->status);
  -        } 
  -        else if (exps != NULL && exp == APR_DATE_BAD) {
  -            /* if a broken Expires header is present, don't cache it */
  -            reason = apr_psprintf(p, "Broken expires header %s", exps);
  -        }
  -        else if (r->args && exps == NULL) {
  -            /* if query string present but no expiration time, don't cache it
  -             * (RFC 2616/13.9)
  -             */
  -            reason = "Query string present but no expires header";
  -        }
  -        else if (r->status == HTTP_NOT_MODIFIED && (NULL == cache->handle)) {
  -            /* if the server said 304 Not Modified but we have no cache
  -             * file - pass this untouched to the user agent, it's not for us.
  -             */
  -            reason = "HTTP Status 304 Not Modified";
  -        }
  -        else if (r->status == HTTP_OK && lastmods == NULL && etag == NULL 
  -                 && (conf->no_last_mod_ignore ==0)) {
  -            /* 200 OK response from HTTP/1.0 and up without a Last-Modified
  -             * header/Etag 
  -             */
  -            /* XXX mod-include clears last_modified/expires/etags - this
  -             * is why we have an optional function for a key-gen ;-) 
  -             */
  -            reason = "No Last-Modified or Etag header";
  -        }
  -        else if (r->header_only) {
  -            /* HEAD requests */
  -            reason = "HTTP HEAD request";
  -        }
  -        else if (ap_cache_liststr(NULL, cc_out, "no-store", NULL)) {
  -            /* RFC2616 14.9.2 Cache-Control: no-store response
  -             * indicating do not cache, or stop now if you are
  -             * trying to cache it */
  -            reason = "Cache-Control: no-store present";
  -        }
  -        else if (ap_cache_liststr(NULL, cc_out, "private", NULL)) {
  -            /* RFC2616 14.9.1 Cache-Control: private
  -             * this object is marked for this user's eyes only. Behave
  -             * as a tunnel.
  -             */
  -            reason = "Cache-Control: private present";
  -        }
  -        else if (apr_table_get(r->headers_in, "Authorization") != NULL
  -                 && !(ap_cache_liststr(NULL, cc_out, "s-maxage", NULL)
  -                      || ap_cache_liststr(NULL, cc_out, "must-revalidate", NULL)
  -                      || ap_cache_liststr(NULL, cc_out, "public", NULL))) {
  -            /* RFC2616 14.8 Authorisation:
  -             * if authorisation is included in the request, we don't cache,
  -             * but we can cache if the following exceptions are true:
  -             * 1) If Cache-Control: s-maxage is included
  -             * 2) If Cache-Control: must-revalidate is included
  -             * 3) If Cache-Control: public is included
  -             */
  -            reason = "Authorization required";
  -        }
  -        else if (r->no_cache) {
  -            /* or we've been asked not to cache it above */
  -            reason = "no_cache present";
  +    /* read expiry date; if a bad date, then leave it so the client can
  +     * read it 
  +     */
  +    exps = apr_table_get(r->headers_out, "Expires");
  +    if (exps != NULL) {
  +        if (APR_DATE_BAD == (exp = apr_date_parse_http(exps))) {
  +            exps = NULL;
           }
  +    }
  +    else {
  +        exp = APR_DATE_BAD;
  +    }
   
  -        if (reason) {
  -            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
  -                         "cache: %s not cached. Reason: %s", url, reason);
  -            /* remove this object from the cache 
  -             * BillS Asks.. Why do we need to make this call to remove_url?
  -             * leave it in for now..
  -             */
  -            cache_remove_url(r, cache->types, url);
  +    /* read the last-modified date; if the date is bad, then delete it */
  +    lastmods = apr_table_get(r->headers_out, "Last-Modified");
  +    if (lastmods != NULL) {
  +        if (APR_DATE_BAD == (lastmod = apr_date_parse_http(lastmods))) {
  +            lastmods = NULL;
  +        }
  +    }
  +    else {
  +        lastmod = APR_DATE_BAD;
  +    }
   
  -            /* remove this filter from the chain */
  -            ap_remove_output_filter(f);
  +    conf = (cache_server_conf *) ap_get_module_config(r->server->module_config, &cache_module);
  +    /* read the etag and cache-control from the entity */
  +    etag = apr_table_get(r->headers_out, "Etag");
  +    cc_out = apr_table_get(r->headers_out, "Cache-Control");
   
  -            /* ship the data up the stack */
  -            return ap_pass_brigade(f->next, in);
  -        }
  -        cache->in_checked = 1;
  -    } /* if cache->saved_brigade==NULL */
  +    /*
  +     * what responses should we not cache?
  +     *
  +     * At this point we decide based on the response headers whether it
  +     * is appropriate _NOT_ to cache the data from the server. There are
  +     * a whole lot of conditions that prevent us from caching this data.
  +     * They are tested here one by one to be clear and unambiguous. 
  +     */
  +    if (r->status != HTTP_OK && r->status != HTTP_NON_AUTHORITATIVE
  +        && r->status != HTTP_MULTIPLE_CHOICES
  +        && r->status != HTTP_MOVED_PERMANENTLY
  +        && r->status != HTTP_NOT_MODIFIED) {
  +        /* RFC2616 13.4 we are allowed to cache 200, 203, 206, 300, 301 or 410
  +         * We don't cache 206, because we don't (yet) cache partial responses.
  +         * We include 304 Not Modified here too as this is the origin server
  +         * telling us to serve the cached copy.
  +         */
  +        reason = apr_psprintf(p, "Response status %d", r->status);
  +    } 
  +    else if (exps != NULL && exp == APR_DATE_BAD) {
  +        /* if a broken Expires header is present, don't cache it */
  +        reason = apr_pstrcat(p, "Broken expires header: ", exps, NULL);
  +    }
  +    else if (r->args && exps == NULL) {
  +        /* if query string present but no expiration time, don't cache it
  +         * (RFC 2616/13.9)
  +         */
  +        reason = "Query string present but no expires header";
  +    }
  +    else if (r->status == HTTP_NOT_MODIFIED && (NULL == cache->handle)) {
  +        /* if the server said 304 Not Modified but we have no cache
  +         * file - pass this untouched to the user agent, it's not for us.
  +         */
  +        reason = "HTTP Status 304 Not Modified";
  +    }
  +    else if (r->status == HTTP_OK && lastmods == NULL && etag == NULL 
  +             && (conf->no_last_mod_ignore ==0)) {
  +        /* 200 OK response from HTTP/1.0 and up without a Last-Modified
  +         * header/Etag 
  +         */
  +        /* XXX mod-include clears last_modified/expires/etags - this
  +         * is why we have an optional function for a key-gen ;-) 
  +         */
  +        reason = "No Last-Modified or Etag header";
  +    }
  +    else if (r->header_only) {
  +        /* HEAD requests */
  +        reason = "HTTP HEAD request";
  +    }
  +    else if (ap_cache_liststr(NULL, cc_out, "no-store", NULL)) {
  +        /* RFC2616 14.9.2 Cache-Control: no-store response
  +         * indicating do not cache, or stop now if you are
  +         * trying to cache it */
  +        reason = "Cache-Control: no-store present";
  +    }
  +    else if (ap_cache_liststr(NULL, cc_out, "private", NULL)) {
  +        /* RFC2616 14.9.1 Cache-Control: private
  +         * this object is marked for this user's eyes only. Behave
  +         * as a tunnel.
  +         */
  +        reason = "Cache-Control: private present";
  +    }
  +    else if (apr_table_get(r->headers_in, "Authorization") != NULL
  +             && !(ap_cache_liststr(NULL, cc_out, "s-maxage", NULL)
  +                  || ap_cache_liststr(NULL, cc_out, "must-revalidate", NULL)
  +                  || ap_cache_liststr(NULL, cc_out, "public", NULL))) {
  +        /* RFC2616 14.8 Authorisation:
  +         * if authorisation is included in the request, we don't cache,
  +         * but we can cache if the following exceptions are true:
  +         * 1) If Cache-Control: s-maxage is included
  +         * 2) If Cache-Control: must-revalidate is included
  +         * 3) If Cache-Control: public is included
  +         */
  +        reason = "Authorization required";
  +    }
  +    else if (r->no_cache) {
  +        /* or we've been asked not to cache it above */
  +        reason = "no_cache present";
  +    }
   
  -    /* 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.
  -     */
  -    {
  -        const char* cl;
  -        cl = apr_table_get(r->headers_out, "Content-Length");
  -        if (cl) {
  -            size = apr_atoi64(cl);
  -        }
  -        else {
  +    if (reason) {
  +        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
  +                     "cache: %s not cached. Reason: %s", url, reason);
  +        /* remove this object from the cache 
  +         * BillS Asks.. Why do we need to make this call to remove_url?
  +         * leave it in for now..
  +         */
  +        cache_remove_url(r, cache->types, url);
   
  -            /* if we don't get the content-length, see if we have all the 
  -             * buckets and use their length to calculate the size 
  -             */
  -            apr_bucket *e;
  -            int all_buckets_here=0;
  -            int unresolved_length = 0;
  -            size=0;
  -            APR_BRIGADE_FOREACH(e, in) {
  -                if (APR_BUCKET_IS_EOS(e)) {
  -                    all_buckets_here=1;
  -                    break;
  -                }
  -                if (APR_BUCKET_IS_FLUSH(e)) {
  -                    unresolved_length = 1;
  -                    continue;
  -                }
  -                if (e->length == (apr_size_t)-1) {
  -                    break;
  -                }
  -                size += e->length;
  -            }
  +        /* remove this filter from the chain */
  +        ap_remove_output_filter(f);
   
  -            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);
  +        /* ship the data up the stack */
  +        return ap_pass_brigade(f->next, in);
  +    }
  +    cache->in_checked = 1;
   
  -                return ap_pass_brigade(f->next, in);
  +    /* Set the content length if known. 
  +     */
  +    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 
  +         */
  +        apr_bucket *e;
  +        int all_buckets_here=0;
  +        int unresolved_length = 0;
  +        size=0;
  +        APR_BRIGADE_FOREACH(e, in) {
  +            if (APR_BUCKET_IS_EOS(e)) {
  +                all_buckets_here=1;
  +                break;
  +            }
  +            if (APR_BUCKET_IS_FLUSH(e)) {
  +                unresolved_length = 1;
  +                continue;
               }
  -            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;
  -                }
  +            if (e->length == (apr_size_t)-1) {
  +                break;
               }
  +            size += e->length;
  +        }
  +        if (!all_buckets_here) {
  +            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}
   };
   
  
  
  
  1.37      +0 -3      httpd-2.0/modules/experimental/mod_cache.h
  
  Index: mod_cache.h
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.h,v
  retrieving revision 1.36
  retrieving revision 1.37
  diff -u -r1.36 -r1.37
  --- mod_cache.h	17 Nov 2002 01:33:25 -0000	1.36
  +++ mod_cache.h	18 Dec 2002 15:07:21 -0000	1.37
  @@ -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 */
  
  
  
  1.89      +80 -17    httpd-2.0/modules/experimental/mod_mem_cache.c
  
  Index: mod_mem_cache.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_mem_cache.c,v
  retrieving revision 1.88
  retrieving revision 1.89
  diff -u -r1.88 -r1.89
  --- mod_mem_cache.c	17 Nov 2002 01:33:25 -0000	1.88
  +++ mod_mem_cache.c	18 Dec 2002 15:07:21 -0000	1.89
  @@ -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,20 +454,28 @@
           return DECLINED;
       }
   
  -    /* 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 == -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;
  +    }
  +
  +    /* Note: cache_insert() will automatically garbage collect 
  +     * objects from the cache if the max_cache_size threshold is
  +     * exceeded. This means mod_mem_cache does not need to implement
  +     * max_cache_size checks.
        */
       if (len < sconf->min_cache_object_size || 
           len > sconf->max_cache_object_size) {
           ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
  -                     "cache_mem: URL %s failed the size check, "
  -                     "or is incomplete", 
  +                     "mem_cache: URL %s failed the size check and will not be cached.",
                        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 +501,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,15 +959,6 @@
   
           /* Content not suitable for fd caching. Cache in-memory instead. */
           mobj->type = CACHE_TYPE_HEAP;
  -        /* 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) {
  -            return APR_ENOMEM; /* ?? DECLINED; */
  -        }
  -        if ((sconf->cache_size + mobj->m_len) > sconf->max_cache_size) {
  -            return APR_ENOMEM; /* ?? DECLINED; */
  -        }
  -        sconf->cache_size += mobj->m_len;
       }
   
       /* 
  @@ -977,6 +980,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 *buf = malloc(obj->count);
  +                if (!buf) {
  +                    return APR_ENOMEM;
  +                }
  +                memcpy(buf, mobj->m, obj->count);
  +                free(mobj->m);
  +                mobj->m = buf;
  +
  +                /* 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 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);                
  +                sconf->cache_size -= (mobj->m_len - obj->count);
  +                if (sconf->lock) {
  +                    apr_thread_mutex_unlock(sconf->lock);
  +                }
  +            }
               /* Open for business */
               obj->complete = 1;
               break;
  @@ -1022,6 +1053,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 +1156,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 +1181,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}
   };
   
  
  
  

RE: cvs commit: httpd-2.0/modules/experimental cache_cache.c mod_cache.c mod_cache.h mod_mem_cache.c

Posted by Bill Stoddard <bi...@wstoddard.com>.
> >   +    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.");
> >   ...
> 
> 
> Wouldn't it be better to write:
>   "MCacheMaxStreamingBuffer must be greater than or equal to 
> MCacheMinObjectSize. "
>   "Resetting MCacheMaxStreamingBuffer to MCacheMinObjectSize.");
> 
>  Kess

Most assuredly. Thanks for the report.

Bill

Re: cvs commit: httpd-2.0/modules/experimental cache_cache.c mod_cache.c mod_cache.h mod_mem_cache.c

Posted by Astrid Keßler <ke...@kess-net.de>.
>   +    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.");
>   ...


Wouldn't it be better to write:
  "MCacheMaxStreamingBuffer must be greater than or equal to MCacheMinObjectSize. "
  "Resetting MCacheMaxStreamingBuffer to MCacheMinObjectSize.");

 Kess