Add ability to exclude bins (tables) from memcache

Out of curiosity, what is the problem with cache_form? And why doesn't cache_page make sense?

We have a need for what we call a 'durable' bin, that doesn't get cleared during deployments. For that, we have a separate memcached daemon cluster running and map a bin to that.

subscribe

Loving this idea ... testing on one of my sites right now ... so far so good

... note that using this patch (and excluding cache_form from being memcachified) has completely alleviated my issue (#962578: If cron runs while a form with an imagefield is open, error: "... This form was missing from the server cache..." on img upload ) which IMO confirms that #966114: cache_clear_all ignores cache "expire" attribute is, in fact, a legitimate problem.

I'm running this in a large production environment without issue.

sweet

I'm a little confused about something. Why would you set up a memcache bin, and then exclude it?

What are the gains/trade-offs for simply not specifying a memcache bin versus specifying a bin but excluding it?

Thanks.

emacken check out this from teh README.txt:

$conf = array(
  'cache_inc' => './sites/all/modules/memcache/memcache.inc',
  'memcache_servers' => array('localhost:11211' => 'default',
                              'localhost:11212' => 'default',
                              '123.45.67.890:11211' => 'default',
                              '123.45.67.891:11211' => 'cluster2',
                              '123.45.67.892:11211' => 'cluster2'),

  'memcache_bins' => array('cache' => 'default',
                           'cache_filter' => 'cluster2',
                           'cache_menu' => 'cluster2'),
);

With this setup, anything that would have normally gone into the cache_form, or cache_block, etc... tables will autmatically be sent to the "default" cluster even though neither are specified in this setup. By explicitly excluding cache_form (or whatever you want to exclude) you are telling memecache to follow this algorithm when cache_set() is called:

1. Check if the bin for the item to be cached is specified
2. IF NO: cache it in the "default" bin
3. IF YES: cache it in the specified bin unless that bin is called "none" in which case use the appropriate Drupal cache table instead of memecache

Does that make sense?

yes, thanks, didn't realize that not specifying a bin would send the cache sets to the default.

I had a minor spelling error in #8

A suggestion regarding the committed code patch:
Could you please move the whole _cache_get, _cache_set and _cache_clear_all function into a separate file? The reason is simple: Many users probably need to exclude certain bins from memcache and handle them another way. (Cacherouter is IMHO no option, as it provides a very good idea, but neglected and inefficient engines support.)

Having the alternative engine functions in a separate file would make custom cache engines besides memcache API way easier to maintain. And users not requiring this functionality in this patch, could just comment it and save some CPU cycles per request.

I get the feeling this is an attempt to mask another bug -- if cache_forms is not working in memcache, why? Is moving cache_menu into the database even an option for large Drupal websites? I also don't like how this was implemented, cutting and pasting code from core. Inevitably as core changes this file will get out of sync. If this logic has to stay in, I agree it should be moved into an include file as requested in #13.

Possibly related:
#978310: drupal.org forms expire way too early

@robert
Regarding the patch - I just moved the functions to seperate files and put in a require_once. This could maybe made nicer and use a better name then 'none'. So I am not uploading it.

One thing I came across, I would suggest to also add the following patch, which moves the cache_clear_all logic out of the db-specific function, by simply slightly postponing the specific functions:

Index: memcache.inc
===================================================================
--- memcache.inc        (revision 2166)
+++ memcache.inc        (revision 2177)
@@ -152,11 +152,6 @@
  *   match. If '*' is given as $cid, the table $table will be emptied.
  */
 function cache_clear_all($cid = NULL, $table = NULL, $wildcard = FALSE) {
-  // Handle database fallback first.
-  $bins = variable_get('memcache_bins', array());
-  if (!is_null($table) && isset($bins[$table]) && $bins[$table] == 'none') {
-    return _cache_clear_all($cid, $table, $wildcard);
-  }

   // Default behavior for when cache_clear_all() is called without parameters
   // is to clear all of the expirable entries in the block and page caches.
@@ -165,6 +160,13 @@
     cache_clear_all('*', 'cache_page', TRUE);
     return;
   }
+
+  // Handle database fallback first.
+  $bins = variable_get('memcache_bins', array());
+  if (!is_null($table) && isset($bins[$table]) && $bins[$table] == 'none') {
+    return _cache_clear_all($cid, $table, $wildcard);
+  }
+
   if (empty($cid) || $wildcard === TRUE) {
     if ($cid == '*') {
       $cid = '';

and in the specific _cache_clear_all:

Index: db_fallback.inc
===================================================================
--- db_fallback.inc     (revision 2207)
+++ db_fallback.inc     (working copy)
@@ -142,14 +142,6 @@
 function _cache_clear_all($cid = NULL, $table = NULL, $wildcard = FALSE) {
   global $user;

-  if (!isset($cid) && !isset($table)) {
-    // Clear the block cache first, so stale data will
-    // not end up in the page cache.
-    _cache_clear_all(NULL, 'cache_block');
-    _cache_clear_all(NULL, 'cache_page');
-    return;
-  }
-
   if (empty($cid)) {
     if (variable_get('cache_lifetime', 0)) {
       // We store the time in the current user's $user->cache variable which

Subscribing. Very interested in this.

Bumping up the priority, as this is currently blocking a release. There have been some performance related fixes that I'd like to release, but not until this patch is either backed out (very tempting), or the logic is moved into an include file.

> I also argue that disk space is cheap and memory is expensive

I see two problems with this argument: 1) your patch sends a configurable number of caches into the database which is also going to end up in memory at least partially, and 2) disks are slow while memory is fast -- if you're deploying the memcache module you're doing it because you want to improve performance. While more expensive than disk space, memory really isn't that expensive these days.

Could 'none' be changed to something else, e.g., database, core, etc, to avoid confusion? It seems 'none' is used in cacherouter to literally mean no cache -- http://drupal.org/node/797610 .

re #20: Please do not back this out ... this change solves a whole bunch of issues that occur when cache is cleared while a form is being filled out. The form cache really is a special case in Drupal and this lets me treat it as such.

> this change solves a whole bunch of issues that occur when cache is
> cleared while a form is being filled out. The form cache really is a
> special case in Drupal and this lets me treat it as such.

Where is the bug report for this? Have you tried the latest development snapshot to confirm whether or not it's necessary to split out cache_form? If it's still true with the latest development snapshot, update the bug report with steps on how to duplicate this, and we'll fix it properly.

The code looks good. Maybe the _cache_* functions in database.inc should be renamed to memcache_database_cache_* instead to avoid name collisions -- http://drupal.org/coding-standards#naming .

@robertDouglass
Yes, collisions are not likely. Just might be a good idea to have module name as a prefix.

@jeremey: just to answer your question from #23, the bug report can be seen here: #966114: cache_clear_all ignores cache "expire" attribute

@robertDouglass: Thanks for keeping this on your radar!

@bleen18 that issue should now be fixed in the latest development snapshot, and hence was closed.

@robert:

> "which is also going to end up in memory, at least partially" - how do you mean that?

Databases use memory for performance. Especially if using innodb, then it's going to load all indexed columns into RAM.

If you have a cache bin that is abnormally large and is pushing other cache bins out of memory, you should move it to a dedicated memcached instance so it only affects itself. Memcache uses LRU to evict pages so the most popular items will live in the cache.

> I agree that hunting down the weirdness with cache form is a good idea, but there's
> a 90% chance that it is not a memcache problem, rather a core architecture problem.

What core architecture problem are you talking about?

@robert: I believe the goal was to make this database.inc file configurable via settings.php, much like 'cache_inc' is configurable in core.

Specifically, this line:

+require_once 'database.inc';

Should look more like this line (from bootstrap.inc):

      require_once variable_get('cache_inc', './includes/cache.inc');

Subscribe. I've been having problems with cache_form in memcache causing form_build_id tokens to become invalid and return empty data from CCK "Add another" buttons. This patch looks promising.

Though this issue is now somewhat old and already done with, I want to back up something robertDouglass pointed out:

Caches should be disposable information. You should be able to run the website (albeit inefficiently) without any caching. cache_form is essential (as I understand it) to multi step forms, and Drupal actually breaks if it isn't available. That's the architectural problem I meant.

That's right. The cache_form table is not really a "cache" at all. It's a temporary storage of in-progress forms. Any form that utilizes multi-step through $form_state['storage'] or #ahah will automatically make entries into the cache_form table. The problem with this table is that it's content's aren't "temporary" in the manner that they can be disposed of at any time. For example let's say you start a node at node/add/image, using ImageField as an upload. If someone flushes the caches while you're typing out the title to your image, then you try to upload after the caches have been cleared, you'll find that it's impossible. ImageField will helpfully give you the error, #795004: An unrecoverable error occurred. This form was missing from the server cache. Try reloading the page and submitting again..

Now moving cache_form into the database instead of memcache still does not entirely solve this problem, but it does prevent Memcache from pruning in-use "cache" entries that may contain a form because it has used up all its space or from restarting memcache causing forms to break. So kudos, thanks for the patch and I'm looking forward to using 1.8+.

for Drupal 7, someone managed to exclude a bin from memcache and have it fall back to the db with these lines in settings.php :

/**
 * Settings for memcache
 */
$conf['cache_backends'][] = 'sites/all/modules/contrib/memcache/memcache.inc';
$conf['cache_default_class'] = 'MemCacheDrupal';
// The 'cache_form' bin must be assigned no non-volatile storage.
$conf['cache_class_cache_form'] = 'DrupalDatabaseCache';
$conf['memcache_servers'] = array(
  'memcache.local:11211' => 'default',
);