Cm_Cache_Backend_Redis
Cm_Cache_Backend_Redis copied to clipboard
colinmollenhour
switch from del() to unklink() for redis >= 4.0
With redis 4.0 there is a new unlink command intruded ( https://redis.io/commands/unlink ) with is a smarter version of del() as it deletes keys in O(1) and frees the memory in background.
See also http://antirez.com/news/93 and https://github.com/redis/redis/issues/1748
This won't change the behavior but only the memory usage as clarified here: https://github.com/redis/redis/issues/1748#issuecomment-43233084 which shouldn't be an issue with most Magento instances
With this, I recomand replacing DEL operations with UNLINK operations.
Hmm, I wonder if there is a potential issue with this since often times when keys are deleted they are just going to be repopulated again shortly after. That is, what is the behavior when a key is set while it is also queued to be deleted in the background?
From my understanding (see https://github.com/redis/redis/issues/1748#issuecomment-43233084 ) this shouldn't be an issue as the behavior doesn't change between DEL and UNLINK. Only difference is, that memory is not imminently freed which should only matter, when all keys are deleted and immediately recreated in a very low memory environment.
As magento cache takes some time to reach is full size from my observations, I don't see an issue with this either.
Can also easily be confirmed on (bash) CLI:
printf "SET fuu bar\nGET fuu\n UNLINK fuu\nSET fuu fubar\n GET fuu" | redis-cli
OK
"bar"
(integer) 1
OK
"fubar"
printf "SET fuu bar\nGET fuu\n DEL fuu\nSET fuu fubar\n GET fuu" | redis-cli
OK
"bar"
(integer) 1
OK
"fubar"
Running the commands on a single client may not be sufficient to prove the issue does not exist in a multi-client environment. Perhaps you'd like to submit a PR and run it for a while yourself and monitor for issues? I'm not comfortable making this change yet and don't have time to test it myself right now. Thanks for the suggestion and your help!
I created a PR (#160) and installed it on this store https://magento2-demoshop-maxcluster.de/
As I'm working for a hosting provider I don't have any "real shop" to test it with through.
it can also bee seen here btw. https://github.com/redis/redis/blob/unstable/src/lazyfree.c#L78
The KEY is always immediately set to a NULL pointer, so there is no reference anymore to the old key.
https://github.com/colinmollenhour/credis/issues/126#issuecomment-802950486
For anyone wanting the async nature of UNLINK for DEL commands, you can enable that for all DEL commands in Redis config:
lazyfree-lazy-eviction yes
lazyfree-lazy-expire yes
lazyfree-lazy-server-del yes
lazyfree-lazy-user-del yes
replica-lazy-flush yes
lazyfree-lazy-user-del is available in Redis 6.
Is there any progress in this issue? I came here from https://github.com/OpenMage/magento-lts/pull/1173 and it seems to me that this topic has been forgotten. Would it help if I deploy this change to our testing environment and report back?
@tobihille The PR has requested changes: https://github.com/colinmollenhour/Cm_Cache_Backend_Redis/pull/160#pullrequestreview-616674664 Otherwise no objection to merging.