Question on 'under-run' in the KetamaNodeLocator.cs #100

Closed
ciaranj opened this Issue Mar 19, 2012 · 0 comments

Comments

Projects
None yet
2 participants
Contributor

ciaranj commented Mar 19, 2012

HI, I'm using a very old version of your library (my last commit that I made, and shared with you was f977937 ) and am looking to upgrade as I just recently fell over an inconsistency between Enyim & spy.memcached. I notice now you've added a new 'KetamaNodeLocator' that professes to be compatible with 'spymemcached' (whereas I've been using the original DefaultNodeLocator with some customisation to the Java side to more closely align the two clients!

I'm looking to move to using outta-the-box implementations of both clients, but an internal issue I've just had raised against me caused by some code that still appears present in the very latest version :)

The logic at https://github.com/enyim/EnyimMemcached/blob/master/Enyim.Caching/Memcached/Locators/KetamaNodeLocator.cs#L182 is such that if a key's hash is smaller than the minimum of all the known hashes then the server with the highest key will be used. ( I can see that htis code is being used back at my version, but am struggling to get the signing working to test at the latest version, although the code is still present)

This is not wrong per http://www.last.fm/user/RJ/journal/2007/04/10/rz_libketama_-_a_consistent_hashing_algo_for_memcache_clients as it does not specify what to do in this situation! HOWEVER spy.memcached takes the exact opposite approach:

https://github.com/dustin/java-memcached-client/blob/master/src/main/java/net/spy/memcached/KetamaNodeLocator.java#L118

Where if any results are returning (including where those results are the set of all nodes) then the first server key will be used. 'Fixing' the java client is actually easier as that conditional statement could be :

if(tailMap.isEmpty()) {
  hash=ketamaNodes.firstKey();
} else {
  if( tailMap.size() == ketamaNodes.size() ) {
    hash= tailMap.lastKey();
  }  else {
    hash= tailMap.firstKey();
  }

}

But if you're aiming for compatibility with that library (and if this is still a problem, I'm struggling to build the latest version of this library atm!) then it might be something to consider fixing [if you agree it is a problem I'll sort out a pull request for you!]

For me, if I use the following server addresses:

172.25.1.135:11211 and 172.25.1.136:11211 and try to locate the node for the key 'all-actioningMapEntries_ns_key' these two clients return different nodes.

See: http://code.google.com/p/spymemcached/issues/detail?id=243 for my related question on the spy.memcached library.

enyim closed this Apr 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment