Update from murmur2 to murmur3 hash.

Bug #738235 reported by Bob Vincent on 2011-03-19
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libmemcached
Low
Brian Aker

Bug Description

Updated libhashkit/murmur.c from murmur2 to murmur3.

Related branches

Bob Vincent (pillarsdotnet) wrote :
Brian Aker (brianaker) wrote :

I get a crash after applying your code:

Testing set==27877== Invalid write of size 4
==27877== at 0x4C1C6FC: hashkit_murmur (murmur.c:91)
==27877== by 0x4C142ED: memcached_generate_hash_with_redistribution (hash.c:115)

Wouldn't we want to have a murmur version 2 and 3 so that anyone who is using it today won't end up with data on different nodes?

Changed in libmemcached:
assignee: nobody → Brian Aker (brianaker)
status: New → Incomplete

On 03/24/11 20:15, Brian Aker wrote:
> I get a crash after applying your code:
>
>
> Testing set==27877== Invalid write of size 4
> ==27877== at 0x4C1C6FC: hashkit_murmur (murmur.c:91)
> ==27877== by 0x4C142ED: memcached_generate_hash_with_redistribution (hash.c:115)
Funny; all the tests completed successfully here. OTOH, the original code
had a bunch of variants (32-bit/64-bit/128-bit/big-endian/little-endian/etc)
and I couldn't get the original to compile cleanly on my setup, so I cut-and
pasted what seemed like the 32-bit version.

(checking...)

It appears that the original at http://smhasher.googlecode.com/svn/trunk/
has been updated since I made my patch. I will try to update shortly.

>
> Wouldn't we want to have a murmur version 2 and 3 so that anyone who is
> using it today won't end up with data on different nodes?
If that were true, I would expect to find a murmur1.c and a murmur2.c file.

Instead, I find a murmur.c file with header comments indicating that it had
been upgraded from murmur to murmur2.

Seriously -- is anybody using memcached in such a way that they would
upgrade the program in-place without restarting the process and thus
clearing the memory cache?

Or maybe I don't understand you correctly.

Brian Aker (brianaker) wrote :

Hi!

On Mar 25, 2011, at 11:58 AM, Bob Vincent wrote:

> Instead, I find a murmur.c file with header comments indicating that it had
> been upgraded from murmur to murmur2.

We never released murmur1 support very wide so with 2 it was pretty simple.

> Seriously -- is anybody using memcached in such a way that they woul
> upgrade the program in-place without restarting the process and thus
> clearing the memory cache?

Yes, that is how many people upgrade. Changing hashing would certainly cause issues.

Cheers,
 -Brian

Bob Vincent (pillarsdotnet) wrote :

On 03/25/11 15:32, Brian Aker wrote:
>
> Yes, that is how many people upgrade. Changing hashing would certainly
> cause issues.
>
> Cheers,
> -Brian
>
Can you explain, or point me to an explanation, as to how this works?

Also, what is the relationship between the hash used in libmemcached and
the hash used in memcached?

Brian Aker (brianaker) wrote :

Hi!

On Mar 25, 2011, at 1:13 PM, Bob Vincent wrote:

> Can you explain, or point me to an explanation, as to how this works?

When someone upgrades the driver they already have servers in place which are storing data. If the hash changes they won't be able to find their data.

> Also, what is the relationship between the hash used in libmemcached and
> the hash used in memcached?

Libmemcached does all mapping, the hash inside of memcached is just for storage in the hash table.

Cheers,
 -Brian

Brian Aker (brianaker) on 2012-11-21
Changed in libmemcached:
milestone: none → 1.0.15
importance: Undecided → Low
status: Incomplete → In Progress
Brian Aker (brianaker) wrote :

Murmur3 support has now been added as a distinct type.

Changed in libmemcached:
status: In Progress → Fix Committed
Brian Aker (brianaker) on 2012-12-23
Changed in libmemcached:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers