backdrop-contrib / memcache

High performance integration with memcache.
2 stars 1 forks source link

Path to default lock file is invalid causing 500 errors #5

Open rbargerhuff opened 4 months ago

rbargerhuff commented 4 months ago

My colleague and I noticed multiple 500 errors being thrown on one of our Backdrop CMS instances where the memcache module is being used. We traced it to the following line:

File: memcache-lock.inc:

Line 27:

$lock_file = BACKDROP_ROOT . '/includes/lock.inc';

This is the location on Drupal 7. For Backdrop, the correct location is /core/includes/lock.inc

Line 27 corrected:

$lock_file = BACKDROP_ROOT . '/core/includes/lock.inc';

This should be fixed as soon as possible because of its potential to cause 500 errors.

Cheers!

rbargerhuff commented 3 months ago

Is this project / codebase still being supported?

quicksketch commented 1 month ago

Hi @rbargerhuff, the short of it is that I ported this module for a client that had previously used memcache but found it to be too unreliable to continue working with it. Memcache is great for niche purposes, but as a general cache replacement (and especially as a lock replacement) Redis is a better choice. The lack of a reliable "expiration" capability in Memcache requires all kinds of PHP-based checking instead of delegating that to the cache system like you can do with Redis.

That said, I'm still open to improving this module for those that have a strong preference.

rbargerhuff commented 1 month ago

Hi @quicksketch , I appreciate your response! If possible, we would like the fix above to be implemented into Memcache. I did talk this over with @smaiorana and we may be interested in Redis but unfortunately we are in the middle of a global transition and we do not have the time to evaluate and switch to Redis. right now.

Thank you!

quicksketch commented 1 month ago

Sure, @rbargerhuff, I committed a fix that corrects that path (https://github.com/backdrop-contrib/memcache/commit/5cb0e62662a42accfcf6533c5d13fea974dc0d59).

Note that the only way the code that include statement is executed is if Memcache's lock connection doesn't work. Which means if that error is occurring, it would be no different to not specify the memcache lock in settings.php.

rbargerhuff commented 1 month ago

@quicksketch Hi Nate, thank you so much for doing that!! Had a chance to test and the commit looks correct!! Thanks again!