Add DataStore::RedisMulti #2443
Conversation
mwiencek
force-pushed
the
redis-multi-store
branch
6 times, most recently
from
f03ad02
to
3dcae74
Compare
|
Outside of our current type of ordeal (migrating one standalone Redis instance to another server), this likely won't be as useful if we have a multi-master Redis/KeyDB cluster in the future. But, it could be used for migrating to such a setup more easily. Or perhaps we may need to migrate from one cluster to another for some reason? |
| $redis_multi->delete('string'); | ||
| is($redis_multi->exists('string'), 0, 'exists returns 0 for deleted key'); | ||
| is($redis1->exists('string'), 0, 'exists returns 0 for deleted key in database 1'); | ||
| is($redis2->exists('string'), 0, 'exists returns 0 for deleted key in database 2'); |
There was a problem hiding this comment.
What should happen if you delete from $redis1 but not $redis2? Should we have a test for that case?
There was a problem hiding this comment.
It'd only be deleted from $redis1, but any servers with RedisMulti enabled would still see the key if it exists in $redis2.
This shouldn't really happen in practice, because we'd be deploying the RedisMulti configuration on all servers at once. But there is a minor race condition where a key is set on a server with a RedisMulti configuration, and then deleted on a server that doesn't have the new configuration yet (because we're in the middle of deploying it). The only situation I can see this happening is if a user logs in and then logs out; this would have to be perfectly timed and forwarded to the right servers. In that case there may be a stale session key on the new Redis instance, but it would expire within some hours anyway.
Test-wise we do have the "string-1-only"/"string-2-only" ones here, which check that keys set in only one of the servers will be retrieved through $redis_multi (doesn't matter if they're deleted first).
There was a problem hiding this comment.
Perfect then, I was just wondering if, since adding to one adds to multi, deleting from one should delete from multi :) All clear now.
mwiencek
force-pushed
the
redis-multi-store
branch
from
3dcae74
to
93ae0b4
Compare
These are all implemented by `DataStore::Redis`, which is currently the only implementation of the `DataStore` role -- so as it turns out, attributes that have `does => 'MusicBrainz::DataStore'` already expect these to be implemented.
remove is an alias for delete, which already calls _prepare_key.
If the `DataStore::RedisMulti` module is active, then `DATASTORE_REDIS_ARGS` may return an array ref of connection details. (How do you know if it's active? grep for `DataStore::RedisMulti->new`. We may revert back to plain-old `DataStore::Redis` if multiple instances aren't needed.) This module is useful when Redis service needs to be migrated to a new server. We'll attempt to read from each connection in order (returning the first non-empty result), and also distribute writes to all connections. This allows time to copy any keys that don't exist on the new instance from the old instance.
mwiencek
force-pushed
the
redis-multi-store
branch
from
93ae0b4
to
3f4d926
Compare
If the
DataStore::RedisMultimodule is active, thenDATASTORE_REDIS_ARGSmay return an array ref of connection details. (How do you know if it's active? grep forDataStore::RedisMulti->new. We may revert back to plain-oldDataStore::Redisif multiple instances aren't needed.)This module is useful when Redis service needs to be migrated to a new server. We'll attempt to read from each connection in order (returning the first non-empty result), and also distribute writes to all connections. This allows time to copy any keys that don't exist on the new instance from the old instance.