autorehash in RedisArray constructor options Doesn't Work #294

Closed
mocheng opened this Issue Feb 2, 2013 · 6 comments

Comments

Projects
None yet
5 participants
@mocheng

mocheng commented Feb 2, 2013

I tried main branch and tag 2.2.2. It turns out that, as code below, autorehash doesn't trigger migration of keys among nodes.

$options = array(0;
$options['previous'] = $previous_server_list;
$options['autorehash'] = TRUE;
$ra = new RedisArray($server_list, $options);

Is there something missing in https://github.com/nicolasff/phpredis/blob/master/arrays.markdown#readme ?

@michael-grunder

This comment has been minimized.

Show comment Hide comment
@michael-grunder

michael-grunder Feb 3, 2013

Member

Hey there,

I will try to replicate this problem for you tonight or tomorrow.

Cheers,
Mike

Member

michael-grunder commented Feb 3, 2013

Hey there,

I will try to replicate this problem for you tonight or tomorrow.

Cheers,
Mike

@nicolasff

This comment has been minimized.

Show comment Hide comment
@nicolasff

nicolasff Feb 17, 2013

Contributor

Morgan,

The "autorehash" feature is tested and the tests are currently passing on master.
Reading a key from the $server_list can fail if the it is different from $previous_server_list. When this happens, the RedisArray object will look it up in the previous ring. When autorehash is enabled, this read will trigger a write to the new ring, so that subsequent reads are resolved in a single step.

Have a look at tests/array-tests.php, in the Redis_Auto_Rehashing_Test class. It writes 1,000 keys to a ring, then adds a node to the ring, reads back all the keys (triggering a rehash and migrating them one by one), before checking that they have all been migrated successfully.

Contributor

nicolasff commented Feb 17, 2013

Morgan,

The "autorehash" feature is tested and the tests are currently passing on master.
Reading a key from the $server_list can fail if the it is different from $previous_server_list. When this happens, the RedisArray object will look it up in the previous ring. When autorehash is enabled, this read will trigger a write to the new ring, so that subsequent reads are resolved in a single step.

Have a look at tests/array-tests.php, in the Redis_Auto_Rehashing_Test class. It writes 1,000 keys to a ring, then adds a node to the ring, reads back all the keys (triggering a rehash and migrating them one by one), before checking that they have all been migrated successfully.

@mocheng

This comment has been minimized.

Show comment Hide comment
@mocheng

mocheng Feb 21, 2013

Thanks for your reply. I'll try it out:)

mocheng commented Feb 21, 2013

Thanks for your reply. I'll try it out:)

@chaosue

This comment has been minimized.

Show comment Hide comment
@chaosue

chaosue Jul 29, 2013

not work!

the Redis_Auto_Rehashing_Test passed just because there's a Redis_Rehashing_Tes before it.
If you comment "run_tests('Redis_Rehashing_Test') " then run_tests('Redis_Auto_Rehashing_Test') fails.

chaosue commented Jul 29, 2013

not work!

the Redis_Auto_Rehashing_Test passed just because there's a Redis_Rehashing_Tes before it.
If you comment "run_tests('Redis_Rehashing_Test') " then run_tests('Redis_Auto_Rehashing_Test') fails.

@koda0611

This comment has been minimized.

Show comment Hide comment
@koda0611

koda0611 Mar 7, 2014

I just tried this today, and it doesn't auto rehash. Can anyone verify that this is working?

koda0611 commented Mar 7, 2014

I just tried this today, and it doesn't auto rehash. Can anyone verify that this is working?

michael-grunder added a commit that referenced this issue Mar 10, 2014

Fix autorehashing in RedisArray
This commit fixes auto rehashing in RedisArray as well as fixes
a couple of memory leaks found along the way

Addresses #442 and #294
@michael-grunder

This comment has been minimized.

Show comment Hide comment
@michael-grunder

michael-grunder Mar 10, 2014

Member

Hey there, can you please test the branch hotfix/ra_autorehash. I believe this commit sorts it.

Member

michael-grunder commented Mar 10, 2014

Hey there, can you please test the branch hotfix/ra_autorehash. I believe this commit sorts it.

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