Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Memcached (not Memcache) #19

Open
swissspidy opened this issue Aug 11, 2016 · 9 comments
Open

Support Memcached (not Memcache) #19

swissspidy opened this issue Aug 11, 2016 · 9 comments
Assignees
Milestone

Comments

@swissspidy
Copy link

Currently, if the Memcache extension is not installed (but the Memcached extension is), there is a fatal error in includes/class-object-cache.php.

If I manually change WP_Spider_Cache_Object::$engine_class_name to 'Memcached', there is no fatal error any more, but in the back-end I get a notice that I should install the Memache extension.

screen shot 2016-08-11 at 12 06 34

First of all, I don't think anyone will ever see that notice because of the fatal error. Second, mixing up Memcache and Memcached throughout the code base is confusing.

Both are PHP extensions to access a Memcached server. Why are their names used interchangeably in this plugin when clearly the Memcache extension is required?

Sorry if I'm missing something, haven't had my coffee yet :-)

@JJJ
Copy link
Contributor

JJJ commented Aug 11, 2016

Memcache is only used for its getExtendedStats() method, which is what's responsible for connecting to the server to allow us to interact with cached contents.

Memcached has a getStats() method, but it's not as comprehensive and won't allow us to interact with the cached contents themselves.

So, the very simple and nondescript notice you see in your screenshot, is my way of saying "This interface will not work without Memcache installed, but Memcached will continue to work OK."

@JJJ
Copy link
Contributor

JJJ commented Aug 11, 2016

I think there might be some conflation between the engine and daemon terminologies, too. I'll look at cleaning these up soon.

@JJJ
Copy link
Contributor

JJJ commented Aug 11, 2016

Okay. Cleaned some things up, but there is more to do.

Not today, unfortunately, but I have in my mind what needs to happen:

  • Memcache & Memcached support need to be separated
  • For Memcached, the getExtendedStats() method needs to probably use getAllKeys() with a bunch of custom loops to get the data, too

@JJJ JJJ self-assigned this Aug 11, 2016
@JJJ JJJ added this to the 2.2.0 milestone Aug 11, 2016
@JJJ
Copy link
Contributor

JJJ commented Aug 11, 2016

Related: #6.

@JJJ JJJ modified the milestones: 2.2.0, 3.0.0 Aug 11, 2016
@JJJ JJJ modified the milestones: 4.0.0, 3.0.0 Aug 22, 2016
@todeveni
Copy link

Subscribing to this, since the official memcache extension doesn't build for PHP7 and seems abandoned. Sure there is one or two forked versions, but...

@JJJ
Copy link
Contributor

JJJ commented May 11, 2017

General update: I've made some nominal changes surrounding this to try and support additional backends (Redis, mostly) but nothing substantial.

I'm reluctant to litter the codebase with caveats and constants to guide the engine/daemon relationships, but it may be necessary eventually. (WP Redis has a bunch, and I assume they're useful there.)

@JJJ
Copy link
Contributor

JJJ commented Jul 10, 2017

Related: #25.

@ihorvorotnov
Copy link

Subscribing to this. Trying to make it work with Memcached and exploring the code right now.

@bueltge
Copy link
Contributor

bueltge commented Nov 8, 2017

I mean the plugin supports Memcached, only a change in https://github.com/stuttter/wp-spider-cache/blob/master/wp-spider-cache/includes/class-object-memcached.php#L30 to Memcached is to do and it works also wit PHP7* and Memcached.

public $engine_class_name = 'Memcache';

should change to

public $engine_class_name = 'Memcached';

The class, there you use is Memcached

$this->success_code = Memcached::RES_SUCCESS;

		// Set daemon flags
		if ( class_exists( $this->daemon_class_name ) ) {
			$this->success_code   = Memcached::RES_SUCCESS;
			$this->preserve_order = Memcached::GET_PRESERVE_ORDER;
		}

The plugin don't supports Memcache, there is really old - I mean 2013 last support change.

I had tested this change in one environment and it works without problems. If you need an pull request, please ping me.
Best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants