cache_menu: data blobs identical and huge;

I think that the main problem is 1. and that there's no cron call to actually clear the menu table. The second part will be discussed at http://drupal.org/node/226728.

We might resolve this other issue by calling drupal_flush_all_caches. In that case this issue would also be fixed since that function calls menu_rebuild which flushed the menu cache.

Not yet marking this a duplicate.

every rendered menu on every page will create a cache entry. The cache is cleared as needed, so I don't think the expiry time is wrong.

So yes, on a default install you'll get 3 entries per page.

D6 cache_menu is only per page - NOT per user. all the access checking, localization, etc, is done dynamically used the cached data.

so, for d.o assuming 200,000 nodes + 150,000 user profiles, at 80 kB per page, that's like 28 GB?

There is no reason you'd want to clear this on cron - the data should not get stale. Since the site is getting spidered, you'd just create load building it all again.

Unfortunately, there's no way I can run d,o with a cache table that big, be it 80 or 28 GB.

My initial calculation was based at 250k nodes, 250k user profiles + the relevant "edit" pages. The "edit" entries might not exist all the time, but there are plenty of other pages.

so, I guess we never did this calculation, but I'd suggest an option for 6.x (At least) is an option in the performance UI to toggle off menu caching?

here's a patch against 6.x, also applies to HEAD with small offset.

This exposes the option via the UI, but we could also make this a silent option (e.g. advanced configuration in settings.php only)

afaik know bugs get fixed in 7.x then backported. Correct me if wrong.

@catch - sure should deal with HEAD first

here's a radically different, and perhaps better, patch. Implements a 2-level caching scheme so we don't duplicate the stored data, but also don't sacrifice the advantage of caching.

here's a patch that covers book module too, and has slightly better logic in the cache_set()

I tested this on a clean-ish site comparing cache_menu entries with and without the patch applied. Without, I get three entries - 26KB for navigation, 47 bytes for my empty secondary/primary links. With the patch applied I get three 36 byte entries instead. So it looks along the right lines to me.

Very good trick! But the cache tables should never be queried directly, always use cache_get because cache.inc is pluggable (for eg. memcached).

right - also CNW since the new cids don't jive with the cache-clearing code. Both problems should be easily fixable, however.

Thanks chx, new patch attached.

Hadn't seen Peter's comment, I changed the CIDs in the cache invalidation code to match the new ones.

I think for D7 we will want to improve the cache API with something like cache_entry_exists(), but that can come later. It seems silly to have to fetch 80kB of data to see that the row exists in the table.

The cache clearing still needs to be fixed more, I'll try to re-roll later today.

ok, here's a working version. Note that ALL cids are back to the form 'links:'. $menu_name . ':something', so our cache clearing code can still work properly using a wildcard against 'links:'. $menu_name . ':'

Tested the patch in #20. Testing scenario:
* * D7 HEAD with all cache tables cleared.
* * Created 10,000 nodes via devel.module in all content-types.
* * Promoted all nodes to 1
* * Ran wget against the localhost site.

Without the patch, the cache_menu table was at 66MiB after the test. With the patch, the result was only 1MiB.

Success!

made nodes with Devel and spidered ~450 pages on my localhost with wget (in a race w/ Senpai)

with patch: table size = 0.50 MB

without patch: table size = 11.56 MB

Note this is a bit over-optimistic, since the menus are the same on every page, but does suggest the scale of the problem with the existing core code.

I had a hard time understanding the code (without reading up on the issue details). Can we take another pass over the code comments?

I suspect this patch might have a negative impact on performance. Is there something better that we can do for Drupal 7?

@Dries - any performance trade-off should be minimal, since it is usually only a single, simple, extra query, and for most pages we will be writing much, much smaller blobs into the DB so those queries will be faster.

For D7 I'd like to add an extra API function as suggested in #19, but since killes says d.o cannot be moved to D6 without this, I'd like to see some version in D6 and HEAD before mucking with the cache API.

I'll take another pass at the code comments.

here's a patch with improved code comments.

OK, thanks Peter. I've committed this to CVS HEAD. Should go in into Drupal 6 as well as this is a pretty bad problem.

Just checked and this still applies cleanly to 6.x

Thank you for that patch. My test site (yes, on a cheap, shared hosting ... :) got a suspension warning the next day after 6.1 upgrade, with the menu cache taking over 60% database space on its own. I can confirm this applies cleanly to 6.1 and works.

Yes - we really need this in before 6.2. Also, we should probably further examine whether to remove or limit the caching by book module - the "printer friendly" is always going to produce a unique subtree (I think) , so any site with a large set of books and which exposes printer-friendly to anonymous users (i.e. spiders) may have trouble.

OK folks... I'm a real newbie here but I have a pretty good grasp on what to do if given good instructions. I ran into this cache problem last night - I'm running Drupal 6x in a Wamp stack locally first before I commit it to my site. I was just "fishing around" and clicked the "Track" link on the "My History" page and the whole thing locked up tight. No errors or warnings, just a blank screen and a progress bar at the bottom of my browser. Being one whose a sucker for punishment, I got fed up with my browser locking up so I uninstalled the whole thing deleted the database in MySQL - fished around and found those two (huge) files and deleted them. I have since reinstalled Drupal and things are working OK.

I'm glad I found this page! But now my question is: How do I install the patch? Of all the issues I've been reading, this is a big one because I'd hate to have to do the same radical surgery on my site as I did on my local test.

I assume these are the cache files were talking about(?)

ib_logfile0: 10,240 KB
ib_logfile1: 10,240 KB
ibdata1: 18,432 KB

I believe these would be the database files containing everything that is in it; i don't know what the overhead should be, but 10MB may not be so large.

On linux you would just put the patch file in the main drupal directory, open a terminal and from within that directory issue "patch -p0 < 2-level-menu-cache-231587-26.patch". Seems there is a version of the patch utility for windows too: http://gnuwin32.sourceforge.net/packages/patch.htm, but i've never tried that, so maybe you'll need another tool (you may want to check CygWin or MinGW environments too).

DaveT777: http://drupal.org/patch has instructions for applying patches, including on windows (easiest using Cygwin).

@DaveT777 - that sounds more like a PHP memory issue (RAM) and unrelated to the bug being fixed here. Try tweaking your settings in php.ini.

Thanks for the quick response! I'll give the php.ini tweak a shot first before I look at applying the patch. Is the patch going to be integrated into newer versions of Drupal? If so, and if I don't have problems with the issue you've been discussing, then I'll wait until the new version is released. I'm an "If-it-ain't-broke-don't-fix-it" type of guy.

Thanks for the help!

DaveT777

Thanks. Reviewed and committed to 6.x too.

Will this improvement ever see the light on Drupal-5 ? Is Drupal-5 also affected by the problems outlined in this post ?

I've encountered this on upgrade to 6.1, 5.x probably has not been affected by this issue at all. Got fixed in 6.2 too.

The toggle menu cache option is not in 6.16. Did it make it into 7? I've custom patched my site to have this menu item and use it. The cache_menu table was getting out of control and it wasn't being cleared by cron. The "expire" field doesn't seem to be used.

I have similar problem on D7. Huge cache_menu table which gets bigger and bigger. Any solution for this?

My site is runing D-6.20 and it looks that the bug is still present:


mysql> select cid, length(data) 'data-len', expire,created from cache_menu;
+-----------------------------------------------------------------+----------+--------+------------+
| cid | data-len | expire | created |
+-----------------------------------------------------------------+----------+--------+------------+
| links:secondary-links:page-cid:node/10290:1 | 64 | 0 | 1305548086 |
| links:primary-links:page-cid:node/10290:1 | 62 | 0 | 1305548087 |
| links:secondary-links:page-cid:node/12992:1 | 64 | 0 | 1305548118 |
| links:primary-links:page-cid:node/12992:1 | 62 | 0 | 1305548118 |
| links:secondary-links:page-cid:node/8683:1 | 64 | 0 | 1305548149 |
| links:navigation:tree-data:c394f6d943d46b6d55b9e10d79058eb7 | 300658 | 0 | 1305548149 |


The expire field is empty. So "cron" will not clean the garbage.

Is there a working patch around?

Same here. Running Drupal 6.22 (Pressflow 6 distribution) and cache_menu @ 190MB

Using query

mysql> select sum(length(data)) 'data-len' from cache_menu where length(data)  > 100;
+----------+
| data-len |
+----------+
| 66040342 |
+----------+
1 row in set (0.20 sec)

I got that the cache_menu table (D-6.22) is growing here about 65MB in 9 hours., what is about 180MB per day.
May be people maintaining D6 core are not aware of this since this issue has been closed. So I will reopen it to get some response.

As you can see from #45, there are some large cache entries, then some very small ones.

The small cache entries exist to avoid duplicating the large cache entries - that is the fix that was committed due to this issue.

If you are concerned about the number and size of cache entries despite that fix, that is a different issue to this one - you should open a new issue (against Drupal 8 since this logic has not changed in there yet and we work in the highest affected release then backport) for that and cross-link here.

OK, I just set up a new issue against D8 containing references to this issue and the one described in 529060.
Please compare new issue in 1234830

Apologies for re-pinging but #1234830: Revert cache_menu patch removal, add a $conf setting instead (was: cache_menu: huge table size) could use the same minds that worked on the problem over here: cache_menu still growing huge and one reason why, anyway, is that a nonsense path that falls back to a real path (node/123/gibberish) gets its own entry, see Stefan Freudenberg's comment.