Interrupted connection to memcache server can cause inconsistencies

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

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-----

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!

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.

No problem. Thanks for taking a look!

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

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?

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.

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.

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