Hello,
After first installing of memcache in D7 I found it not working. Adopted to D7 style, renamed files and fixed various bugs. Here's a patch for both memcache and memcache_admin to work in D7 out of the box.
Bugs - how it was fixed:

  1. Memcache module doesn't appear in the modules list - renamed memcache.inc to memcache.module and updated references
  2. MemCacheDrupal::getMultiple() returned results, indexed by full key, not cid - move item from $results[$full_key] to results[$cid] in MemCacheDrupal::getMultiple()
  3. Memcache Admin is not working without memcache module - add dependency to memcache admin module

One addition from me:

  • define new bin - memcache. Can be used for plain caching, using memcache

Sessions are still not ported, probably I will take care of it in a free time.
I can be wrong somewhere, please review the code and let me know.

Thanks,
Alex

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jeremy’s picture

Status: Needs review » Closed (works as designed)

The 7.x version of this module is deployed in many places. Be sure you're using -beta3 or the 7.x-1.x-dev branch. Your changes are wrong: memcache.inc is properly named. And memcache should not depend on memcache_admin. See the INSTALL.txt file for directions on how to use memcache.

mente’s picture

FileSize
929 bytes

I'm sure, what I've checked. Actually, memcache works, but installation of the module is not user-friendly. Include file in settings.php, instead of using modules page. Why do we need modules then? Just include file and that's it. I thought drupal-way was to use admin panel and not vim with ftp.
Patch provides user-friendly installation, bringing the same functionality. Explain me, please, where I'm wrong.
If you think current deployment method is good - ok, let it be. At least review this patch for MemCacheDrupal::getMultiple().

Jeremy’s picture

Status: Closed (works as designed) » Needs review

Re-opening so we remember to take a look. For it to be merged it would be preferred that you break out each functional difference into an individual patch. One big patch that does a bunch of unrelated things 1) may never actually get reviewed, 2) is much less likely to get merged.

catch’s picture

You can't use modules for caching backends - the page cache, variables and system_list cache (which includes module_list()) are all requested before modules are loaded.

catch’s picture

Title: memcache adoption to D7 » Remove 'full key' from cache objects

Re-titling for reduced scope.

Berdir’s picture

FileSize
963 bytes

Here is a new patch that fixes it directly in dmemcache.inc.

Berdir’s picture

FileSize
1.2 KB

I figured out that $value->cid already contains the correct cid, so that $reverse stuff isn't necessary.

I also found a second bug: $cids is not correctly changed because you are trying to delete entries from which the key is the $cid, but the key is just an integer, the values are the cids.

New patches fixes both.

Without this patch, both the fields cache from field_storage_sql and export defaults cache from ctools are completely broken and rewrite their cache *on every single site*, which takes >0.5s on every page, making the site twice as slow as it is without memcached :)

How can anyone actually use this module... ;)

Berdir’s picture

FileSize
3.79 KB

And now with tests!

I had some trouble having the tests actually use MemCacheDrupal, I had to manually set the default value in _cache_get_object(), Simpletest seems to have somehow overriden what I set in settings.php.

The tests fail horribly without this patch :)

All I did in the tests was to prefix item1 and item2 with test:, so that the $full_key is urlencoded and different and I verified that $items_ids was changed correctly.

catch’s picture

Status: Needs review » Fixed

Thanks for the patch, and the tests! Committed and pushed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.