httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Davi Arnaut <d...@haxent.com.br>
Subject Re: [PATCH] mod_disk_cache working LFS (filecopy)
Date Mon, 02 Oct 2006 15:33:07 GMT
Niklas Edmundsson wrote:
> On Mon, 2 Oct 2006, Davi Arnaut wrote:
> 
>>> Simpler, yes. But it only has the benefit of not eating all your
>>> memory...
>> Well, that was the goal. Maybe we could merge this one instead and work
>> together on the other goals.
> 
> As I have said before, we have a large patchset that fixes a bunch of 
> problems. However, since the wish was to merge it in pieces, I have 
> started breaking it into pieces for merging.

Thank you.

> If the intent is a total redesign of mod_disk_cache, ie you're not 
> interested in these patches at all, please say so and I would have not 
> wasted a lot of work on bending our patches to get something that 
> works when applying them one by one and then do QA on the thing.

Your work is never wasted, but the overall goal is to achieve a good
code quality whether is comes from you, me or anyone - personally
speaking since I'm not a apache committer.

I'm also interested in the same features you are, but we have to think
about long-term maintainability/quality to avoid half-backed short-term
solutions which wastes everybody's work.

> 
>>> * It leaves the brigade containing the uncached entity, so it will
>>>    cause the backend to first deliver stuff to be cached and then stuff
>>>    to the client.
>> Yeah, that's how mod_disk_cache works. I think it's possible to work
>> around this limitation without using threads by keeping each cache
>> instance with it's own brigades and flushing it occasionally with
>> non-blocking i/o.
> 
> The replace_brigade_with_cache()-function simply replaced the brigade 
> with an instance pointing to the cached entity.
> 
>> Or we could move all disk i/o to a mod_disk_cache exclusive thread pool,
>> it could be configurable at build time whether or not to use a thread pool.
>>
>> Comments ?
> 
> I would very happy if people would fix the mod_disk_cache mess so I 
> didn't have to. However, since noone seems to have produced something 
> usable for our usage in the timeframe mod_disk_cache has existed I was 
> forced to hack on it.
> 
> I'm trying my best to not give up on having it merged as I know that 
> there are other sites interested in it, and now that the first trivial 
> bit has been applied I'm hoping that people will at least look at the 
> rest... There are bound to be cool apachier ways to solve some of the 
> problems, but given that our patch is stable in production and has a 
> generally much higher code quality than mod_disk_cache (ever heard of 
> error checking/handling?) it would be nice if people at least could 
> look at the whole thing before starting to complain at the complexity 
> of small parts (or code not touched by the patch, for that matter).

As I said, we should always strive for the best possible quality or we
are doomed to keep fixing it over and over.

>>> * When this evolves to wanting to spawn a thread/process to do the
>>>    copying you'll need the "is this a file"-thingie anyway (at least I
>>>    need it, but I might have missed some nifty feature in APR).
>> You would just need to copy the remaining buckets (granted if there are
>> no concurrency problems) and send then to a per-process thread pool.
> 
> And when not having threads?

non-blocking when possible.

--
Davi Arnaut

Mime
View raw message