Merge pull request #2168 from DanCech/carbon714-dcech

Fix hash position computation of fnv1a_ch

.travis.yml

@@ -20,6 +20,9 @@ matrix:
     - python: 3.6
       env:
         - TOXENV=py36-django111-pyparsing2-msgpack
+    - python: 3.6
+      env:
+        - TOXENV=py36-django111-pyparsing2-pyhash
     - python: 3.6
       env:
         - TOXENV=lint
@@ -30,6 +33,7 @@ env:
   - TOXENV=py27-django110-pyparsing2
   - TOXENV=py27-django111-pyparsing2-rrdtool
   - TOXENV=py27-django111-pyparsing2-msgpack
+  - TOXENV=py27-django111-pyparsing2-pyhash
   - TOXENV=py27-django111-pyparsing2-mysql TEST_MYSQL_PASSWORD=graphite
   - TOXENV=py27-django111-pyparsing2-postgresql TEST_POSTGRESQL_PASSWORD=graphite
   - TOXENV=docs
@@ -50,6 +54,7 @@ services:
 before_install:
   - bash -c "if [[ '$TOXENV' == *mysql* ]]; then mysql -e "'"'"GRANT ALL ON test_graphite.* TO 'graphite'@'localhost' IDENTIFIED BY 'graphite';"'"'"; fi"
   - bash -c "if [[ '$TOXENV' == *postgresql* ]]; then psql -c "'"'"CREATE USER graphite WITH CREATEDB PASSWORD 'graphite';"'"'" -U postgres; fi"
+  - bash -c "if [[ '$TOXENV' == *pyhash* ]]; then sudo apt-get -qq update; sudo apt-get install -y libboost-python-dev; fi"
 
 install:
   - pip install tox

tox.ini

@@ -1,6 +1,6 @@
 [tox]
 envlist =
-	py{27,34,35,36,py}-django1{8,9,10,11}-pyparsing2{,-mysql,-postgresql,-rrdtool,-msgpack},
+	py{27,34,35,36,py}-django1{8,9,10,11}-pyparsing2{,-mysql,-postgresql,-rrdtool,-msgpack,-pyhash},
 	lint, docs
 
 [testenv]
@@ -38,6 +38,7 @@ deps =
 	postgresql: psycopg2
 	rrdtool: rrdtool
 	msgpack: msgpack-python
+	pyhash: pyhash
 
 [testenv:docs]
 basepython = python2.7

webapp/graphite/render/hashing.py

@@ -16,32 +16,43 @@
 from hashlib import md5
 from itertools import chain
 import bisect
+import sys
 
 try:
   import pyhash
   hasher = pyhash.fnv1a_32()
-  def fnv32a(string, seed=0x811c9dc5):
-    return hasher(string, seed=seed)
+
+  def fnv32a(data, seed=0x811c9dc5):
+    return hasher(data, seed=seed)
 except ImportError:
-  def fnv32a(string, seed=0x811c9dc5):
+  def fnv32a(data, seed=0x811c9dc5):
     """
     FNV-1a Hash (http://isthe.com/chongo/tech/comp/fnv/) in Python.
     Taken from https://gist.github.com/vaiorabbit/5670985
     """
     hval = seed
     fnv_32_prime = 0x01000193
     uint32_max = 2 ** 32
-    for s in string:
-      hval = hval ^ ord(s)
-      hval = (hval * fnv_32_prime) % uint32_max
+    if sys.version_info >= (3, 0):
+      # data is a bytes object, s is an integer
+      for s in data:
+        hval = hval ^ s
+        hval = (hval * fnv_32_prime) % uint32_max
+    else:
+      # data is an str object, s is a single character
+      for s in data:
+        hval = hval ^ ord(s)
+        hval = (hval * fnv_32_prime) % uint32_max
     return hval
 
-def hashRequest(request):
-  # Normalize the request parameters so ensure we're deterministic
-  queryParams = ["%s=%s" % (key, '&'.join(values))
-                 for (key,values) in chain(request.POST.lists(), request.GET.lists())
-                 if not key.startswith('_')]
 
+def hashRequest(request):
+  # Normalize the request parameters to ensure we're deterministic
+  queryParams = [
+    "%s=%s" % (key, '&'.join(values))
+    for (key,values) in chain(request.POST.lists(), request.GET.lists())
+    if not key.startswith('_')
+  ]
   normalizedParams = ','.join( sorted(queryParams) )
   return compactHash(normalizedParams)
 
@@ -55,9 +66,7 @@ def hashData(targets, startTime, endTime, xFilesFactor):
 
 
 def compactHash(string):
-  hash = md5()
-  hash.update(string.encode('utf-8'))
-  return hash.hexdigest()
+  return md5(string.encode('utf-8')).hexdigest()
 
 
 class ConsistentHashRing:
@@ -73,13 +82,10 @@ def __init__(self, nodes, replica_count=100, hash_type='carbon_ch'):
 
   def compute_ring_position(self, key):
     if self.hash_type == 'fnv1a_ch':
-      big_hash = '{:x}'.format(int(fnv32a( str(key) )))
-      if len(big_hash) > 4:
-          small_hash = int(big_hash[:4], 16) ^ int(big_hash[4:], 16)
-      else:
-          small_hash = int(big_hash, 16)
+      big_hash = int(fnv32a(key.encode('utf-8')))
+      small_hash = (big_hash >> 16) ^ (big_hash & 0xffff)
     else:
-      big_hash = compactHash(str(key))
+      big_hash = compactHash(key)
       small_hash = int(big_hash[:4], 16)
     return small_hash
 
@@ -92,6 +98,8 @@ def add_node(self, key):
       else:
         replica_key = "%s:%d" % (key, i)
       position = self.compute_ring_position(replica_key)
+      while position in [r[0] for r in self.ring]:
+        position = position + 1
       entry = (position, key)
       bisect.insort(self.ring, entry)
     self.ring_len = len(self.ring)
@@ -112,6 +120,10 @@ def get_node(self, key):
 
   def get_nodes(self, key):
     nodes = []
+    if not self.ring:
+      return nodes
+    if self.nodes_len == 1:
+      return list(self.nodes)
     position = self.compute_ring_position(key)
     search_entry = (position, None)
     index = bisect.bisect_left(self.ring, search_entry) % self.ring_len

webapp/tests/test_render.py

@@ -979,7 +979,12 @@ def test_chr_compute_ring_position_fnv1a(self):
         ("127.0.0.3", "866a18b81f2dc4649517a1df13e26f28")]
         hashring = ConsistentHashRing(hosts, hash_type='fnv1a_ch')
         self.assertEqual(hashring.compute_ring_position('hosts.worker1.cpu'), 59573)
+        self.assertEqual(hashring.compute_ring_position('hosts.worker1.load'), 57163)
         self.assertEqual(hashring.compute_ring_position('hosts.worker2.cpu'), 35749)
+        self.assertEqual(hashring.compute_ring_position('hosts.worker2.network'), 43584)
+        self.assertEqual(hashring.compute_ring_position('hosts.worker3.cpu'), 12600)
+        self.assertEqual(hashring.compute_ring_position('hosts.worker3.irq'), 10052)
+        self.assertEqual(hashring.compute_ring_position(u'a\xac\u1234\u20ac\U00008000'), 38658)
 
     def test_chr_get_node_fnv1a(self):
         hosts = [("127.0.0.1", "ba603c36342304ed77953f84ac4d357b"), ("127.0.0.2", "5dd63865534f84899c6e5594dba6749a"),