Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
create separate directories for each cache bins to overcome number of files in a single directory and increase performance.
Comment | File | Size | Author |
---|---|---|---|
#18 | 1211166-18-1.patch | 13.54 KB | roderik |
#18 | 1211166-18-2.patch | 11.85 KB | roderik |
#17 | 1211166-17-1.patch | 13.32 KB | roderik |
#17 | 1211166-17-2.patch | 11.79 KB | roderik |
#9 | 1211166-9-3.patch | 12.76 KB | roderik |
Comments
Comment #1
W32 CreditAttribution: W32 commentedthis is a good proposition.
Comment #2
vinoth.3v CreditAttribution: vinoth.3v commentedadding these lines to filecache.inc works great to me.
Comment #3
lismail CreditAttribution: lismail commentedIt works with me too. However, in the long run I think better to have it done on settings.php, just like cacherouter or memcache module do.
I will try and share it.
Comment #4
W32 CreditAttribution: W32 commentedThis proposition is not compatible with filecache_cron() function.
Does anybody know where DrupalFileCache::delete_expired() is called ?
Comment #5
W32 CreditAttribution: W32 commentedDear ogi,
can you realize this proposition ? Because, if your module installed with entitycache module than filecache folder is very huge! Subdividing it on sub folders by bins is good solution.
Comment #6
ogi CreditAttribution: ogi commentedAbility to have separate directory for any cache bin is definately needed. I didn't consider as important so I postponed it but it looks like beta2 have to include it, and it will.
Comment #7
roderikI dived into the code to see how to do this best, and ended up introducing some helper functions, and getting rid of the 'prefix' property & all 'chdir' commands. This works for me.
(I guess it's functionally equivalent to what is proposed above, but I needed to dive into the code for some other stuff, which is coming up later.)
But while looking at this, I also discovered a little bug. The clear() command will currently also delete cache entries from other bins, since everything is in the same directory. This patch fixes the bug.
---
There's a separate patch introducing only whitespace changes, to make it adhere to coding standards. I don't want to be pedantic, but I was afraid that otherwise my patch would not be readable because of too many lines.
Comment #8
roderik....and I thought that this still would not be enough. Because e.g. the directory for the cache_field bin will still be huge.
Actually, that should be a multi-level directory with ':' as the separator.
But.... that doesn't work well for other cache bins, which have relatively few entries. It's not useful to split those into a multi-level directory.
So... I hacked different 'encoding methods' into the encode_cid() function. Plus a set_encoding() method, which defaults to setting varying encoding methods for different bins.
See 1211166-7-3.patch.
1211166-7-ALL.patch is patches 1/2/3 rolled into one, in case someone just wants to update their sources to the latest work I did.
Comment #9
roderikHere's an extension of what I posted in #8.
This adds more comments, and more 'encoding methods' - which are used by default for cache_entity_* bins. (The site which I'll install this on, has a lot of comments/nodes, and I'm afraid of storing them all in one flat directory.)
(FYI: I provided a default for 'cache_entity_comment' which seems sane to me for most sites, but I want something else on my site - and successfully tested the hook_init() override which is documented in the patch's comments.)
Status:
Comment #10
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedError when apply patch
git apply 1211166-9-ALL.patch
1211166-9-ALL.patch:48: space before tab in indent.
$hint = $t('%dir is a directory but PHP cannot write to it.', array('%dir' => $dir));
1211166-9-ALL.patch:313: space before tab in indent.
return FALSE;
1211166-9-ALL.patch:346: space before tab in indent.
($cache_flush && $cache->created < $cache_flush))) {
1211166-9-ALL.patch:399: space before tab in indent.
$this->delete_expired();
1211166-9-ALL.patch:406: space before tab in indent.
$this->delete_expired();
error: patch failed: filecache.inc:12
error: filecache.inc: patch does not apply
Comment #11
roderikAh, more whitespace errors. ogi uses tabs in his file and sets his tab width to 8, which does not work well for my editor (or Drupal Coding Standards). I thought I squashed all tabs in 1211166-7-1.patch, but didn't.
I'll take care of that later. For now, use --ignore-whitespace flag for git apply.
Also:
I discovered filecache_cron() is 'broken': since cache bins are now in their own directories, and it checks the 'main' dir, files never expire. You can still apply the patch if you don't mind that. I'll try to get to this, soon.
Comment #12
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedstill doesn't apply
if I use git apply I get the same error as in #9
patch -p0 --ignore-whitespace < 1*ALL*patch
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/filecache.inc b/filecache.inc
|index 39d1c9d..3757724 100644
|--- a/filecache.inc
|+++ b/filecache.inc
--------------------------
File to patch: ./filecache.inc
patching file ./filecache.inc
Hunk #1 FAILED at 12.
1 out of 8 hunks FAILED -- saving rejects to file ./filecache.inc.rej
Comment #13
ogi CreditAttribution: ogi commentedroderik, thank you for all your efforts :-) Separate directories are in the TODO list since the beginning.
I'm thinking about optionally adding a new directory level like /var/spool/postfix queues.
Comment #14
roderikSwamped now.
I'll have some new patches for review soon. Planning to take the "separate styles of directory structures" (a.k.a. patches with a -3 behind them) into a separate issue, because it's gotten larger than I thought.
Comment #15
Danny EnglanderI tested the "all" patch in #9 now and indeed File Cache now creates separate directories but I am still plagued by this error which I am sure is probably something different. Note - fixing typo in issue title.
Comment #16
ogi CreditAttribution: ogi commentedI just want to point out that splitting into directories will decrease performance instead of increasing it. All modern file systems implement directory hashing and it's faster to lookup in one giant directory than in many smaller directories. Of course, for humans it's still more convenient to have this split so it will be part of filecache.
Comment #17
roderikNew patches.
Since this has gotten bigger, I will split out the '-3' portion into a separate issue. Remark #16 can be discussed there. This issue then will be purely what the title said: one different directory per cache bin. This is definitely necessary, because right now the different bins are (potentially) overwriting and deleting each other's files!
This patch has grown larger since #9... in hook_cron(), expired files were just deleted from the one large 'global' cache directory. Since this is not the case anymore, hook_cron() should be able to know which cache directories are actually being used. And Drupal Core doesn't implement a 'registry' of cache bins.
I could have gone for "assume all directories within the 'global parent' are cache bin directories", but it seemed better to implement a kind of 'registry' in the FileCache module.
The deletion code used by hook_cron() has been moved into DrupalFileCache::delete_expired(). Note that followup patches will remove more code duplication (by removing delete_flushed()) - but this was not part of the functionality for this issue.
Rehash:
* patch 1 makes the code adhere to Drupal coding standards because otherwise patch 2 would get unreadable. But I also added something else: watchdog logging for delete_expired() and delete_wildcard(). This should be useful for testing stuff later on. delete_flushed() is unfortunately called way too often to add the watchdog call to.
* patch 2 is the actual patch that fixes the issue. It also has some other general things: checks for empty $bin on __construct() and for empty $cid on get() / set(). I encountered these several times while testing.
Comment #18
roderik...and some minor code reshuffling that would otherwise needlessly end up in #1488870. I'm getting really tired of working with 4 interdependent patches simultaneously...
Comment #19
roderikThe '-3' patch from #9 has been moved to #1488976: Possibility to have directory trees.
Comment #20
ogi CreditAttribution: ogi commentedPHP files are untabified in d3a4614.
Comment #21
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedthis was committed: http://drupalcode.org/project/filecache.git/commit/d3a4614
Comment #22
vinoth.3v CreditAttribution: vinoth.3v commentedAre you sure?
last commit on 2012-07-25 ?
Comment #23
vinoth.3v CreditAttribution: vinoth.3v commentedComment #24
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedCheck the link. It's in the commit log
Comment #25
vinoth.3v CreditAttribution: vinoth.3v commentedIssue number is there, Not the patch!
Comment #26
4kant CreditAttribution: 4kant commentedI´m having 75 GB of data in the cache directory.
Since this issue is pretty old: has there been any progress - am I missing something? Perhaps any configuration steps?
Thanks!
Comment #27
cockers CreditAttribution: cockers commentedWill this feature be committed?
Comment #29
ogi CreditAttribution: ogi commented