I've just been attempting to review Graham's proposed backport. Looks interesting, but just a few points:
Trivial: needs r809335. Trivial: indentation doesn't quite fit code style guidelines. - In ap_cache_try_lock - do we really need the hashed directories hardwired? I thought most modern filesystems had abandoned linear search, so that kind of thing is redundant! At least make it optional - perhaps an additional arg to the Cachelock directive. - Why is ap_cache_remove_lock called from so many places rather than just use a pool cleanup, and maybe in cache_save_filter as the earliest possible point in the normal processing path? - Do we need to use lock files like this? Not I think in every case: with mod_disk_cache we already have files we could lock (and create if they don't already exist). - Your ELSE clause (cache_util, line 623) for requests for a stale object that is being refreshed by another request serves the stale object and adds a warning. Is this the best thing to do? What about waiting for the file to be fetched? Should still be quicker than going to the backend in parallel with the other request. -- Nick Kew
