I noticed that after I upgraded to 5.x-1.7 that it seemed like some of my cache was being cleared on every page load.

I narrowed down the problem to this new code in cache_clear_all()

  if (empty($cid) || ($cid == '*' && $wildcard !== TRUE)) {
    // don't do anything if cid is unset. this matches the default drupal behavior...
    if ($wildcard && $cid != '*') {
      if (variable_get('memcache_debug', FALSE)) {
        // call watchdog, since you probably didn't want to flush the entire bin.
        watchdog('memcache', "illegal wildcard in cache_clear_all - not flushing entire bin. table: $table. cid: $cid", WATCHDOG_WARNING);
      }
    }
  }
  else if ($cid == '*' || $wildcard === TRUE) {
    dmemcache_flush($table);
  }
  else {
    dmemcache_delete($cid, $table);
  }

There are cases (pollfield_clear() for example in pollfield.module) that call cache_clear_all with a 3rd parameter which in this case is $wildcard. An example would be:

  $cid = 'content:'. $node->nid;
  cache_clear_all($cid, 'cache_content', TRUE);

In this case $cid is a string and is not '*'.

If this cache_clear_all() call hits the above code, then it will result in a cache flush everytime because the $wildcard parameter is TRUE even though the $cid != '*'.

I rolled back to 5.x-1.6 which has a rather simpler logic to cache_clear_all() and no problems after that.

Hope this makes sense...

Files: 
CommentFileSizeAuthor
#9 dmemcache.patch3.42 KBgaolei

Comments

firebus’s picture

that makes sense.

the logic in 1.6 is simpler, but does not correspond with drupal's core cache logic.

the problem is that if $cid has a '*' in it - either '*' or something like 'nid-1234*', and wild card is true, memcache needs to flush the whole cache, because memcache cannot handle wildcards.

can you provide an example of a specific cache_clear_all that pollfield_clear is generating which should not cause memcache to flush?

if pollfield_clear is really setting wildcard=TRUE when there is no * in cid, that's probably bad logic on their part.

however, we should be able to change our code to catch that case fairly easily...

ilmaestro’s picture

Whoa, memcache flushes the whole cache whenever you attempt to flush something with a wildcard? That is seriously not cool. I have to do wildcard cache wipes whenever a user posts new content (which if the site gets busy, will be quite frequently), but I don't want to lose everything else in my memcache...

arg.

firebus’s picture

this is a limitation of memcached itself. it does not handle wildcards when expiring keys.

you can (should) set up separate bins (run a number of memcache instances on different ports) for each cache table so that you don't throw out all the babies with the bathwater - i'm not sure if this is well documented.

i personally don't use memcache.db.inc - it may be the case that if you're using db.inc, the database cache will serve as a backup for the times when the memcache is flushed due to a wildcard.

it might be interesting, in the case of db.inc, to add an extra query to search for keys that match a wildcard, and then expire them one-by-one from memcache, to get around this problem. i'm not sure if it would really increase performance, especially in the case of a really large set of keys.

perhaps we need a third option - just store a list of cache keys in the db, and use that to handle wildcards for memcache?

ilmaestro’s picture

"it might be interesting, in the case of db.inc, to add an extra query to search for keys that match a wildcard, and then expire them one-by-one from memcache, to get around this problem."

That's exactly what I was thinking, except this "index" entry doesn't necessarily need to be in the database cache... it could be in the memcache as well.

"i'm not sure if it would really increase performance, especially in the case of a really large set of keys."

If memcache did support deletes based on a wildcard, the underlying library would still essentially be looping through everything that matches the wildcard and doing one delete at a time. I'd hope the overhead of initiated a delete of one entry through the library isn't that much more.

Seriously, I might have to implement something like this myself. The reason I use wildcard deletes is because I am caching bits of paged content so I don't have to do pager queries each time a user browses my site. Without knowing how many pages I have for any category though, the only way to delete this stuff when a user submits new content is to do a wildcard delete.

ilmaestro’s picture

I want to outline my potential solution and get some feedback from you guys. My solution would be to create an index type of entry in memcache which for some base entry would track all wildcard entries underneath that base entry. For example, I might have several keys for paged content that look like this:

pagedNodes::category-2::srt-latest::pg-0
pagedNodes::category-2::srt-latest::pg-2
pagedNodes::category-2::srt-latest::pg-3
pagedNodes::category-2::srt-popular::pg-0
pagedNodes::category-2::srt-popular::pg-1

If a user submits some new content in category 2, I'll need to wipe out all cached pages in that category, basically wiping:

pagedNodes::category-2::*

So under my solution, I'd create an index entry in memcache for the key above containing the suffixes of everything underneath it, such as:

srt-latest::pg-0
srt-latest::pg-2
srt-latest::pg-3
srt-popular::pg-0
srt-popular::pg-1

A wildcard delete on pagedNodes::category-2:: would then lookup all the keys in that index and wipe them as well. The question is, am I better off just not even using memcache for this sort of paged content requiring wildcard deletes or would the gains from memcache still outweigh the performance hit of deleting potentially several hundred individual memcache entries at a time?

One upside is that the PECL memcache 3 release looks like it will support delete operations on multiple keys at the same time. I'm developing on a windows box though and all the PECL builds are fox *nix :(

ilmaestro’s picture

I wrote up an implementation of what I described above. It worked fine, but for my own purposes, I settled on a different approach where for wildcards. Whenever I update some content that has lots of cache related cache entries (in memcache) that would need to be deleted via a wildcard, instead of doing a delete I create a "parent" cache entry with a simple timestamp.

Then each time any of my cached content that falls under the wildcard is loaded, I also retrieve that parent cache entry and ensure that the parents timestamp is < each content cache's timestamp. If not, I update the content cache.

Downside is 2 cache_get calls for my content loads but upside is I don't have to track all of my wildcard content and do cache_deletes for all of it when it becomes stale.

slantview’s picture

I am willing to take a look at this if we get the consensus that it should work this way. This is how the code in the APC module works. It stores an array with a list of all objects per "bin". Every time you flush, it locks, gets all the keys for that "bin" and then deletes each and removes the lock.

We could do this with memcache, but there is the overhead of reading/writing the index or map object every time. With APC it's not bad cause it's a bit faster than memcache, but still has some occasional issues.

anyone else got an opinion about this?

firebus’s picture

+1

core drupal caching really depends on the wildcard operator, and working around wildcards when deploying memcache is a major headache, with major performance implications (we flush caches in many cases when drupal core can be more selective).

if there's a scalable way to handle wildcards in the memcache module, i would expect to see major performance increases for a number of common use cases (and mine particularly!)

gaolei’s picture

FileSize
3.42 KB
3.44 KB

Here I've finished a patch which makes it possible to empty one bin without flushing other bins on the same server.

The basic idea is to have a version number (which is called "salt" in my code changes) for each bin, and append this number to all the key names within the bin. When a flush() operation is wanted on a bin, just update its version number and the old data will therefore all be outdated and possibly cleared later by memcached automatically. Note that the version numbers are also kept in memcached, and updated via the increment() api.

I think the good thing is that, in this way no expensive operations are required: nothing gets really deleted or flushed. All we have to do is to maintain the version number of each bin, and let memcached to do the dirty work deciding which part of memory should be recycled.

More flexible wildcard matching is not supported by this way. However, I think wildcard matching is generally not a target of memcached. And most of the problems I guess can be solved by using namespace/bin properly.

I am attaching the patch. Any comments and testing results against it are welcomed. Thanks!

slantview’s picture

So I took a look at this patch, and while I do like it, you have to understand that delete operations in memcache are fast and not expensive. For memcache in the Cache Router project, what I did was add in a "shared" configuration option so you can define if the bin is shared. This way instead of doing a full flush, it will keep a lookup table with each entry in the cache for each "bin" and then just perform deletes on each item in the lookup table.

I have seen great performance with this and allows you to do failover for memcache like just assign two big bins for usage and if one of the processes dies, it will keep going.

The one thing that I did on APC for Cache Router that I haven't implemented yet, is the wildcard handling described above. I can get that done in the next beta, and hopefully some of these code changes will be approved (by consensus) to be put into this module as well.

-s

gaolei’s picture

Simply deleting a single entry via memcache_delete() is of course no big deal, but I think deleting upon the lookup table is another case.

Actually I was also discussing and working on the index/map idea here with 2 other engineers. Should say that it is indeed a good idea. However, I was somehow feeling uncomfortable with it, since:

First, looking up the table and then calling memcache_delete() on each item might grow to be relatively expensive when the bin contains a lot of entries. Considering the frequency of flush operation on bins, I feel that the chance of hitting a bad case is big.

Second and more important, as you mentioned in a post above, is the overhead of maintaining this table, i.e. the read/write operations on it, especially the write operation. To keep it in consistency, I'd guess some kinda locks must be used to make sure that there is always at most one client modifying this table. Unfortunately, every set/flush operation will attempt to write it and all other attempts should wait until the current one finishes, which is expensive when the site's traffic load is high.

Finally, I think it will be much easier to implement in the way I suggested and works well to fix the problem with no obvious overhead, and without requiring extra modules like Cache Router to be able to empty one bin without affecting others. The only problem is that it does not support general wilcard matching.

The worries against the table lookup way are not verified by any tests yet. So feel free to comment and please correct me if I was wrong.

slantview’s picture

Hi gaolei,

I think those are valid concerns, however I don't think in practice this is as bad as you think.

First, _full_ table flushes should not happen very frequently, if code needs to flush the bin on a regular basis, then the code should be refactored.

Second, memcache is way faster than you think, a simple lock/unlock for updates has very little overhead. In my testing with the Cache Router project, I was getting speeds of over 800 requests per second on a macbook pro with memcache in "shared" mode.

Lastly, Cache Router is not an addition to this module, it is a completely separate module (read: full on replacement) that uses memcache as _one_ of it's caching mechanisms. it also supports file, db, apc and xcache. Not only that but it allows you to mix and match per bin. I ported most of the code from this module into that module, and I am hoping to try to get it into core.

It is definitely worth a look as it does things very different than memcache alone. and it's fast ;)

pmcg’s picture

This might not be the right place to ask but I'd be GREATLY interested in seeing some details regarding how you arrived at those numbers! Very exciting!

Also, should we take this to mean that the memcache module will no longer be maintained in favor of the cache routing module?

Thanks.

slantview’s picture

I have some details on my cache router testing on that project page: http://drupal.org/project/cacherouer.

And no, it doesn't necessarily mean that this module is deprecated. Those of us who maintain this module will continue to do so as far as I know.

I think I would prefer to consolidate several of these caching modules and invite the authors to co-maintain one module and even better get it into core. But we'll see what happens.

pmcg’s picture

I'm doing some testing here, I'll just say it doesn't look good for the locking argument - results to follow. Also i'll open a bug request on your cacherouter project regarding an unthrottled while loop in the locking mechanism.

~Paul

andypost’s picture

I've tried to expose the idea of "lookup" tables - idea is to store "salted" version of lookup-table and "salted" keys but separate bins as "shared" and "not shared"
"shared" means using wildcards

bin
-index {2}(stores salt)
-lookup-[1] array of $key => $expire (on flush by cron to check temporary and expired keys in lookup)
-lookup-[2] -"-
-key-[1]
-key-[2]

in cacherouter I tried different on wildcard deletions logic
if wildcard
if key == '*' -get lookup, lock, store empty lookup array, unlock, clean all keys
else lock lookup, unset and store keys for deletion in other array, store lookup ,unlock, process cleaning of keys
else delete single entry, lookup updates on cron flush if needed

andypost’s picture

Overview of cache_clear_all with wildcards

in clean d5.7 installation used in 19 files

filter.module
cache_clear_all($form_values['format'] .':', 'cache_filter', TRUE); 3 times
cache_clear_all($format .':', 'cache_filter', TRUE);
menu.module
cache_clear_all($account->uid .':', 'cache_menu', TRUE);

all other use '*' as wildcard

But in clean d6.2 installation used in 31 files

locale.inc
('locale:'. $form_state['values']['langcode'], 'cache');
cache_clear_all('locale:', 'cache', TRUE); 3 times
menu.inc
('links:'. $menu_name .':', 'cache_menu', TRUE); 2 times
theme.inc
('theme_registry', 'cache', TRUE);
filter.admin.inc
($format .':', 'cache_filter', TRUE);
locale.module
('locale:', 'cache', TRUE); 4 times

all other use '*' as wildcard

Cons:
- wildcard cleaning is most needed for d6 and later systems
- in d5 systems "shared" config option only needed for cache_filter and cache_menu
- for d6 shared for cache, cache_filter, cache_menu

robertDouglass’s picture

Bump. I'd like the other memcache maintainers to weigh in on this and see some resolution.

inforeto’s picture

subscribing

robertDouglass’s picture

Btw: anyone gets extra credit for writing simpletests that confirm this problem and it's eventual solution.

robertDouglass’s picture

Priority: Normal » Critical
nirl’s picture

hey all. this issue gave us alot of headace for the last week or so (if not more). We are currently using gaolei's patch (http://drupal.org/node/224772#comment-799489) which is fast albeit expensive. We are using it on a large production system that depends heavily on memcache and we did not see any problems yet. (thank you gaolei).

We have tried to implement a lock-add-unlock scenario such as was suggested in several comments but this will be a definite nightmare for high-traffic/high-update sites.

I would suggest letting the user choose which flush mechanism to use: the current one, or the salt one. I see no reason why the module's developers should decide for me. On a site with little updates and little memory i would prefer to have the whole memcache flush; On a site with many updates and tons of memory i'm willing to sacrifice space for the sake good performance. a simple memcache_flush_method variable would do the job just fine.

** Until this issue is fixed, please add a reference to it in the README.txt. ***
Most people do not go to check for critical issues in the module's project page and this issue is sooo critical for high-traffic sites that it should be noted in the readme file.

yhager’s picture

so much time and this is still marked critical. subscribing and bumping.

DamienMcKenna’s picture

subscribe.

Taras_’s picture

subscribe.

Jeremy’s picture

Status: Active » Closed (duplicate)