valkey icon indicating copy to clipboard operation
valkey copied to clipboard

Unshare object to avoid LRU/LFU being messed up.

Open CharlesChen888 opened this issue 1 year ago • 1 comments

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.

CharlesChen888 avatar Apr 07 '24 09:04 CharlesChen888

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 70.17%. Comparing base (1c55f3c) to head (43584f9). Report is 14 commits behind head on unstable.

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     
Files Coverage Δ
src/db.c 88.30% <100.00%> (+0.03%) :arrow_up:
src/object.c 78.60% <100.00%> (+0.02%) :arrow_up:
src/server.h 100.00% <100.00%> (ø)

... and 13 files with indirect coverage changes

codecov[bot] avatar May 06 '24 03:05 codecov[bot]