Add hashing client #59

sontek · 2015-06-20T20:25:13Z

This is the beginning of how I would expect the hashing client to work. It is built off of #51 so that would need to be merged first.

It is an alternative to #44 and a fix for #52

Things left to do:

Implement all client functions (set_many, get_many, incr, etc)
Implement failover detection if a client is detected as down
Write Integration Tests
Write Unit tests
Add proper logging
Write detailed documentation

For failover support HashClient will support the following settings:

retry_attempts - After retry_attempts has been hit it will mark the server as dead and remove it from the rotation.
retry_timeout - It will wait this long in between retries
dead_timeout - After retry_attempts have been reached it will wait this long before attempting to add the server back into the pool.

Once a server is marked dead the keys that were allocated to it will be rebalanced across the nodes that are still up.

You can use this like this:

from pymemcache.client.hash import HashClient
import time

client = HashClient([
    ('127.0.0.1', 11211),
    ('127.0.0.1', 11212)
])

while True:
    for key in ['foo', 'bar', 'baz']:
        client.set(key, 'bar')
        result = client.get(key)
        print(result)
    time.sleep(1)

sontek · 2015-06-20T22:08:44Z

With babb8c0 I added support for failover. This doesn't recover dead servers yet but it will remove misbehaving servers from the pool.

sontek · 2015-06-20T23:20:25Z

8879147 brings dead servers back into rotation after the dead_timeout

sontek · 2015-06-21T04:30:49Z

With 2470508 this is fairly feature complete. There is a lot of polish I need to throw in like better logging / error checking and plenty more unit tests but its definitely working.

cgordon · 2015-06-22T05:52:09Z

I did a quick first pass on the code, and it looks good! I'll take a closer look this week, probably on Wednesday (that's the first day I don't have wall-to-wall meetings). We use a memcache proxy internally, so we haven't needed this, but it's been high on my list of things to add for people that aren't using a proxy.

sontek · 2015-06-22T07:11:50Z

@cgordon Sounds good, what proxy do you guys use? We currently use pylibmc with ketama but have thought about using mcrouter or twemproxy instead

sontek · 2015-06-22T07:31:42Z

@cgordon One thing I thought about with this is making clandestined an optional extra pip install pymemcache[murmur3] or something. Then we can default to using crc32 which is built into the standard library. crc32 isn't perfect but it works

cgordon · 2015-06-28T23:04:09Z

pymemcache/client/base.py

@@ -684,7 +650,7 @@ def _fetch_cmd(self, name, keys, expect_cas):
                    result[key] = value
                else:
                    raise MemcacheUnknownError(line[:32])
-        except Exception:
+        except Exception as e:


Why do we need the "as e" here?

Oops, added that during a debugging session, wasn't supposed to be included in the commit

cgordon · 2015-06-28T23:27:45Z

@sontek The code looks great, I just have a few cosmetic comments and one comment about the overall contract of the methods. Let's talk about that a little bit when you have time.

Also, we're currently using mcrouter, and previously used twemproxy. I haven't been very involved in those decisions, but my understanding is that mcrouter is the better of the two.

…hing_client

sontek · 2015-07-02T15:39:52Z

@cgordon Addressed all of your concerns in a935261, what do you think about the overall contract? I initially was trying to clean the contract up with this but I think we can do that slower with v2 clients and deprecation warnings for the old ones.

I'm not completely against having the hashing client raise an exception but it would mean you would have to opt-in to graceful failover (unless we made it not raise exceptions by default)

sontek · 2015-07-02T16:16:22Z

@cgordon The big question I have about this is if the murmur3/clandestined hash client should be the default. clandestined uses murmur3 C extension for performance, which makes this client no longer pure python.

What I think might be better is provide a less efficient/less consistent default using CRC32 which is pure python and provide an extras (client.ext.murmur3) that can be used like:

pip install pymemcache[murmur3] and then they will get the better client. What do you think? Is it fine to depend on the C extension in clandestined or should we make it optional?

sontek · 2015-07-02T17:41:27Z

@cgordon I decided to go ahead and make a pure python3 version of this and make using the C dependency a documentation issue, here is the PR:

sontek#1

I haven't merged it yet because I want to hear your feedback first.

cgordon · 2015-07-02T18:25:56Z

@sontek I agree with your last comment, that we should make the C dependency possible but optional. I took a look at the branch and it looks great to me.

Remove dependency on clandestined and use pure python murmur3

cgordon · 2015-07-08T18:55:28Z

@sontek Sorry for the long pause, was out of town for the 4th. Would you mind re-basing this change to pick up the documentation and test changes that we've already merged? The rest of the code looks great, but I just want to do one more pass through it before merging.

cgordon · 2015-07-13T23:25:12Z

@sontek I'm ready to merge this change, but it needs to be merged with the documentation and testing changes from the last week. Can you take care of that?

sontek · 2015-07-13T23:39:51Z

@cgordon Yeah, sorry I was busy last week but I'll clean it up in the next few days! :)

cgordon · 2015-07-13T23:43:29Z

@sontek Awesome! Just double checking that I wasn't dropping the ball. Very excited about this change :)

Make ignoring exceptions optional

…hing_client Conflicts: docs/apidoc/pymemcache.test.rst

sontek · 2015-07-16T16:54:29Z

@cgordon I think its ready for review! Want to take it for a spin on pinterest codebase to make sure I didn't break any backwards compatibility?

thedrow · 2015-07-16T17:34:41Z

clandestined already doesn't compile murmur if on PyPy/PyPy3 and uses a pure python implementation of murmur instead following ewdurbin/clandestined-python#8 so that's a non-issue.

sontek · 2015-07-16T17:43:24Z

@thedrow Yeah, I think the important part is that pymemcache is pure python by default but people can opt-in to using clandestined with C dependencies if they want. This allows us to use universal wheels and deploy with platforms without having to compile anything.

thedrow · 2015-07-16T18:01:48Z

can we please add a build job that tests this both with clandestined and without?

thedrow · 2015-07-16T18:01:48Z

can we please add a build job that tests this both with clandestined and without?

sontek · 2015-07-16T18:37:24Z

@thedrow I'll document how to use it instead of just recommending it in the docs

…hing_client Conflicts: setup.py

Add hashing client

cgordon · 2015-07-17T00:22:03Z

LGTM. Thanks @sontek!

cgordon · 2015-07-17T00:23:54Z

@sontek, separately, can you switch the readthedocs page to use the master repository so the docs are up to date? It looks like they point at your personal branch right now.

sontek · 2015-07-17T00:58:24Z

@cgordon Done! Do you have a readthedocs user? I can add you on there as well

cgordon · 2015-07-17T01:26:44Z

@sontek I just created a user on readthedocs.org, I'm "charles.gordon".

sontek added 2 commits June 20, 2015 13:21

Initial implementation of the hash client

f394d42

This adds failover support.

babb8c0

Add bringing dead servers back into rotation

8879147

sontek mentioned this pull request Jun 20, 2015

add shardingclient #44

Closed

Implemented the other functions and added tests

2470508

sontek added 3 commits June 20, 2015 23:41

Fix tests + pep8

9892226

drop support for pypy3 for now

236643a

Fix API to be backwards compatible

abf4bd8

sontek changed the title ~~Add hashing client (Work in progress)~~ Add hashing client Jun 21, 2015

Added unit tests

381715f

cgordon reviewed Jun 28, 2015
View reviewed changes

sontek added 2 commits July 2, 2015 08:02

Merge branch 'master' of github.com:pinterest/pymemcache into add_has…

f43cf74

…hing_client

Code review clean ups and doc fixes

a935261

Remove dependency on clandestined and use pure python murmur3

cff5abf

sontek added 3 commits July 2, 2015 13:36

Merge pull request #1 from sontek/make_murmur3_optional

d6fc5ce

Remove dependency on clandestined and use pure python murmur3

Make ignoring exceptions optional

b7c02a3

flake8 fixes

deb2cbf

sontek added 3 commits July 16, 2015 09:49

Merge pull request #2 from sontek/make_default_values_configurable

75bba31

Make ignoring exceptions optional

Merge branch 'master' of github.com:pinterest/pymemcache into add_has…

6ef4ab0

…hing_client Conflicts: docs/apidoc/pymemcache.test.rst

Updated docs

88b9fc5

Added pypy3 support back

aca08b5

sontek mentioned this pull request Jul 16, 2015

Introduce ChangeLog #63

Merged

sontek added 4 commits July 16, 2015 12:42

Import PooledClient in its old location for backwards compat

fe53429

Merge branch 'master' of github.com:pinterest/pymemcache into add_has…

917167a

…hing_client Conflicts: setup.py

Added ChangeLog entries for the unreleased code

a844d20

Use restructured text instead of markdown for README

ed85bcf

cgordon added a commit that referenced this pull request Jul 17, 2015

Merge pull request #59 from sontek/add_hashing_client

63a2d96

Add hashing client

cgordon merged commit 63a2d96 into pinterest:master Jul 17, 2015

sontek deleted the add_hashing_client branch July 17, 2015 00:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add hashing client #59

Add hashing client #59

sontek commented Jun 20, 2015

sontek commented Jun 20, 2015

sontek commented Jun 20, 2015

sontek commented Jun 21, 2015

cgordon commented Jun 22, 2015

sontek commented Jun 22, 2015

sontek commented Jun 22, 2015

cgordon Jun 28, 2015

sontek Jul 2, 2015

cgordon commented Jun 28, 2015

sontek commented Jul 2, 2015

sontek commented Jul 2, 2015

sontek commented Jul 2, 2015

cgordon commented Jul 2, 2015

cgordon commented Jul 8, 2015

cgordon commented Jul 13, 2015

sontek commented Jul 13, 2015

cgordon commented Jul 13, 2015

sontek commented Jul 16, 2015

thedrow commented Jul 16, 2015

sontek commented Jul 16, 2015

thedrow commented Jul 16, 2015

thedrow commented Jul 16, 2015

sontek commented Jul 16, 2015

cgordon commented Jul 17, 2015

cgordon commented Jul 17, 2015

sontek commented Jul 17, 2015

cgordon commented Jul 17, 2015

Add hashing client #59

Add hashing client #59

Conversation

sontek commented Jun 20, 2015

sontek commented Jun 20, 2015

sontek commented Jun 20, 2015

sontek commented Jun 21, 2015

cgordon commented Jun 22, 2015

sontek commented Jun 22, 2015

sontek commented Jun 22, 2015

cgordon Jun 28, 2015

Choose a reason for hiding this comment

sontek Jul 2, 2015

Choose a reason for hiding this comment

cgordon commented Jun 28, 2015

sontek commented Jul 2, 2015

sontek commented Jul 2, 2015

sontek commented Jul 2, 2015

cgordon commented Jul 2, 2015

cgordon commented Jul 8, 2015

cgordon commented Jul 13, 2015

sontek commented Jul 13, 2015

cgordon commented Jul 13, 2015

sontek commented Jul 16, 2015

thedrow commented Jul 16, 2015

sontek commented Jul 16, 2015

thedrow commented Jul 16, 2015

thedrow commented Jul 16, 2015

sontek commented Jul 16, 2015

cgordon commented Jul 17, 2015

cgordon commented Jul 17, 2015

sontek commented Jul 17, 2015

cgordon commented Jul 17, 2015