Closed iain-buclaw-sociomantic closed 6 years ago
The diff between the two lists of replica_key = hash
is:
--- carbon.txt 2017-12-21 17:46:57.480823985 +0100
+++ crelay.txt 2017-12-21 17:46:43.069027781 +0100
@@ -39,7 +39,7 @@
38-a = 15441
39-a = 31464
40-a = 12862
-41-a = 16159
+41-a = 65465
42-a = 32947
43-a = 64019
44-a = 11346
@@ -62,7 +62,7 @@
61-a = 43031
62-a = 34683
63-a = 51405
-64-a = 53693
+64-a = 2000
65-a = 14002
66-a = 39671
67-a = 47752
@@ -139,7 +139,7 @@
38-b = 536
39-b = 32069
40-b = 16003
-41-b = 25241
+41-b = 1795
42-b = 33146
43-b = 65214
44-b = 5319
@@ -160,9 +160,9 @@
59-b = 52127
60-b = 30767
61-b = 43146
-62-b = 25803
+62-b = 31190
63-b = 51972
-64-b = 44644
+64-b = 3849
65-b = 15303
66-b = 39426
67-b = 35137
That is a worry, seems like some keys produce different hashes.
It's after applying https://github.com/graphite-project/carbon/pull/679, right?
It's after applying #679, right?
I didn't have it applied. But applying that patch makes no difference.
If I calculate the small_hash the "C" way (by using bitwise shifts) then the computed hash rings match.
--- a/lib/carbon/hashing.py
+++ b/lib/carbon/hashing.py
@@ -38,13 +38,10 @@ class ConsistentHashRing:
def compute_ring_position(self, key):
if self.hash_type == 'fnv1a_ch':
if sys.version_info >= (3, 0):
- big_hash = '{:x}'.format(int(fnv32a(key)))
+ big_hash = int(fnv32a(key))
else:
- 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(str(key)))
+ small_hash = (big_hash >> 16) ^ (big_hash & 0xFFFF)
else:
if sys.version_info >= (3, 0):
big_hash = md5(key.encode('utf-8')).hexdigest()
Cool! Could you please make a PRs? Thanks!
Right, just as soon as I understand why the original computation sometimes fails.
OK, it looks like the high and low parts are split incorrectly.
i.e:
key = '41-b'
fnv32a = 104464697
hex = 63a0139
The big hash is 7 hex digits in length, and is split into [63a0 : 139]
, which yields the computation:
small_hash = 0x63a0 ^ 0x139 # 25504 ^ 313 == 25241
Where it should instead be split into [63a : 0139]
, which gives us the correct answer:
small_hash = 0x63a ^ 0x0139 # 1594 ^ 313 == 1795
Makes sense, nice catch @iain-buclaw-sociomantic ! In any case it'll be more efficient with your patch. A test case to cover this would also be great.
Alternatively, the big hash could be assigned as:
big_hash = '{:08x}.format(int(fnv32a(str(key)))
And the original logic could be kept (removing the condition that works around smaller hashes).
Yes, I agree that not bothering with formatting and just doing the bitwise approach should be more efficient.
Just wanted to say thanks to @iain-buclaw-sociomantic. I was experiencing this issue and thought I was losing my mind.
Thank you for your work!
Given the following carbon config:
And the equivalent carbon-c-relay config:
They both produce different hash rings.
Carbon:
Carbon-c-relay:
This means that any tooling that uses carbon's consistent hash ring (such as carbonate) can not be used with carbon-c-relay. However I'm not sure whether who has the bug here in this discrepancy.