Skip to content

getVersion used as connection validation in MemcachedConnector breaks redundant memcached setups #17957

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

Closed
ukautz opened this issue Feb 16, 2017 · 1 comment

Comments

@ukautz
Copy link

ukautz commented Feb 16, 2017

  • Laravel Version: 5.4.12 (since >=4)
  • PHP Version: 7.0.15 (not a factor)
  • Database Driver & Version: libmemcached 1.0.18 + php-memcached 3.0.2

Description:

In the MemcachedConnector class the validateConnection method, which is called after memcached servers have been added, uses Memcached::getVersion to verify that memcached servers have been added.

Now, getVersion implemented by php-memcached uses memcached_version from libmemcache. Should any memcached server fail, then memcached_version() will return MEMCACHED_SOME_ERRORS which causes getVersion to return false instead not an array of the "working servers". So any failing servers results in validateConnection throwing a \RuntimeException.

This is especially counterproductive when using a replicated memcached setup, in which one or multiple memcached servers could fail without problems - would it not be for the check and which disallows any memcached server to fail.

Both memcached_version from libmemcached as well as getVersion from php-memcached act as intended (as I understand it), so it's not a bug with either. Just using getVersion to validate the connection is not the intended usage. As far as I can tell this is the case with any available libmemcached, at least since 1.0.0. Also php-memcached, at least since 2.0.0.

Steps To Reproduce:

  • Setup memcached as cache in Laravel using more than one memcached servers
  • Write a controller, which uses \Cache::(put|get|...) so that MemcachedConnector is created
  • Simulate an outage in one of the memcached servers
  • See the \RuntimeException

Here a short test setup. Laravel configured as above is available locally under http://localhost:8080/, then:

# start up memcached
$ memcached -p 11211 &
$ memcached -p 11212 &

# issue a request to your Laravel App, while both servers are up
$ curl http://localhost:8080/

# simulate outage of _one_ of the memcached servers
$ iptables -I INPUT -p tcp --destination-port 11211 -j DROP

# issue another request, see the exception
$ curl http://localhost:8080/

Solution (idea)

Either remove the getVersion check, make it controllable (disable/enable) via configuration or use set("some-special-key", ..) + get("some-special-key"). The latter is of course more expensive than getVersion.

I can provide a patch, if you let me know which of the solution ideas you prefer.

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

No branches or pull requests

2 participants