Unshare object to avoid LRU/LFU being messed up. #250
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CharlesChen888
force-pushed
the
unshare-obj-when-lrulfu
branch
from
29c44ec to
73395f3
Compare
zuiderkwast
reviewed
May 2, 2024
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #250 +/- ##
============================================
- Coverage 70.19% 70.17% -0.03%
============================================
Files 109 110 +1
Lines 59905 59889 -16
============================================
- Hits 42050 42026 -24
- Misses 17855 17863 +8
|
Signed-off-by: chentianjie.ctj <chentianjie.ctj@alibaba-inc.com>
Signed-off-by: Chen Tianjie <TJ_Chen@outlook.com>
Signed-off-by: Chen Tianjie <TJ_Chen@outlook.com>
CharlesChen888
force-pushed
the
unshare-obj-when-lrulfu
branch
from
aef424b to
43584f9
Compare
This was referenced May 27, 2024
zuiderkwast
approved these changes
Jun 1, 2024
zuiderkwast
added
bug
naglera
pushed a commit
to naglera/placeholderkv
that referenced
this pull request
Jun 10, 2024
When LRU/LFU enabled, Valkey does not allow using shared objects, as value objects may be shared among many different keys and they can't share LRU/LFU information. However `maxmemory-policy` is modifiable at runtime. If LRU/LFU is not enabled at start, but then enabled when some shared objects are already used, there could be some confusion in LRU/LFU information. For `set` command it is OK since it is going to create a new object when LRU/LFU enabled, but `get` command will not unshare the object and just update LRU/LFU information. So we may duplicate the object in this case. It is a one-time task for each key using shared objects, unless this is the case for so many keys, there should be no serious performance degradation. Still, LRU will be updated anyway, no matter LRU/LFU is enabled or not, because `OBJECT IDLETIME` needs it, unless `maxmemory-policy` is set to LFU. So idle time of a key may still be messed up. --------- Signed-off-by: chentianjie.ctj <chentianjie.ctj@alibaba-inc.com> Signed-off-by: Chen Tianjie <TJ_Chen@outlook.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When LRU/LFU enabled, Valkey does not allow using shared objects, as value objects may be shared among many different keys and they can't share LRU/LFU information.
However
maxmemory-policyis modifiable at runtime. If LRU/LFU is not enabled at start, but then enabled when some shared objects are already used, there could be some confusion in LRU/LFU information.For
setcommand it is OK since it is going to create a new object when LRU/LFU enabled, butgetcommand will not unshare the object and just update LRU/LFU information.So we may duplicate the object in this case. It is a one-time task for each key using shared objects, unless this is the case for so many keys, there should be no serious performance degradation.
Still, LRU will be updated anyway, no matter LRU/LFU is enabled or not, because
OBJECT IDLETIMEneeds it, unlessmaxmemory-policyis set to LFU. So idle time of a key may still be messed up.