Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Radioactivity checks class_exists("Memcache"), but this is FALSE when using the Memcache module.
The attached patch also checks $conf['cache_default_class'] == 'MemCacheDrupal').
Comment | File | Size | Author |
---|---|---|---|
#14 | radioactivity-7956459-memcached-compatibility-14.patch | 3.74 KB | q0rban |
Comments
Comment #1
tcmug CreditAttribution: tcmug commentedThis is most likely caused by the environment not actually having PHP Memcache installed (different than Memcached).
I'd actually fix this by making it possible to either class.
Comment #2
moehac CreditAttribution: moehac commentedI'm only running Memcached, so I made the following change. You might want to consider looking to add support for Memcache and Memcached.
Comment #3
infines CreditAttribution: infines commentedThese fixes work for me! However the original patch and #2 need to be rolled into a single patch. I don't know how to do this, otherwise I would!
Comment #4
androidi CreditAttribution: androidi commentedHere's a patch, which works both on PHP Memcache and Memcached modules.
Comment #5
infines CreditAttribution: infines commentedworking for me
Comment #6
tcmug CreditAttribution: tcmug commentedThanks! Committed to be released soon.
Comment #8
rv0 CreditAttribution: rv0 commentedIf you do a simple egrep -R "memcache" * in the module folder, you'll notice there's plenty of other places that check for Memcache class that need to be adapted.
E.g. to select memcached incident storage on a decay profile.
If I finally get my setup working, I'll provide a patch, but I'm still struggling with other issues atm..
Comment #9
rv0 CreditAttribution: rv0 commentedOk, so I'm digging deeper now.
I got
Reading http://stackoverflow.com/questions/11926376/memcache-connect-vs-addserver
it seems the "connect" isn't needed, and from the looks of it, even bad practice when you have multiple memcache servers.
Following patch removes the connect function, and borrows some logic of memcache_storage module.
Also fixes issues mentioned in #8
Seems to work just fine for me.
Comment #10
RobKoberg CreditAttribution: RobKoberg commentedIn RadioactivityIncidentStorage there is a space missing in the method:
public function addIncident(RadioactivityIncident$incident) {
Then in RadioactivityMemcachedIncidentStorage there needs to be:
public function addIncident(RadioactivityIncident $incident) {
To get rid of compatibility warnings.
Comment #11
rv0 CreditAttribution: rv0 commented@RobKoberg
Agreed, but that didn't cause any issues for me.
Comment #12
tcmug CreditAttribution: tcmug commentedI need your help man! I've committed this to the repository - can you please verify everything is OK regarding this as I haven't got an environment where I can test the bug(s).
I also changed the missing space issue mentioned in this issue thread.
Comment #13
rv0 CreditAttribution: rv0 commented@tcmug
Since I submitted the patch in #9, we are running it on a live website with plenty of traffic and it appears to be working very well.
I will post back here if I notice any issue.
Thanks for this excellent module!
Comment #14
q0rban CreditAttribution: q0rban commentedThe patch from #9 does indeed appear to make the error go away! Nice work. It introduces some Drupal whitespace and coding standard issues, and unnecessary logic. Attached patch simplifies the connect() method, cleans up the whitespace issues, and follows Drupal coding standards.
Comment #15
q0rban CreditAttribution: q0rban commentedOne other thing to note is that in #9 and #14, the precedence of Memcached over Memcache was switched (HT @e0ipso for catching this). I'm not sure if this was intentional in #9, or even something we necessarily care about, but I thought I'd mention it.
Comment #16
tcmug CreditAttribution: tcmug commentedLets close this. Reopen if needed for memcache vs. memcached.