Needs work
Project:
Redis
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
30 Dec 2016 at 09:24 UTC
Updated:
20 Jan 2025 at 13:17 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
berdirYeah, that would make sense, patches welcome.
Switching will not be trivial however, as key_value contains a lot of crucial information, so if you want to use it, you *must* configure redis to persist the data, so you might even want to have a separate redis instance than the one you use for cache/temporary data, with different settings.
key_value_expire might be easier to support.
Comment #3
pounardNot necessarily, the permanent items cache lifetime feature that exists since 7.x-2.x can solve this, by forcing a TTL/EXPIRE on all cache items, even permanent ones, you can safely use the same Redis server configured with no global LRU - when memory goes missing, cache items are dropped, not K/V one since they won't be expireable.
Comment #4
kfritscheI started working on the issue and have a first draft ready for the KeyValue store. Next step for me is to also do the same for KeyValueExpire, which shouldn't be so complicated as I probably could re-use a lot from KeyValue. After that this also needs to be working for predis.
But I identified two issues at the moment, where I'm a little bit stuck.
1. Initial fill of KeyValues. To have the possibility to enable this service later when chaning service.yml you need copy the values from the DB into redis. I have a small script ready, which does this. So one solution would be to have a drush command or similar which needs to be called, after the service is enabled. Or the user has to call the fill function in a update hook manually. Both sounds for me a bit strange.
2. To test if all is running, the best thing would be to run the core tests with redis enabled. Is there a more easy method than extend the core tests and overwrite the setup method? Cool would be also to have maybe functional tests running, to see if all works.
Comment #5
socialnicheguru commentedComment #6
jmullikin commented@kfritsche I'd be interested in working with you to expand this to KeyValueExpirable in a way they can be enabled independently. I'm on a quest to make Drupal multi-region ready in AWS and allow use of read-only databases, and KeyValueExpirable in Redis is the final step.
Comment #7
pounard@kfritsche
Another solution is to make it a decorator of the core implementation: load the key, and fallback on mlss onto the default backend. Same goes for writes: write in both backends.
Comment #8
dave reidI have written a patch to add support for the key/value expirable in #3013411: Backend for key/value expirable store since I consider it a separate implementation from this, but re-uses some of the code to make it possible in the future to have a non-expirable Redis backend. There's also a bug in the getAllKeys() method in the patch above that will not return all possible keys because sometimes the iterator will return no results, but you still need to keep looking. That has been fixed in the new issue's patch.
Comment #9
berdirI agree with #7, that seems like a very interesting approach, it's basically a cache wrapper around the existing implementation then. Allows to add and remove that any time, and would also not require any dev/local installation to use redis as well (and sync that data).
I guess the expirable implementation then couldn't really share much of the code then, unless we make the persistent backend optional and wrap everything in either and if or loops for possibly multiple backends. We could have two implementations, the actual redis implementation of the interface and a chain implementation. Just like \Drupal\Core\Cache\BackendChain. And then expirable can use the normal base class.
Comment #10
fgmFWIW if you want to look at it, that feature is implemented in the MongoDB project (8.x-2.x): it may give ideas for the Redis project.
Comment #11
mounir_abid commentedHello,
I was looking for a way to decrease database calls for "key_value" data.
I tested the patch but I have some bugs.
So I tried to do as suggested in comment #7 by @pounard.
A decorator to use redis as a "proxy cache",
- writes are done in database and redis,
- reads are done in redis with a fallback in database (and resynchronized in redis).
Comment #12
berdirI suggest you look at #2575105: Use cache collector for state, that will likely replace almost all state related queries with a single cache get. If you still have a lot of requests after that patch I suggest you investigate where exactly they are coming from and try to optimize that.
Comment #13
socialnicheguru commentedWhen applying this I got the following:
TypeError: Typed property Drupal\redis\KeyValueStore\PhpRedis::$client must be an instance of Drupal\redis\KeyValueStore\mixed, Redis used in Drupal\redis\KeyValueStore\PhpRedis->__construct() (line 26 of drupal9.3/html/modules/contrib/redis/src/KeyValueStore/PhpRedis.php) #0 drupal9.3/html/modules/contrib/redis/src/KeyValueStore/KeyValueRedisFactory.php(49): Drupal\redis\KeyValueStore\PhpRedis->__construct()
Comment #14
artemboikoUpdated patch from #11 by @mounir_abid
Corrected paths to files and name.
I didn't test deep. But webprofiler shows that queries to key value table disappear.
Comment #18
pfructuoso commentedI created a new issue #3467120: Use redis as replacement for core's database driven key value store AND key/value expirable store to unify changes this task and #3013411: Backend for key/value expirable store
Comment #19
socialnicheguru commentedDoes not work on redis 1.9.