Bug 39380 - mod_disk_cache eats memory, has no LFS support, etc
Summary: mod_disk_cache eats memory, has no LFS support, etc
Status: RESOLVED LATER
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_cache_disk / mod_disk_cache (show other bugs)
Version: 2.2.0
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: MassUpdate, PatchAvailable
Depends on:
Blocks:
 
Reported: 2006-04-21 20:49 UTC by Niklas Edmundsson
Modified: 2018-11-07 21:08 UTC (History)
1 user (show)



Attachments
FIxes for mod_disk_cache: LFS-support, don't eat all your memory, etc. (17.96 KB, patch)
2006-04-21 20:50 UTC, Niklas Edmundsson
Details | Diff
Implement Large File Support in mod_disk_cache (4.36 KB, patch)
2006-04-22 09:46 UTC, Niklas Edmundsson
Details | Diff
mod_cache: Provide r->filename so %f in LogFormat works (1.16 KB, patch)
2006-04-22 09:52 UTC, Niklas Edmundsson
Details | Diff
mod_disk_cache: Consequently use disk_cache: in error log strings (2.65 KB, patch)
2006-04-23 08:34 UTC, Niklas Edmundsson
Details | Diff
mod_disk_cache: check if allowed to cache the file before caching (1.94 KB, patch)
2006-04-23 08:36 UTC, Niklas Edmundsson
Details | Diff
mod_disk_cache: Implement Large File Support (LFS) (5.18 KB, patch)
2006-04-23 08:37 UTC, Niklas Edmundsson
Details | Diff
httpd 2.2.3 - mod_disk_cache jumbo patch - lfs/diskformat/read-while-caching etc. (114.99 KB, patch)
2006-09-14 08:59 UTC, Niklas Edmundsson
Details | Diff
mod_disk_cache LFS-aware config (3.83 KB, patch)
2006-09-14 09:01 UTC, Niklas Edmundsson
Details | Diff
mod_disk_cache working LFS (filecopy) (12.89 KB, patch)
2006-09-26 08:52 UTC, Niklas Edmundsson
Details | Diff
Rearrange load/store to not depend on temporary files. (44.20 KB, patch)
2006-10-05 14:05 UTC, Niklas Edmundsson
Details | Diff
Read While Caching - don't stall clients accessing a file being cached. (11.75 KB, patch)
2006-10-05 14:08 UTC, Niklas Edmundsson
Details | Diff
Fixups for the load/store-patch. (3.28 KB, patch)
2006-10-08 10:37 UTC, Niklas Edmundsson
Details | Diff
Do background copy of a file while caching (12.10 KB, patch)
2006-10-08 11:03 UTC, Niklas Edmundsson
Details | Diff
httpd 2.2.4 - mod_disk_cache jumbo patch - lfs/diskformat/read-while-caching etc. (122.51 KB, patch)
2007-01-17 01:56 UTC, Niklas Edmundsson
Details | Diff
httpd-2.2.4 - mod_disk_cache jumbo patch - lfs/diskformat/read-while-caching etc. (133.92 KB, patch)
2007-07-27 14:06 UTC, Niklas Edmundsson
Details | Diff
httpd-2.2.6 - mod_disk_cache jumbo patch - 2.2.6 version (132.98 KB, patch)
2007-10-21 01:42 UTC, Niklas Edmundsson
Details | Diff
FIX htcacheclean for mod_disk_cache jumbo patch - 2.2.6 version (5.34 KB, patch)
2008-02-13 05:52 UTC, Rafael Pereira
Details | Diff
FIX corruption in near-simultaneous requests of uncached files for mod_disk_cache jumbo patch - 2.2.6 version (2.92 KB, patch)
2008-03-25 13:32 UTC, Kris Beevers
Details | Diff
httpd-2.2.9 - mod_disk_cache jumbo patch - 2.2.9 version (139.88 KB, patch)
2008-06-14 10:38 UTC, Niklas Edmundsson
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Niklas Edmundsson 2006-04-21 20:49:19 UTC
The attached patch addresses the following issues:

* Implement Large File Support (LFS) in mod_disk_cache.
* Try to check if allowed to cache the file before caching it too, first
  caching bits of a huge file and then toss it makes little sense.
* When caching a file, copy it using the file descriptor in the brigade instead
  of using apr_bucket_read which forces the file into memory. This produced a
  segfault if trying to cache a file larger than the available amount of memory.
* When having cached a file, replace the brigade referring to the source file
  with our cached copy. This makes a huge difference when the file is larger
  than your memory and thus not in cache, given that your cache filesystem
  is faster than your backend (a natural assumption, why cache otherwise?).
* When caching a file, keep the cache file even if the connection was aborted.
  There is no reason to toss it, and the penalty for doing so when caching
  DVD images is really huge.
* When multiple downloads of an uncached file is initiated, only allow one of
  them to cache the file and let the others wait for the result. It's not a
  theoretically perfect solution, but in practice it seems to work well.
* Consequently use disk_cache: in error log strings.
* In mod_cache, restore r->filename so %f in LogFormat strings work. This
  really should be solved by saving r->filename with the headers and restore
  it in mod_disk_cache et al, but this at least provides something.

This allows us (http://ftp.acc.umu.se/) to use mod_disk_cache to cache DVD
images on a 32bit machine with "only" 3GB of memory, with the thing behaving
in a sane way and our LogFormat xferlog style emulation working.

An annoying issue remains: When caching a file, mod_disk_cache copies the
entire file before data is being sent to the client. This gets really annoying
if you have large files (say 4.3GB DVD images) and a slow backend. If there
is work in progress to solve this, please point us in that direction so we can
get it finished.

I'm not on any Apache mailing lists, so please CC me any feedback on this.
Comment 1 Niklas Edmundsson 2006-04-21 20:50:36 UTC
Created attachment 18152 [details]
FIxes for mod_disk_cache: LFS-support, don't eat all your memory, etc.
Comment 2 Ruediger Pluem 2006-04-21 23:13:08 UTC
Many thanks for your patch. I will go through the points and the patch as soon
as I have time to. In order to ease the discussion, review and integration into
the trunk please split your patch into a separate patch for each issue you
address and attach them to this report. So there should be 8 patch files instead
of one.
Comment 3 Niklas Edmundsson 2006-04-22 09:45:02 UTC
OK. I'll start with two patches that are small and separated.

How do you propose to handle the rest? The problem is that since almost all
changes affect store_body() the only way for all of them to apply cleanly is
to make them depend on eachother. Is this OK?
Comment 4 Niklas Edmundsson 2006-04-22 09:46:57 UTC
Created attachment 18154 [details]
Implement Large File  Support in mod_disk_cache
Comment 5 Niklas Edmundsson 2006-04-22 09:52:18 UTC
Created attachment 18155 [details]
mod_cache: Provide r->filename so %f in LogFormat works

* In mod_cache, restore r->filename so %f in LogFormat strings work. This
  really should be solved by saving r->filename with the headers and restore
  it in mod_disk_cache et al, but this at least provides something.

%f is most often used when emulating the xferlog format (ie. when serving
files), so even if it's not 100% accurate at all times the side effects of it
being wrong isn't deadly.
Comment 6 Ruediger Pluem 2006-04-22 15:13:09 UTC
(In reply to comment #3)
> OK. I'll start with two patches that are small and separated.
> 
> How do you propose to handle the rest? The problem is that since almost all
> changes affect store_body() the only way for all of them to apply cleanly is
> to make them depend on eachother. Is this OK?

Yes, this is ok. Sorry that I have not been more precise on this subject. I
would like to see the low hanging fruits first, so that we get in the easiest
things first. By easy I also mean with possible least discussion on the mailing
list. So I would propose the following order for the patches:

* Consequently use disk_cache: in error log strings.
  (Yes, I am serious. I like to see this as a separate patch that goes in first,
as it actually does not really touch code.)
* Try to check if allowed to cache the file before caching it too, first
  caching bits of a huge file and then toss it makes little sense.
* Implement Large File Support (LFS) in mod_disk_cache.
* When caching a file, copy it using the file descriptor in the brigade instead
  of using apr_bucket_read which forces the file into memory. This produced a
  segfault if trying to cache a file larger than the available amount of memory.
* When having cached a file, replace the brigade referring to the source file
  with our cached copy. This makes a huge difference when the file is larger
  than your memory and thus not in cache, given that your cache filesystem
  is faster than your backend (a natural assumption, why cache otherwise?).
* In mod_cache, restore r->filename so %f in LogFormat strings work. This
  really should be solved by saving r->filename with the headers and restore
  it in mod_disk_cache et al, but this at least provides something.
* When caching a file, keep the cache file even if the connection was aborted.
  There is no reason to toss it, and the penalty for doing so when caching
  DVD images is really huge.
* When multiple downloads of an uncached file is initiated, only allow one of
  them to cache the file and let the others wait for the result. It's not a
  theoretically perfect solution, but in practice it seems to work well.

Don't feel discouraged if answers to single patches take longer. This is no lack
of interest, but in most cases lack of time or the need for a discussion on the
developer list. Feel free to give a ping here if you think that things stalled
completely. Thanks for your understanding and patience.
Comment 7 Niklas Edmundsson 2006-04-23 08:34:45 UTC
Created attachment 18157 [details]
mod_disk_cache: Consequently use disk_cache: in error log strings
Comment 8 Niklas Edmundsson 2006-04-23 08:36:09 UTC
Created attachment 18158 [details]
mod_disk_cache: check if allowed to cache the file before caching

* Try to check if allowed to cache the file before caching it too, first
  caching bits of a huge file and then toss it makes little sense.
Comment 9 Niklas Edmundsson 2006-04-23 08:37:50 UTC
Created attachment 18159 [details]
mod_disk_cache: Implement Large File Support (LFS)
Comment 10 Niklas Edmundsson 2006-04-23 08:43:50 UTC
I'll start with the first three patches so you can start processing stuff at
your end, and if there are things I have to do differently I don't have to
respin so many patches.

The mod_cache patch is totally separated from the rest, so that one can also
be processed even though it's further down in your preferred order :)
Comment 11 Ruediger Pluem 2006-04-23 10:54:37 UTC
(In reply to comment #10)
> I'll start with the first three patches so you can start processing stuff at
> your end, and if there are things I have to do differently I don't have to
> respin so many patches.

Makes sense. Thanks for doing so. I will go through them. I already committed
the first one to trunk as r396252
(http://svn.apache.org/viewcvs?rev=396252&view=rev)

Comment 12 Joe Orton 2006-04-24 13:13:17 UTC
The LFS support can be done by just using apr_brigade_insert_file() - that code
was refactored out already.
Comment 13 Joe Orton 2006-04-24 13:15:26 UTC
Plus the string->off_t conversion can be done using apr_strtoff() rather than
sscanf.
Comment 14 Niklas Edmundsson 2006-04-24 13:21:26 UTC
(In reply to comment #12,#13)
> The LFS support can be done by just using apr_brigade_insert_file() -
> that code was refactored out already.

Ah. And I thought that server/*.c was a good example of how to do things,
silly me ;)

> Plus the string->off_t conversion can be done using apr_strtoff() rather than
> sscanf.

Good point. That code was cut&paste from mod_mem_cache.

In general, it seems to me that there are a lot of code in httpd that would
benefit from an update to use current/recommended functions/API/etc ...
Comment 15 Niklas Edmundsson 2006-04-27 10:56:21 UTC
The more I look at mod_disk_cache the more I find that needs revamping to solve
the larger problems.

I'll take the whole mod_disk_cache-discussion to the devel maillist when we have
ironed out all problems, and then we'll se how to proceed regarding merging patches.

However, the r->filename issue still has to be fixed. We have fixed it in
mod_disk_cache, but someone should take a look at fixing it in mod_mem_cache
too, it should be a pretty trivial fix.
Comment 16 Niklas Edmundsson 2006-09-14 08:59:34 UTC
Created attachment 18860 [details]
httpd 2.2.3 - mod_disk_cache jumbo patch - lfs/diskformat/read-while-caching etc.

The condensed version of what this patch does is the following:
* Implement Large File Support (LFS).
* Realise that files are files, by:
   - copy files instead of reading them into memory or eat your mmap
     space (depending on whether you have mmap enabled or not). This
     caused segfaults on 32bit machines when caching LFS-files.
   - cache files and URLs pointing to files separately, so if you have
     a bunch of vhosts allowing access to a DVD image you'll only get
     one copy in cache.
* Only initiate one caching session per file, originally all sessions
   initiated separate caching sessions until the file was cached.
* When a file is being cached, other clients are served the data that has
   been cached so far.
* Don't throw away a painfully cached file just because the connection
   was aborted, since it's a file the result is valid anyway.
* Allow the caching of a file to happen in the background, the
   original behaviour is to cache the whole file and then return data
   to the client. This is bad for DVD-isos and other large files.
* Rerarrange the on-disk structure to allow the above items. The big
   improvement here is to cache both header and body in the same file,
   the original has separate files which are a hassle to do atomic
   operations on without locks and also makes cleaning the cache
   unneccesarily hard.
* As a side effect of the disk format change, %f in LogFormat now
   works.
* Add option to not try to remove directories in the cache structure.
* Lots of code reorganisation and error handling to make it all
   possible.
Comment 17 Niklas Edmundsson 2006-09-14 09:01:07 UTC
Created attachment 18861 [details]
mod_disk_cache LFS-aware config

LFS-config part of the jumbo patch.

This patch makes it possible to configure mod_disk_cache to cache
files that are larger than the LFS limit. While at it, I implemented
error handling so it doesn't accept things like "CacheMinFileSize
barf" anymore.
Comment 18 Graham Leggett 2006-09-14 12:37:20 UTC
Ok, the mod_disk_cache LFS-aware config patch is easily digestible. The jumbo
patch is too big - is the jumbo patch the sum of the patches that were posted in
April, or is the jumbo patch something new?
Comment 19 Niklas Edmundsson 2006-09-14 13:33:13 UTC
The jumbo patch is what our production code looks like vs. 2.2.3. It's intended
as reference and for those who wants to know where I'm heading with all this.
It's not intended for merging as is.

The old attachments were work in progress before I realised how sad the state of
mod_disk_cache were.

The "LFS aware config"-patch is for trunk, identical to the one posted to the
dev mailing list and should be suitable for merging. It doesn't break any
existing valid configurations.
Comment 20 Niklas Edmundsson 2006-09-26 08:52:01 UTC
Created attachment 18916 [details]
mod_disk_cache working LFS (filecopy)

This patch depends on "mod_disk_cache LFS-aware config" and is for trunk. It's
a subset of our jumbo-patch.

It makes caching of large files possible on 32bit machines by:

* Realising that a file is a file and can be copied as such, without
  reading the whole thing into memory first.
* When a file is cached by copying, replace the brigade with a new one
  refering to the cached file so we don't have to read the file from
  the backend again when sending a response to the client.
* When a file is cached by copying, keep the file even if the client
  aborts the connection since we know that the response is valid.
* Check a few more return values to be able to add "successfully" in
  the appropriate places above.
Comment 21 Graham Leggett 2006-09-26 16:30:31 UTC
Attachments 18916 and 18861 have been applied.
Comment 22 Niklas Edmundsson 2006-10-05 14:05:38 UTC
Created attachment 18968 [details]
Rearrange load/store to not depend on temporary files.

The goal of this patch is to do away with the
write-to-file-then-move-in-place mentality.

This is done by rearranging the load (open_entity) and store (store_headers)
into multiple functions to get better overview, and thus enable the
possibillity to detect partial reads and other issues that can happen.

This also introduces a disk format change, since we need the size of the
body in the header to be able to know what's happening.

The patch is for trunk.
Comment 23 Niklas Edmundsson 2006-10-05 14:08:36 UTC
Created attachment 18969 [details]
Read While Caching - don't stall clients accessing a file being cached.

Implements Read While Caching by introducing a DISKCACHE bucket which morphs
into FILE buckets as the file is being cached.

The setaside-function is incomplete, but those cases doesn't seem to happen in
normal production use for us.

This is copied as is from our production code, and is rock solid for us.
Comment 24 Niklas Edmundsson 2006-10-08 10:37:29 UTC
Created attachment 18979 [details]
Fixups for the load/store-patch.

I discovered a few misses, mostly not NULL:ing fd pointers when closing
them, missing close/flush, and some unneccessary code duplication instead of
calling the right helper in replace_brigade_with_cache().

The misses are in the load/store-patch, so I would recommend applying this
before reviewing the results even though it's generated from a file with the
read-while-caching patch applied.
Comment 25 Niklas Edmundsson 2006-10-08 11:03:34 UTC
Created attachment 18980 [details]
Do background copy of a file while caching

This patch implements copying a file in the background so the client
initiating the caching can get the file delivered by read-while-caching
instead of having to wait for the file to finish.
Comment 26 Niklas Edmundsson 2007-01-17 01:56:34 UTC
Created attachment 19418 [details]
httpd 2.2.4 - mod_disk_cache jumbo patch - lfs/diskformat/read-while-caching etc.

This is an update of the mod_disk_cache jumbo patch.

It's been running for a couple of months on ftp.acc.umu.se now, so it's fairly
stable.

Highlights from previous patch:
* Reverted to separate files for header and data, there were too many corner
cases and having the data file separate allows us to reuse the cached data for
other purposes (for example rsync).
* Fixed on disk headers to be stored in easily machine parseable format which
allows for error checking instead human readable form that doesn't.
* Attaching the background thread to the connection instead of request pool
allows for restarts to work, the thing doesn't crash when you do apachectl
graceful anymore :)
* Lots of error handling fixes and corner cases, we triggered most of them when
our backend went bezerk-go-slow-mode.
* Deletes cached files when cache decides that the object is stale for real,
previously it only NULL:ed the data structure in memory causing other requests
oto read headers etc.

Known issues:
* htcacheclean builds but is broken.
Comment 27 Niklas Edmundsson 2007-07-27 14:06:44 UTC
Created attachment 20558 [details]
httpd-2.2.4 - mod_disk_cache jumbo patch - lfs/diskformat/read-while-caching etc.

httpd 2.2.4 - mod_disk_cache jumbo patch - lfs/diskformat/read-while-caching
etc.

A snapshot from 20070727 of our mod_disk_cache jumbo patch and some assorted
additional patches that's needed for stability. We've been running this for a
couple of months on ftp.acc.umu.se, they have survived Debian/Ubuntu/Mozilla
releases gracefully.

This version plays well with other entities using/updating the cache. We are
using a open()-wrapper in combination with rsync which lets rsync utilise the
cached bodies, and also cache files.

This patch is provided mostly as a one-patch solution for other sites that
wishes to use these mod_disk_cache modifications.

Highlights from previous patch:
* More corner case error fixes, most of them triggered by Mozilla releases.
* Greatly reduced duplicated data in the cache when using an NFS backend by
hashing the body on the source files device and inode when available. HTTPD has
already done the stat() of the file for us, so it's essentially free.
* Tidied up the handling of updated files, only delete files in cache if
they're
really obsolete.
Comment 28 Niklas Edmundsson 2007-10-21 01:42:55 UTC
Created attachment 21016 [details]
httpd-2.2.6 - mod_disk_cache jumbo patch - 2.2.6 version

Adaptation of the patch to httpd 2.2.6. Includes the fixes made to the original
version of mod_disk_cache made in httpd 2.2.6.

It has survived a Ubuntu-release, so it's fairly stable.
Comment 29 Rafael Pereira 2008-02-13 05:52:57 UTC
Created attachment 21519 [details]
FIX htcacheclean for mod_disk_cache jumbo patch - 2.2.6 version

This patch fixes htcacheclean, to be compatible with LFS cache structure. It
must be applied after patch 21016: httpd-2.2.6 - mod_disk_cache jumbo patch -
2.2.6 version
The -i option still not working.
Comment 30 Kris Beevers 2008-03-25 13:32:02 UTC
Created attachment 21715 [details]
FIX corruption in near-simultaneous requests of uncached files for mod_disk_cache jumbo patch - 2.2.6 version

This patch addresses an issue when (a) mod_proxy is in use; and (b) two requests for the same uncached file arrive almost simultaneously.  Data is proxied initially, but upon attempting to cache the headers, a conflicting header cache is discovered; this results in a later call to replace_brigade_with_cache.  This patch modifies the behavior of replace_brigade_with_cache (via changes to recall_body) to account for bytes already proxied to the client.
Comment 31 Niklas Edmundsson 2008-06-14 10:38:41 UTC
Created attachment 22127 [details]
httpd-2.2.9 - mod_disk_cache jumbo patch - 2.2.9 version

Adaptation of the patch to httpd-2.2.9.

Includes the submitted fixes for:
"FIX htcacheclean" - Since we use a script that looks at atime we don't use htcacheclean. The fix looks sane though.

"FIX corruption in near-simultaneous requests of uncached files" - We're not hitting this, so it probably only affects usage in combination with proxies.

Major additional fixes:
- Adapt to recent APR sub-second file timestamps, meaning that we have to truncate to whole-second granularity when comparing http timestamps with file timestamps.
- Be a tad more clever when trying to detect corrupted files (commonly caused by a machine using xfs crashing). It's perfectly valid to have the consumed size smaller than the actual size, for example filesystems with compression (SUN ZFS). Also, don't check consumed size on new files since for example ZFS only updates it when data is commited to disk.
Comment 32 Graham Leggett 2013-04-30 16:33:42 UTC
Is there any news about an updated patch for httpd v2.4?
Comment 33 William A. Rowe Jr. 2018-11-07 21:08:51 UTC
Please help us to refine our list of open and current defects; this is a mass update of old and inactive Bugzilla reports which reflect user error, already resolved defects, and still-existing defects in httpd.

As repeatedly announced, the Apache HTTP Server Project has discontinued all development and patch review of the 2.2.x series of releases. The final release 2.2.34 was published in July 2017, and no further evaluation of bug reports or security risks will be considered or published for 2.2.x releases. All reports older than 2.4.x have been updated to status RESOLVED/LATER; no further action is expected unless the report still applies to a current version of httpd.

If your report represented a question or confusion about how to use an httpd feature, an unexpected server behavior, problems building or installing httpd, or working with an external component (a third party module, browser etc.) we ask you to start by bringing your question to the User Support and Discussion mailing list, see [https://httpd.apache.org/lists.html#http-users] for details. Include a link to this Bugzilla report for completeness with your question.

If your report was clearly a defect in httpd or a feature request, we ask that you retest using a modern httpd release (2.4.33 or later) released in the past year. If it can be reproduced, please reopen this bug and change the Version field above to the httpd version you have reconfirmed with.

Your help in identifying defects or enhancements still applicable to the current httpd server software release is greatly appreciated.