Interrupted connection to memcache server can cause inconsistencies

Bug #887765 reported by Daniel Benamy on 2011-11-08
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Python Memcached
Undecided
Sean Reifschneider

Bug Description

First off, thanks for releasing and supporting this awesome tool!

I've describes a situation which seems to allow inconsistencies in the cache- http://stackoverflow.com/questions/6876250/how-does-django-handle-multiple-memcached-servers/7933163#7933163

I would think that if python-memcached loses the connection to a server in the pool, when it reconnects it should empty the cache in that server.

Am I missing something?

Thanks!

Dan

Sean Reifschneider (jafo) wrote :

I don't know of any other clients that implement that, so doing so would be unexpected in many uses. However, if you were to implement such a thing, I would look at including it in the library.

Sean

Changed in python-memcached:
status: New → Opinion
Daniel Benamy (dbenamy) wrote :

What do you think of this patch? I haven't tested it yet. I want to make sure you like the approach before spending that time.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/10/2011 02:40 PM, Daniel Benamy wrote:
> What do you think of this patch? I haven't tested it yet. I want to make
> sure you like the approach before spending that time.
>
> ** Patch added: "flush-on-reconnect.diff"
> https://bugs.launchpad.net/python-memcached/+bug/887765/+attachment/2592438/+files/flush-on-reconnect.diff
>
>
I've been a bit distracted, but thanks for the patck, I hope to take a look
at it over the next couple of days, but it may take the weekend before I
can look at it.

Thanks,
Sean
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iD8DBQFOxAq2xUhyMYEjVX0RApMRAJ9TPdMQVUXdRAQZxo6oF56PnXA5kwCgowuL
UP2MYnXg/sENK5/mHjMTzK0=
=/2F4
-----END PGP SIGNATURE-----

Daniel Benamy (dbenamy) wrote :

Is there any chance you'll get to this today or tomorrow? If not, I'll need to get to work on an alternate plan to solve this problem in my environment. Thanks!

Sean Reifschneider (jafo) wrote :

Sorry, I got confused this weekend while I was working on the memcached module and couldn't "find" this patch. I'll try to get it looked at today.

Daniel Benamy (dbenamy) wrote :

No problem. Thanks for taking a look!

Sean Reifschneider (jafo) wrote :

This patch looks good, but it needs to default to behaving the way it currently does, meaning not to flush the cache on a re-connect. Then some way of turning this on, either a creation flag in the Client() object, or a method to turn it on and off. Is this something you could add to the patch?

I just pushed out a new module yesterday, but I wouldn't be opposed to pushing out a new one with these changes in.

Thanks again for the patch,
Sean

Daniel Benamy (dbenamy) wrote :

I've attached a new patch with that change, but I don't have any way to use it through existing versions of django. Can you think of a way to expose this option that can be used?

Sean Reifschneider (jafo) wrote :

Ok, I've fixed an issue in this that caused "make test" to fail (the self.flush_on_reconnect line needed to be moved up above set_servers in the Client.__init__), and committed it as r55. I'm doing a release of version 1.49 right now.

However, one thing that this patch doesn't address is this situation:

You get disconnected from a memcached.
You store a value for key X.
You reconnect to that memcached.
You flush that memcached.
You store a new value for key X.
You get disconnected from a memcached.
You pull a value for key X.

At that point you're going to get the stale key from when the memcached was disconnected.

Thanks for the patch.

Changed in python-memcached:
status: Opinion → Fix Released
assignee: nobody → Sean Reifschneider (jafo)
Daniel Benamy (dbenamy) wrote :

Good point. I guess the only safe thing to do is to flush all servers when reconnecting to one of them. I'll try to make another pass if I have time.

About using this from django: how do you feel about allowing options to be set in the server string? Something like

memcache.Client(['127.0.0.1:11211?flush_on_reconnect=1'])

I'll also need to check that django passes the whole string through.

Sean Reifschneider (jafo) wrote :

Allowing options to be set in the connect string sounds fine to me.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers