httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Niklas Edmundsson <>
Subject Re: Solved: mod_disk_cache and mod_include bugs and suggestions
Date Wed, 17 Jan 2007 09:36:23 GMT
On Wed, 17 Jan 2007, Giuliano Gavazzi wrote:

> I have a solution for the r470455 mod_disk_cache not caching SSI.
> There are two points where the module seems incorrect to me, changing those 
> makes it work:

Since you're talking about the code on trunk, you should be warned 
that the current state is somewhat unreliable due to merging patches 
which then ran into an implementation discussion that never got solved 
(I think). Last I heard, the current plan is to revoke most patches 
and redo stuff.

However, since I'm the one to blame for the patches that has been 
partially landed on trunk (which is the parts you touch) I can provide 
my comments on your solutions (and I hope that others can chime in 
where I'm wrong).

First, don't reindent code when not needed. That only serves to make 
your patch hard to read.

> 1) in store_body the condition (!APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) was 
> incorrectly stopping the flow from ever going past (for static and dynamic 
> pages). I moved it, changing the condition. I will post the patch tomorrow.

>From looking at the patch I can only say "huh?". The brigade is 
complete when EOS is present, and only then can you complete the 
storing procedure. From a quick look at your patch I can't see how it 
could change things (instead of dropping out if not EOS you have a big 
if-chunk if it indeed is EOS, only adding an indentation level).

I might have missed some detail, but it's not obious from the 
hard-to-read patch...

> 2) in store_disk_headers nothing should happen (well, it should just return 
> or never be called) if the dobj->initial_size < 0.

It should be called, and it should do stuff.

One of the points of those patches are to solve the "thundering herd" 
problem, simply described as when a frequently accessed object is 
expired all accesses are served directly by your backend until one 
access has completed successfully and the cache has been able to store 
it. This is Bad if it causes your backend to grind to a halt.

To avoid this, the header is always written when the cache thinks it 
should cache something. Other requests will find this header, and if 
the size is unknown they will wait until it's updated with the correct 
size, otherwise they will do read-while-caching and return the 
contents as the file is being cached.

> Those two changes make the header cache file store the correct resource size 
> also for dynamic pages.

It stores the size, but doing so it breaks quite a few things.

I think it would be best if someone (Graham?) could revoke the status 
of mod_disk_cache on trunk to the agreed "last good" status, which is 
essentially the same as 2.2.4 if I remember correctly.

As for your problems, I would recommend staying on 2.2.4 proper and 
look further into the issue of expired/last-modified headers.

  Niklas Edmundsson, Admin @ {acc,hpc2n}      |
  And tomorrow will be like today, only more so.

View raw message