Re: CHANGES file for 1.3 and 2.x

[Justin Erenkrantz]

On 5/7/07, Jim Jagielski <[EMAIL PROTECTED]> wrote:

Seems to me that the more we work on the various 2.x trees
(2.0.x, 2.2.x and trunk), the harder it becomes to get
the various correct CHANGES entries in sync... For example,
the CHANGES for 2.2 and trunk just refer to changes up
to 2.0.56... What's the best way of syncing these? Should
we stop having a single CHANGES that tries to merge all
development together? Why not have something like:

   o Apache 1.3.x: CHANGES stay as is
   o Apache 2.0.x: CHANGES just incorporates 2.0.x work
   and we can either URL refer to older changes
   at the bottom of the file or, when we release,
   merge them.
   o Apache 2.2.x: CHANGES just notes 2.2.x work and we
   either refer to 2.0.x CHANGES and 1.3.x CHANGES
   or auto-merge when we release.
   o : Same pattern...

Comments? Ideas?


+1 for having the CHANGES file be local to the branch and stop trying
to keep them in sync.

For example, what I'd say is that when we have something in trunk and
it gets merged to the current stable release, it just gets removed
from the trunk's CHANGES altogether.

Furthermore, at the end of the 2.2.x CHANGES file, it should point at
a URL or something for 2.0.x CHANGES.  2.0.x CHANGES at the end can
point to a URL for 1.3.x CHANGES...(I wouldn't touch 1.3.x CHANGES,
but it's unlikely we'd do many more releases for that.)

I also doubt that many users are optimistic to read a 650KB+ file of
CHANGES (!).  I think those doing anthropology studies or are
voyeurs can learn how to use Subversion to follow the history.  Most
folks, at best, would only want to know what's up since the prior
minor release (2.0->2.2->2.4).  Plus, it's a good way to slim down our
checkouts - yay!  -- justin

Re: svn commit: r539155 - /httpd/httpd/branches/2.2.x/STATUS

[William A. Rowe, Jr.]

[EMAIL PROTECTED] wrote:
>  http://people.apache.org/~wrowe/mpm_winnt_waits.patch
>  is easier to read (-U8)
>+1: mturk
> +  +0: fielding (patch is okay, underlying code is crap)
>wrowe notes: a patch should have the necessary effect with the
>  minimum lines of code - there's alot of redecorating that's
>  going on in this patch to no net effect.  The WAIT_TIMEOUT
>  result value seems to be ignored in the revised code?
>mturk notes: WAIT_TIMEOUT is replaced by WAIT_FAILED with
>  the accompanied patch in mpm\winnt\child.c.
> +  fielding notes: the routine is brain-dead -- one cannot replicate
> +a wait for many objects by iterating through multiple waits with
> +a one second sleep in between loops.  That's insane.
> +The right ways to do that are explained in the MSDN article
> +
> +In any case, it should be checking nCount <= MAXIMUM_WAIT_OBJECTS
> +first and use the simple wait in that case.

+1 to all of your observations.  I believe the theory was that all of the
waits would be satisfied eventually and be terminating in parallel, so the
'fine detail' of which ones terminated when was of very little importance.

But on the risk that this code would show up somewhere that *mattered*, I
totally agree with your suggestion of replacing the logic.

Bill

Re: [PATCH] mod_wombat separate out connection and server

[Garrett Rooney]

On 5/17/07, Akins, Brian <[EMAIL PROTECTED]> wrote:

Had a couple hours while on vacation after reading PiL.  This makes
connection, server, and apr_table into real lua "modules."  I also separated
out the code and started playing with getters and setters.

I like the idea of doing the function tables in "plain" C, rather than "Lua"
C.  Mostly because I understand it more :)  Also, it may be faster as it
avoids using a bunch of Lua tables to keep track of callbacks and functions.
Would be interesting to have a performance bake-off.

Haven't found a good way to rework request.c into this arrangement, but I
ran out of time.


I'm not a fan of the way the pools and hash tables are lazily
initialized, as it isn't thread safe and one of the nice things about
mod_wombat is its thread safety.  Perhaps something that's initialized
during server startup instead?

Also the new files all need license headers.

-garrett

Re: Any progress on PR41230 (HEAD issues on cached items)?

[Justin Erenkrantz]

On 5/17/07, Ruediger Pluem <[EMAIL PROTECTED]> wrote:

why. Also the entity is not physically removed from the cache if it is really 
stale.
This does not matter in the non HEAD case as it gets overwritten by the fresh 
response,
but in the HEAD case it should be physically removed IMO.


Well, because rv == !OK, wouldn't the CACHE_REMOVE_URL filter hit?
That should do the dirty deed, no?  -- justin

Re: [PATCH] Stop mod_cache from being too helpful was Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Justin Erenkrantz]

On 5/17/07, Ruediger Pluem <[EMAIL PROTECTED]> wrote:

Hm. In this case date would be undefined and it is used later on.


Oh.  Good catch.


Maybe we should remove the else branch and set date = info->date in
any case so that it is either the contents of Date: or now.


Sure.  That'll work.

See below.  Also removed an unused variable too.  -- justin

Index: modules/cache/mod_cache.c
===
--- modules/cache/mod_cache.c   (revision 539156)
+++ modules/cache/mod_cache.c   (working copy)
@@ -318,7 +318,6 @@
static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
{
int rv = !OK;
-int date_in_errhdr = 0;
request_rec *r = f->r;
cache_request_rec *cache;
cache_server_conf *conf;
@@ -648,10 +647,7 @@

/* Read the date. Generate one if one is not supplied */
dates = apr_table_get(r->err_headers_out, "Date");
-if (dates != NULL) {
-date_in_errhdr = 1;
-}
-else {
+if (dates == NULL) {
dates = apr_table_get(r->headers_out, "Date");
}
if (dates != NULL) {
@@ -663,25 +659,10 @@

now = apr_time_now();
if (info->date == APR_DATE_BAD) {  /* No, or bad date */
-char *dates;
/* no date header (or bad header)! */
-/* add one; N.B. use the time _now_ rather than when we were checking
- * the cache
- */
-if (date_in_errhdr == 1) {
-apr_table_unset(r->err_headers_out, "Date");
-}
-date = now;
-dates = apr_pcalloc(r->pool, MAX_STRING_LEN);
-apr_rfc822_date(dates, now);
-apr_table_set(r->headers_out, "Date", dates);
-ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
- "cache: Added date header");
-info->date = date;
+info->date = now;
}
-else {
-date = info->date;
-}
+date = info->date;

/* set response_time for HTTP/1.1 age calculations */
info->response_time = now;
@@ -709,7 +690,6 @@
 *  expire date = date + defaultexpire
 */
if (exp == APR_DATE_BAD) {
-char expire_hdr[APR_RFC822_DATE_LEN];
char *max_age_val;

if (ap_cache_liststr(r->pool, cc_out, "max-age", &max_age_val) &&
@@ -746,13 +726,9 @@
x = conf->maxex;
}
exp = date + x;
-apr_rfc822_date(expire_hdr, exp);
-apr_table_set(r->headers_out, "Expires", expire_hdr);
}
else {
exp = date + conf->defex;
-apr_rfc822_date(expire_hdr, exp);
-apr_table_set(r->headers_out, "Expires", expire_hdr);
}
}
info->expire = exp;

[PATCH] mod_wombat separate out connection and server

[Akins, Brian]

Had a couple hours while on vacation after reading PiL.  This makes
connection, server, and apr_table into real lua "modules."  I also separated
out the code and started playing with getters and setters.

I like the idea of doing the function tables in "plain" C, rather than "Lua"
C.  Mostly because I understand it more :)  Also, it may be faster as it
avoids using a bunch of Lua tables to keep track of callbacks and functions.
Would be interesting to have a performance bake-off.

Haven't found a good way to rework request.c into this arrangement, but I
ran out of time.




-- 
Brian Akins
Chief Operations Engineer
Turner Digital Media Technologies



bakins-big.diff
Description: Binary data

RE: TLP Name

[Daniel J. Popowich]

 +1 on pypache

Re: [PATCH] Stop mod_cache from being too helpful was Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Ruediger Pluem]

On 05/18/2007 01:33 AM, Justin Erenkrantz wrote:
> 
> With this patch, the only thing mod_cache ever touches is...Age.  =)
> 
> I'll let this sit here for a bit and if we agree this is right, we can
> commit.  -- justin
> 
> Index: modules/cache/mod_cache.c
> ===
> --- modules/cache/mod_cache.c(revision 539156)
> +++ modules/cache/mod_cache.c(working copy)
> @@ -663,21 +663,8 @@
> 
> now = apr_time_now();
> if (info->date == APR_DATE_BAD) {  /* No, or bad date */
> -char *dates;
> /* no date header (or bad header)! */
> -/* add one; N.B. use the time _now_ rather than when we were
> checking
> - * the cache
> - */
> -if (date_in_errhdr == 1) {
> -apr_table_unset(r->err_headers_out, "Date");
> -}
> -date = now;

Hm. In this case date would be undefined and it is used later on.
Maybe we should remove the else branch and set date = info->date in
any case so that it is either the contents of Date: or now.

Regards

Rüdiger

Re: Any progress on PR41230 (HEAD issues on cached items)?

[Ruediger Pluem]

On 05/18/2007 01:26 AM, Justin Erenkrantz wrote:
> On 5/17/07, Niklas Edmundsson <[EMAIL PROTECTED]> wrote:
> 
>> Has there been any progress on PR41230? I submitted a patch that at
>> least seems to improve the situation that now seems to have seen some
>> testing by others as well.
>>
>> As I have stated before, it would be really nice if a fix for this
>> could be committed, be it my patch or some other solution.
> 
> 
> (It'd be helpful if you posted the patch here instead of forcing us to
> go to Bugzilla...)

He has a while ago :-):

http://mail-archives.apache.org/mod_mbox/httpd-dev/200704.mbox/[EMAIL PROTECTED]

> 
> Anyway, the core of the patch is okay - but what's the deal with the
> rv = !OK line?  I don't see how or why that's necessary as rv is
> already initialized to that.
> 
> I'd tweak it to be as follows for readability though.  If this still
> fixes your problem, I can commit and propose for backport.  Thanks!

It (meaning your and his patch) does not fix it in all cases IMO. See

http://mail-archives.apache.org/mod_mbox/httpd-dev/200704.mbox/[EMAIL PROTECTED]

why. Also the entity is not physically removed from the cache if it is really 
stale.
This does not matter in the non HEAD case as it gets overwritten by the fresh 
response,
but in the HEAD case it should be physically removed IMO.

Anyway, thanks for removing this rv = !OK stuff.

Regards

Rüdiger

Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Roy T. Fielding]

On May 17, 2007, at 4:37 PM, Justin Erenkrantz wrote:

See the patch I just posted - we should also remove the Date
manipulation too, IMO.


Probably.  If Date is added, it should be done by the protocol filter
(mod_proxy) when the message is received, not the cache.

Just for clarification: It is still ok for us to define an cache  
internal expiration
date (default / calculated) if there is nothing valid (max-age,  
Expires) in the

response, right?


Yes.

We just do not tell the client about it as we do not adjust the  
headers in the response

accordingly.


Exactly.

BTW: What about s-max-age here? Shouldn't we use s-maxage instead  
of max-age in the

case it is present in the response?


Probably.  I've only dug into the max-age case so far - I haven't
chased down the semantics for us supporting s-maxage yet.  Though
max-age is explicitly called out as an explicit expiration time, while
s-maxage isn't.  Again, I'd need to refresh in my head what s-maxage
is for...  -- justin


s-maxage is for use when the value for a shared cache would differ
from the value for a private cache.

  s-maxage
   If a response includes an s-maxage directive, then for a shared
   cache (but not for a private cache), the maximum age  
specified by

   this directive overrides the maximum age specified by either the
   max-age directive or the Expires header. The s-maxage directive
   also implies the semantics of the proxy-revalidate directive  
(see

   section 14.9.4), i.e., that the shared cache must not use the
   entry after it becomes stale to respond to a subsequent request
   without first revalidating it with the origin server. The s-
   maxage directive is always ignored by a private cache.

We should probably have a directive somewhere to indicate whether
we are operating a shared cache (default) or private cache.

Roy

Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Justin Erenkrantz]

On 5/17/07, Ruediger Pluem <[EMAIL PROTECTED]> wrote:

I don't want to pick on you, but ironically it was you who introduced this :-)
(http://svn.apache.org/viewvc?view=rev&revision=152973).


Oh, it probably was.  What can I say?  I'm less stupid now.  =)


Ok. So we should remove the

apr_rfc822_date(expire_hdr, exp);
apr_table_set(r->headers_out, "Expires", expire_hdr);

lines there.


See the patch I just posted - we should also remove the Date
manipulation too, IMO.


Just for clarification: It is still ok for us to define an cache internal 
expiration
date (default / calculated) if there is nothing valid (max-age, Expires) in the
response, right?


Yes.


We just do not tell the client about it as we do not adjust the headers in the 
response
accordingly.


Exactly.


BTW: What about s-max-age here? Shouldn't we use s-maxage instead of max-age in 
the
case it is present in the response?


Probably.  I've only dug into the max-age case so far - I haven't
chased down the semantics for us supporting s-maxage yet.  Though
max-age is explicitly called out as an explicit expiration time, while
s-maxage isn't.  Again, I'd need to refresh in my head what s-maxage
is for...  -- justin

[PATCH] Stop mod_cache from being too helpful was Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Justin Erenkrantz]

On 5/17/07, Roy T. Fielding <[EMAIL PROTECTED]> wrote:

On May 17, 2007, at 2:53 PM, Justin Erenkrantz wrote:
> BTW, I'm not a fan of us inventing Expires headers in this section of
> code - I'd think it'd be far cleaner to off-load Expires response
> header generation to mod_expires and leave the cache out of it
> entirely - inventing Expires values deep inside of mod_cache seems
> unclean.  mod_cache, IMO, should just respect what it is told and not
> change how things appear to downstream folks.  (mod_expires is more
> than capable of inserting in the Expires header if the admin wants
> it.)

I agree -- caches are not allowed to invent header fields like Expires.
They can only do so by explicit override in the configuration
(mod_expires).
Setting Expires here is wrong.  Changing max-age would be even worse.
Age is the only thing the cache should be setting.

> Does my position make sense?  I'm of the opinion that we should go one
> of two ways:
>
> - mod_cache shouldn't touch the response - so it stops setting Expires
> or anything like that which affects cachability

+1

> - mod_cache always tweaks the response - so my CC: max-age fix needs
> to mimic what we do for Expires.

-1


With this patch, the only thing mod_cache ever touches is...Age.  =)

I'll let this sit here for a bit and if we agree this is right, we can
commit.  -- justin

Index: modules/cache/mod_cache.c
===
--- modules/cache/mod_cache.c   (revision 539156)
+++ modules/cache/mod_cache.c   (working copy)
@@ -663,21 +663,8 @@

now = apr_time_now();
if (info->date == APR_DATE_BAD) {  /* No, or bad date */
-char *dates;
/* no date header (or bad header)! */
-/* add one; N.B. use the time _now_ rather than when we were checking
- * the cache
- */
-if (date_in_errhdr == 1) {
-apr_table_unset(r->err_headers_out, "Date");
-}
-date = now;
-dates = apr_pcalloc(r->pool, MAX_STRING_LEN);
-apr_rfc822_date(dates, now);
-apr_table_set(r->headers_out, "Date", dates);
-ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
- "cache: Added date header");
-info->date = date;
+info->date = now;
}
else {
date = info->date;
@@ -709,7 +696,6 @@
 *  expire date = date + defaultexpire
 */
if (exp == APR_DATE_BAD) {
-char expire_hdr[APR_RFC822_DATE_LEN];
char *max_age_val;

if (ap_cache_liststr(r->pool, cc_out, "max-age", &max_age_val) &&
@@ -746,13 +732,9 @@
x = conf->maxex;
}
exp = date + x;
-apr_rfc822_date(expire_hdr, exp);
-apr_table_set(r->headers_out, "Expires", expire_hdr);
}
else {
exp = date + conf->defex;
-apr_rfc822_date(expire_hdr, exp);
-apr_table_set(r->headers_out, "Expires", expire_hdr);
}
}
info->expire = exp;

Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Ruediger Pluem]

On 05/18/2007 12:19 AM, Roy T. Fielding wrote:
> On May 17, 2007, at 2:53 PM, Justin Erenkrantz wrote:
> 
>> BTW, I'm not a fan of us inventing Expires headers in this section of

I don't want to pick on you, but ironically it was you who introduced this :-)
(http://svn.apache.org/viewvc?view=rev&revision=152973).

>> code - I'd think it'd be far cleaner to off-load Expires response
>> header generation to mod_expires and leave the cache out of it
>> entirely - inventing Expires values deep inside of mod_cache seems
>> unclean.  mod_cache, IMO, should just respect what it is told and not
>> change how things appear to downstream folks.  (mod_expires is more
>> than capable of inserting in the Expires header if the admin wants
>> it.)
> 
> 
> I agree -- caches are not allowed to invent header fields like Expires.
> They can only do so by explicit override in the configuration 
> (mod_expires).
> Setting Expires here is wrong.  Changing max-age would be even worse.
> Age is the only thing the cache should be setting.
> 
>> Does my position make sense?  I'm of the opinion that we should go one
>> of two ways:
>>
>> - mod_cache shouldn't touch the response - so it stops setting Expires
>> or anything like that which affects cachability
> 
> 
> +1

Ok. So we should remove the

apr_rfc822_date(expire_hdr, exp);
apr_table_set(r->headers_out, "Expires", expire_hdr);

lines there.
Just for clarification: It is still ok for us to define an cache internal 
expiration
date (default / calculated) if there is nothing valid (max-age, Expires) in the
response, right?
We just do not tell the client about it as we do not adjust the headers in the 
response
accordingly.
BTW: What about s-max-age here? Shouldn't we use s-maxage instead of max-age in 
the
case it is present in the response?

Regards

Rüdiger

Re: Any progress on PR41230 (HEAD issues on cached items)?

[Justin Erenkrantz]

On 5/17/07, Niklas Edmundsson <[EMAIL PROTECTED]> wrote:

Has there been any progress on PR41230? I submitted a patch that at
least seems to improve the situation that now seems to have seen some
testing by others as well.

As I have stated before, it would be really nice if a fix for this
could be committed, be it my patch or some other solution.


(It'd be helpful if you posted the patch here instead of forcing us to
go to Bugzilla...)

Anyway, the core of the patch is okay - but what's the deal with the
rv = !OK line?  I don't see how or why that's necessary as rv is
already initialized to that.

I'd tweak it to be as follows for readability though.  If this still
fixes your problem, I can commit and propose for backport.  Thanks!
-- justin

Index: modules/cache/mod_cache.c
===
--- modules/cache/mod_cache.c   (revision 539156)
+++ modules/cache/mod_cache.c   (working copy)
@@ -476,8 +476,10 @@
reason = "No Last-Modified, Etag, or Expires headers";
}
else if (r->header_only) {
-/* HEAD requests */
-reason = "HTTP HEAD request";
+/* Forbid HEAD requests unless we have it cached already */
+if (!cache->stale_handle) {
+reason = "HTTP HEAD request";
+}
}
else if (!conf->store_nostore &&
 ap_cache_liststr(NULL, cc_out, "no-store", NULL)) {
@@ -611,8 +613,8 @@
}
}

-/* no cache handle, create a new entity */
-if (!cache->handle) {
+/* no cache handle, create a new entity only for non-HEAD requests */
+if (!cache->handle && !r->header_only) {
rv = cache_create_entity(r, size);
info = apr_pcalloc(r->pool, sizeof(cache_info));
/* We only set info->status upon the initial creation. */

Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Roy T. Fielding]

On May 17, 2007, at 2:53 PM, Justin Erenkrantz wrote:

BTW, I'm not a fan of us inventing Expires headers in this section of
code - I'd think it'd be far cleaner to off-load Expires response
header generation to mod_expires and leave the cache out of it
entirely - inventing Expires values deep inside of mod_cache seems
unclean.  mod_cache, IMO, should just respect what it is told and not
change how things appear to downstream folks.  (mod_expires is more
than capable of inserting in the Expires header if the admin wants
it.)


I agree -- caches are not allowed to invent header fields like Expires.
They can only do so by explicit override in the configuration  
(mod_expires).

Setting Expires here is wrong.  Changing max-age would be even worse.
Age is the only thing the cache should be setting.


Does my position make sense?  I'm of the opinion that we should go one
of two ways:

- mod_cache shouldn't touch the response - so it stops setting Expires
or anything like that which affects cachability


+1


- mod_cache always tweaks the response - so my CC: max-age fix needs
to mimic what we do for Expires.


-1

Roy

Re: svn commit: r539117 - /httpd/httpd/branches/2.2.x/STATUS

[Roy T. Fielding]

On May 17, 2007, at 2:48 PM, Ruediger Pluem wrote:

On 05/17/2007 11:28 PM, Justin Erenkrantz wrote:

On 5/17/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:

+ rpluem : Now that CacheIgnoreQueryString has been  
backported

r538807
+  applies cleanly to 2.2.x. So I assume it is ok  
to use

+  r538807 instead of max-age-2.2.x.patch.



Aye - that's fine and just keeps them in sync.

Do you want to do the merge or shall I?  =)


As Roy has already voted for it before I added my comment, I just  
wanted to give
him a chance to give a comment if he thinks that this is needed (in  
the same way as
I wanted to give you a chance to comment). But in this simple case  
it may not be

needed to wait. So feel free to merge r538807 into 2.2.x if you like.


Go ahead -- I tested trunk and visually inspected the patches, so the
merged version is okay by me.

Roy

Re: sys.path and htaccess change?

[Graham Dumpleton]

Read:

 http://www.modpython.org/live/current/doc-html/pyapi-apmeth.html

Especially the area which starts just before:

 PythonOption mod_python.importer.path "['~']"

Your particular issue is mentioned just after this example.

Graham

On 18/05/07, Daniel J. Popowich <[EMAIL PROTECTED]> wrote:


In mod_python 3.2.x, if I set a directory to use mod_python with a
.htaccess file, I will find that directory in sys.path.  That is, if:

   /var/www/somedir/.htaccess

contains:

  SetHandler python-program

then sys.path will have "/var/www/somedir" as element zero.

With 3.3.x this does not happen.

The problem?  I have a handler with co-located python modules that get
imported (the handler does "import somemod" and somemomd is in the
same directory as the .htaccess file).  This works fine with 3.2.x,
but when I upgraded to 3.3.1 I get import errors.

I searched the changelogs in the documentation, but I see no mention
of this change.

Thanks,

Daniel Popowich
---
http://www.astro.umass.edu/~dpopowich/python/mpservlets/

Output Filtering

[Mark Zetts]

  I'm processing some custom XML of considerable length in an output 
filter and I collect the entire response, flatten it, process it, then 
pass it on.  One particular response body comprises 17 brigades of which 5 
are zero length.  As I'm coalescing these bucket brigades, is it correct 
behavior to:

1) Pass on any brigades of 0 length?

2) Delete non-EOS metadata buckets?

  Everything seems to work, but I'd like to know if there are any pitfalls 
to avoid when processing response bodies in the whole.

Thanks,
Mark Zetts 
 

Re: svn commit: r539117 - /httpd/httpd/branches/2.2.x/STATUS

[Justin Erenkrantz]

On 5/17/07, Ruediger Pluem <[EMAIL PROTECTED]> wrote:

As Roy has already voted for it before I added my comment, I just wanted to give
him a chance to give a comment if he thinks that this is needed (in the same 
way as
I wanted to give you a chance to comment). But in this simple case it may not be
needed to wait. So feel free to merge r538807 into 2.2.x if you like.


No rush.  I'll let it sit for a bit and come back to it later this evening.

Thanks!  -- justin

Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Justin Erenkrantz]

On 5/17/07, Ruediger Pluem <[EMAIL PROTECTED]> wrote:

But it already does in the case of the Expires header. If the Expires
header is missing or bogus and no max-age field is present (valid or invalid),
we set one with the expires date the cache calculated (either heuristic or
default). And this creates the next question for me: Why always use the default
expire setting in the case of a bogus max-age? In the case of a bogus Expire
header we try to calculate a heuristic expiration date using the Last-Modified
header (if present and valid).
I think we should handle Expires and max-age in the same way here.


My concern with falling back to the Last-Modified date on a bogus
Cache-Control max-age field is that the server was *trying* to tell us
something via the max-age field.  So, falling back to Last-Modified
seems wrong-ish.  I'd rather just stay silent here and go with our
default value.

BTW, I'm not a fan of us inventing Expires headers in this section of
code - I'd think it'd be far cleaner to off-load Expires response
header generation to mod_expires and leave the cache out of it
entirely - inventing Expires values deep inside of mod_cache seems
unclean.  mod_cache, IMO, should just respect what it is told and not
change how things appear to downstream folks.  (mod_expires is more
than capable of inserting in the Expires header if the admin wants
it.)


> shouldn't tweak the value under any circumstances.
>
> I can be convinced that's the wrong approach and that we should indeed
> normalize it in the event of an error, but my gut says letting is just
> pass-through seems right.  -- justin

But shouldn't the client have the same expiration expectation as the cache?
In the case of an invalid max-age field I would see it as a correct decision
by a client to ignore it completely for its local cache and thus possibly
do not cache it at all.


Since the cache is not authoritative, I think it's slightly sketchy
for us to tweak any downstream value based on our local policy
decisions.

Does my position make sense?  I'm of the opinion that we should go one
of two ways:

- mod_cache shouldn't touch the response - so it stops setting Expires
or anything like that which affects cachability

- mod_cache always tweaks the response - so my CC: max-age fix needs
to mimic what we do for Expires.

WDYT?  -- justin

Re: svn commit: r539117 - /httpd/httpd/branches/2.2.x/STATUS

[Ruediger Pluem]

On 05/17/2007 11:28 PM, Justin Erenkrantz wrote:
> On 5/17/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:
> 
>> + rpluem : Now that CacheIgnoreQueryString has been backported
>> r538807
>> +  applies cleanly to 2.2.x. So I assume it is ok to use
>> +  r538807 instead of max-age-2.2.x.patch.
> 
> 
> Aye - that's fine and just keeps them in sync.
> 
> Do you want to do the merge or shall I?  =)

As Roy has already voted for it before I added my comment, I just wanted to give
him a chance to give a comment if he thinks that this is needed (in the same 
way as
I wanted to give you a chance to comment). But in this simple case it may not be
needed to wait. So feel free to merge r538807 into 2.2.x if you like.


Regards

Rüdiger

Re: TLP Name

[m.banaouas]

PyPache +1


Jeffrey Robbins a écrit :

Scales works for me - I like the double entendre
-Original Message-
From: Jorey Bump <[EMAIL PROTECTED]>
Date: Wednesday, May 16, 2007 5:51 pm
Subject: Re: TLP Name
To: "Gregory (Grisha) Trubetskoy" <[EMAIL PROTECTED]>
CC: [EMAIL PROTECTED]

For me, it's a tie between Quetzalcoatl & Scales.

Gregory (Grisha) Trubetskoy wrote:
 
 So I think we've got (in no particular order):
 
 PythonScript

 Pythonidae
 PyPache
 pythonalia
 Quetzalcoatl
 Asphyxia
 Scales
 Pythonistas
 PigeonPy
 Pungi
 
 Would people (ANYONE here on the list, yes, that includes *YOU*) please 
 respond with the one they like most and I will compile the results?
 
 Thanks!
 
 Grisha



  

Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Ruediger Pluem]

On 05/17/2007 10:56 PM, Justin Erenkrantz wrote:
> On 5/17/07, Ruediger Pluem <[EMAIL PROTECTED]> wrote:
> 
>> > +x = apr_atoi64(max_age_val);
>> > +if (errno) {
>> > +x = conf->defex;
>>
>> Hm. Shouldn't we adjust the bogus max-age field value in the
>> Cache-Control header in r->headers_out
>> / r->err_headers_out to conf->defex (in seconds of course) in this case?
> 
> 
> I thought about doing that - but I came to the idea that our local
> cache is doing its own validation, but it shouldn't make its local
> expiration decisions binding upon downstream requestors - i.e. it

But it already does in the case of the Expires header. If the Expires
header is missing or bogus and no max-age field is present (valid or invalid),
we set one with the expires date the cache calculated (either heuristic or
default). And this creates the next question for me: Why always use the default
expire setting in the case of a bogus max-age? In the case of a bogus Expire
header we try to calculate a heuristic expiration date using the Last-Modified
header (if present and valid).
I think we should handle Expires and max-age in the same way here.

> shouldn't tweak the value under any circumstances.
> 
> I can be convinced that's the wrong approach and that we should indeed
> normalize it in the event of an error, but my gut says letting is just
> pass-through seems right.  -- justin

But shouldn't the client have the same expiration expectation as the cache?
In the case of an invalid max-age field I would see it as a correct decision
by a client to ignore it completely for its local cache and thus possibly
do not cache it at all.


Regards

Rüdiger

Re: svn commit: r539117 - /httpd/httpd/branches/2.2.x/STATUS

[Justin Erenkrantz]

On 5/17/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:

+ rpluem : Now that CacheIgnoreQueryString has been backported r538807
+  applies cleanly to 2.2.x. So I assume it is ok to use
+  r538807 instead of max-age-2.2.x.patch.


Aye - that's fine and just keeps them in sync.

Do you want to do the merge or shall I?  =)

Thanks!  -- justin

Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Justin Erenkrantz]

On 5/17/07, Ruediger Pluem <[EMAIL PROTECTED]> wrote:

> +x = apr_atoi64(max_age_val);
> +if (errno) {
> +x = conf->defex;

Hm. Shouldn't we adjust the bogus max-age field value in the Cache-Control header 
in r->headers_out
/ r->err_headers_out to conf->defex (in seconds of course) in this case?


I thought about doing that - but I came to the idea that our local
cache is doing its own validation, but it shouldn't make its local
expiration decisions binding upon downstream requestors - i.e. it
shouldn't tweak the value under any circumstances.

I can be convinced that's the wrong approach and that we should indeed
normalize it in the event of an error, but my gut says letting is just
pass-through seems right.  -- justin

Re: svn commit: r539063 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c

[Ruediger Pluem]

On 05/17/2007 08:25 PM, [EMAIL PROTECTED] wrote:
> Author: jerenkrantz
> Date: Thu May 17 11:25:13 2007
> New Revision: 539063

> Modified: httpd/httpd/trunk/modules/cache/mod_cache.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_cache.c?view=diff&rev=539063&r1=539062&r2=539063
> ==
> --- httpd/httpd/trunk/modules/cache/mod_cache.c (original)
> +++ httpd/httpd/trunk/modules/cache/mod_cache.c Thu May 17 11:25:13 2007
> @@ -701,19 +701,42 @@
>  }
>  
>  /* if no expiry date then
> - *   if lastmod
> + *   if Cache-Control: max-age
> + *  expiry date = date + max-age
> + *   else if lastmod
>   *  expiry date = date + min((date - lastmod) * factor, maxexpire)
>   *   else
>   *  expire date = date + defaultexpire
>   */
>  if (exp == APR_DATE_BAD) {
>  char expire_hdr[APR_RFC822_DATE_LEN];
> +char *max_age_val;
>  
> -/* if lastmod == date then you get 0*conf->factor which results in
> - *   an expiration time of now. This causes some problems with
> - *   freshness calculations, so we choose the else path...
> - */
> -if ((lastmod != APR_DATE_BAD) && (lastmod < date)) {
> +if (ap_cache_liststr(r->pool, cc_out, "max-age", &max_age_val) &&
> +max_age_val != NULL) {
> +apr_int64_t x;
> +
> +errno = 0;
> +x = apr_atoi64(max_age_val);
> +if (errno) {
> +x = conf->defex;

Hm. Shouldn't we adjust the bogus max-age field value in the Cache-Control 
header in r->headers_out
/ r->err_headers_out to conf->defex (in seconds of course) in this case?

Regards

Rüdiger

Any progress on PR41230 (HEAD issues on cached items)?

[Niklas Edmundsson]

Has there been any progress on PR41230? I submitted a patch that at 
least seems to improve the situation that now seems to have seen some 
testing by others as well.


As I have stated before, it would be really nice if a fix for this 
could be committed, be it my patch or some other solution.



/Nikke
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se  | [EMAIL PROTECTED]
---
 Don't hide your contempt of the contemptible!
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

Re: svn commit: r538863 - /httpd/httpd/branches/2.2.x/STATUS

[Justin Erenkrantz]

On 5/17/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:

@@ -298,6 +298,8 @@
http://people.apache.org/~jerenkrantz/max-age-2.2.x.patch
(Trivially conflicts with CacheIgnoreQueryString above.)
  +1: jerenkrantz
+ rpluem asks: Why not backporting the adjusted error message :-)?
+  Otherwise +1.


It'd minimize the churn.  *shrug*  -- justin