Posted by robertDouglass on April 16, 2009 at 4:31pm
| Project: | Memcache API and Integration |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Issue Summary
memcached supports data values up to 1M but not larger. Views, for example, regularly tries to cache values larger than 1M and this breaks. Size detection should be implemented so that we never try to write something to memcached is too large.
Comments
#1
I'm not sure it's possible to measure the size of a variable accurately - or at least I've not seen code that does that.
IMO this is a bug in Views (and CCK, theme registry, variables, Field API and all the other places that have massive individual cache items), but it'd be good to document it at a minimum.
#2
http://groups.drupal.org/node/116764
http://groups.google.com/group/google-appengine-python/browse_thread/thr...
I'd like to see us catch when objects are too large (memcache should throw some kind of error), and log this. If we throw enough errors around, people should then run memache with -I, or file bug reports against the modules with too large items.
#3
Subscribing.
#4
Any way for memcache catch the error and drop it in watchdog? Have a special call just for this occasion, kinda like this one I made #1011000: Catch & report errors thrown by $memcache->connect.
#5
I'd love to see this happen, as I think >1MB cache sets can potentially be a nasty performance hit, memcache or not (it was for us).
Although I cannot replicate the use case scenario for #1011000: Catch & report errors thrown by $memcache->connect. and don't quite have that patch running as intended (no error message is returned in the Exception object), my initial feeling is that this will be difficult to do.
As far as I can tell,
dmemcache_set()calls the set() class function, which maps to a C equivalent. I skimmed the c code inphp_memcached.cand AFAICT the related function (php_memc_handle_error) either returns 0 or -1. So no specific error message will ever bubble up to PHP and Drupal for us to use in Watchdog.Besides this, my tests show that the memcached log only logs a message about the item size being to large, if you start the daemon with
-vv. It then writes something likeWriting an error: Too large..#6
Right, memcache fails silently at least as far as PHP is concerned for this.
I have seen posts around the internet where people run strlen on the serialized value, and thrown an error if it's over a certain length. Could possibly live with this as a debug option but not as standard behaviour, not sure how that would play with the fact that we try to compress the items too though.
I would rather see people's effort going into fixing some of the root causes of this - there are pending patches at http://drupal.org/project/issues/search/drupal?issue_tags=memory - many of those have not had much review.
#7
subscribe
memcache do not store large items even if memcached daemon started with -I 32M (store items with <32MB)
so it is a code behaviour
#8
the same on 7.x
#9
In accordance with #6, I've started issuing watchdog warnings on my installation to help me identify when cache sets fail. See attached patch for 6.x.
There is still the mentioned issue about cache item size when using compression, which I have not adressed.
This procedure may be the best we can get, since memcached does not let us know specifically when a cache_set fails due to item size, just that it fails.
#10
Thanks for the patch achton. Works for me:
http://www.localhost:8080|1312474790|memcache|127.0.0.1|http://www.localhost:8080/devel/php|http://www.localhost:8080/devel/php|1||Cache set attempt failed. Key: cache_size_test. Cache bin: cache. Item size (uncompressed): 3385 KB.Re-roll without trailing whitespace attached. Also, bumped status to error, seems like an error when a cache set fails, though I guess that could be debated.
#11
Just as an FYI, this patch has been very informative. We were sort of assuming cache_set was working most of the time, but our logs are pilling up with fails, some of which don't appear to be caused by the object size.
#12
Note to self, update patch to report error code when possible:
http://www.php.net/manual/en/memcached.getresultcode.php
#13
Re-rolled patch, changed watchdog call to look more like other calls in core, removed em tags, etc.
Primary change added the error message if we have Memcached.
#14
Depending on where we are in the bootstrap, "t()" may not yet be defined. I re-rolled the watchdog error message patch to check whether "t()" exists before using it.
#15
Let's drop the t() usage altogether here and just log in English. I don't see this ever getting translated. Alternatively could use get_t().
#16
Unfortunately, this apparently can occur at times when module.inc hasn't been included yet, so it errors when watchdog tries to run module_implements. Two options - not log if module_implements function does not exist, or require_once module.inc so that we have the required functions. I'm leaning toward the latter - do you have a preference catch?
#17
You can use a shutdown function for watchdog. This patch was committed to memcache #1011000-9: Catch & report errors thrown by $memcache->connect.
#18
mikeytown2, thanks for the suggestion. updated patch attached.
#19
Subscribing.
If we could log when the 1M limit is restricting caching, it just might solve (or help pinpoint) huge performance problems that I'm seeing on a complex site (or several).
It seems in our case the problems are caused by a few key cache component that never make it to memcache (and as you know, the database cache is not used either), it's slowing the site more than I like to admit. And has been for a long while.
Several serialized cache blobs are over 1M, probably even when gzipped. For example:
cache
***********
theme_registry:main_theme_name [BLOB - 8.1 MiB] 0 1316556483 1
theme_registry:admin_theme_name [BLOB - 7.8 MiB] 0 1316553845 1
cache_menu
***********
links:admin_menu:tree-data:abbcf0407c9c6cc16599da6... [BLOB - 1.5 MiB] 0 1316534351 1
cache_content
***************
content_type_info:fi [BLOB - 3.4 MiB] 0 1316538815 1
#20
The patch adds these warnings within dmemcache_set and lets it return the error status, but the problem is exacerbated by the fact that set() discards that status by invoking dmemcache_set() without assigning its result anywhere. The commit mentioned in #1011000: Catch & report errors thrown by $memcache->connect. appears not to touch this part of the code.