If both extensions are installed, setting a cache item causes the module to try with both the new and old extension APIs. Because the API in use on an instance should depend on the object type (not what extensions are installed), I've updated the code to use the instanceof operator, which is more appropriate.
The factory function still checks which extensions exist and prefers Memcached.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | memcache_default.patch | 4.61 KB | catch |
| #3 | memcache_default.patch | 4.42 KB | catch |
| memcache_plus_memcached.diff | 1.65 KB | david strauss |
Comments
Comment #1
david straussI marked this "major" instead of "critical" because you can work around the problem by only having one extension installed.
Comment #2
longwaveThe ->set() code was already changed in #837758: Memcached::set() expects at most 3 parameters, 4 given in dmemcache.inc on line 42 so the original problem no longer exists and the patch no longer applies. Using instanceof rather than class_exists is technically more appropriate but I can't see how it will ever cause a problem.
Comment #3
catchSlightly re-purposing this, started on a patch at #1131454: Default to Memcache extension but the code here is a prerequisite to making that work. Re-rolled with the changes from there, and the instanceof change from here.
Comment #4
catchAdded a @see for the memcached bug.
Comment #5
jeremy commentedI tested this with both extensions enabled together, and extension enabled individually. When testing the override functionality I wrote 'memcached' with a lower case 'm' and ran into problems -- so added logic to force only the first character upper case and the rest lower. I also updated the README:
http://drupalcode.org/project/memcache.git/commit/11511cd
Needs to be ported to Drupal 7.
Comment #6
catchThis introduced a regression, my fault #1133964: Memcache isn't connecting to all servers fixes it and that needs to be included here before we can commit, so marking CNW.
Comment #7
catchPorted all this to 7.x