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
