httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Justin Erenkrantz" <jus...@erenkrantz.com>
Subject Re: svn commit: r468373 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c modules/cache/mod_cache.h modules/cache/mod_disk_cache.c modules/cache/mod_disk_cache.h modules/cache/mod_mem_cache.c
Date Mon, 30 Oct 2006 05:58:04 GMT
On 10/29/06, Graham Leggett <minfrin@sharp.fm> wrote:
> The current expectation that it be possible to separate completely the
> storing of the cached response and the delivery of the content is broken.
>
> We have a real world case where the cache is expected to process a many
> MB or many GB file completely, before sending that same response to the
> network. This is too slow, and takes up too much RAM, resulting in a
> broken response to the client.

In short, I haven't seen any evidence presented by you or others that
this is due to a design flaw in the cache/provider abstraction.  At
best, mod_disk_cache could be smarter about storing the file when it's
large - but that's just a small lines of code to fix - it doesn't
require any massive changes or new bucket types.  Just copy the file
bucket and consume that within store_body()'s implementation.

If that isn't enough, please identify here on list and backed by
references to the implementation and timings that it is 'too slow',
'takes up too much RAM', and results in a 'broken response to the
client' and why we must break all of our cache structures and
abstractions.

I'm tired of reviewing large code changes with only vague generalities
about why the code must change.  These decisions need to be explained
and reviewed on list before any more code is committed.  Your recent
commits have been chock full of mistakes and style violations - which
make it almost impossible to review what's going on.  If you are going
to commit, please take care to follow all of our style guidelines and
please ensure the commit works before using our trunk as your personal
dumping ground.  If your changes are incomplete, feel free to post
them to the list instead of committing.

Looking at the current implementation of mod_disk_cache, someone has
turned it into unreadable and unmanagable code.

Take a look at:

http://svn.apache.org/repos/asf/httpd/httpd/tags/2.2.3/modules/cache/mod_disk_cache.c

versus

http://svn.apache.org/repos/asf/httpd/httpd/trunk/modules/cache/mod_disk_cache.c

The code somehow went from 9KB total to 15KB for no good reason.
Somewhere, we went really wrong here.

> So, we have disgreement over the right way to solve the problem of the
> cache being expected to swallow mouthfuls too big for it to handle.
>
> I agree with you that a design needs to be found on list first, as I
> have wasted enough time going round in circles coming up with solution
> after solution nobody is happy with.
>
> Do we put this to a vote?

We're not even close to knowing what we'd be voting on.  So, please
draft up a proposed design that explains in detail why you think there
is a problem and what specific changes you propose and why that is the
best solution.  If you want to submit a patch to go with your
rationale, cool.  Yet, I certainly don't see any fundamental reason
why Joe's concerns and mine can't both be addressed in the same
design.

I will also note that the mod_cache provider system has explicit
versioning, so any modifications to the providers should be
represented with a new version number.  (i.e. providers for version
"0" should work while offering new features in version "1"-class
providers.)  We do not arbitrarily tweak the old provider structures
any more - instead, we introduce new versions.  -- justin

Mime
View raw message