Doist / hash_ring

Implements consistent hashing in Python (using md5 as hashing function)
http://amix.dk/blog/viewEntry/19367
108 stars 38 forks source link

Not compatible with hash_ring for python2 #11

Closed tumb1er closed 2 years ago

tumb1er commented 10 years ago

python2.7 hash_ring from pip

>>> h._hash_digest('00000000000000000000000000000063')
[91, 23, 236, 125, 39, 146, 246, 191, 11, 12, 111, 53, 228, 134, 206, 125]

python3.3 this fork hash_ring

>>> h._hash_digest('00000000000000000000000000000063')
[98, 34, 91, 92, 120, 49, 55, 92, 120, 101, 99, 125, 39, 92, 120, 57, 50, 92, 120, 102, 54, 92, 120, 98, 102, 92, 120, 48, 98, 92, 120, 48, 99, 111, 53, 92, 120, 101, 52, 92, 120, 56, 54, 92, 120, 99, 101, 125, 34]

Even more, first two bytes of _hash_digest are always 98 and 34, because you map ord to representation of bytes object:

>>> m.digest()
b"[\x17\xec}'\x92\xf6\xbf\x0b\x0co5\xe4\x86\xce}"

and if you do bisect you will have a strongly non-uniform distribution for one of nodes.

tumb1er commented 10 years ago

Just use

list(map(int, m.digest()))
thedrow commented 10 years ago

@tumb1er Use where?

tumb1er commented 10 years ago

@thedrow see #13

a10y commented 8 years ago

@tumb1er Did you ever get this merged into some other fork of this project? I also have a feature that I'd like to merge upstream but it seems like this project hasn't had any action in a very long time

tumb1er commented 8 years ago

@andreweduffy, I've created sdist package for internal use just from PR branch. Also, I've seen @asvetlov's fork with same fix. He is an author of aiohttp package, so may be it'll be better to ask him to maintain another hash_ring version at PyPi.

a10y commented 8 years ago

@tumb1er As far as I can tell, the aiohttp package doesn't seem to depend on hash_ring so I'm not sure why asvetlov would be interested in maintaining a fork, but if he is then I'll have a PR for him :) I was just curious because I assumed that there must be some well-maintained consistent hashing library for Python, but if that's not currently the case (and I'm not looking to maintain one right now) then I'll just make my requirements.txt point to the git url for my fork.

asvetlov commented 8 years ago

Guys, sorry. I'm not interesting in hash_ring library maintaining. At least not now.

a10y commented 8 years ago

thanks @asvetlov

goncalossilva commented 2 years ago

13 is merged.