[BUG] LFU/LRU is not applied to shared objects

[VoletiRam]

Describe the bug

LFU/LRU is not applied to the shared objects. This PR: https://github.com/valkey-io/valkey/commit/d71478a88 delays the initialization of LRU/LFU for objects until it is added to the db as a value. Currently, we are first marking the objects as shared and then initializing the LFU/LRU here. The would return, not adding LRU/LFU information to the object, as we have a check inside initObjectLRUOrLFU() to return for shared objects.

void initObjectLRUOrLFU(robj *o) {
    if (o->refcount == OBJ_SHARED_REFCOUNT)
        return;

All shared objects had default initialized to zero now. When we overwrite shared object with regular object, the old LRU/LFU value will be assumed to new object and incorrectly refer to least frequently a item.

To reproduce

test {lfu value of the shared object key} {
        r config set maxmemory 0
        r config set maxmemory-policy allkeys-lfu
        r incr foo
        assert {[getlru foo] > 0}

Expected behavior

Shared objects are assigned with LRU/LFU information.

Additional information

Any additional information that is relevant to the problem.

[enjoy-binbin]

for the record: if we want to be consistent with the previous version (redis7), we can simply remove the shared judgment in initObjectLRUOrLFU. However, because the shared object is shared, the original lru information is inaccurate.

This seems to be related to https://github.com/redis/redis/pull/13134, are you still willing to continue it? @CharlesChen888

[VoletiRam]

I raised a pull request that will fix the initial LRU/LFU information for shared object. Please let me know if it looks good. @enjoy-binbin

cc @madolson

[hpatro]

@VoletiRam Could you point the PR to Valkey and not your fork?

[VoletiRam]

Here it is https://github.com/valkey-io/valkey/pull/228

[CharlesChen888]

Shared objects may be shared as values among more than one keys, and this may cause confusion in LRU/LFU info. Currently we don't use shared objects when LRU/LFU is enabled to avoid this. So there is no need to apply LRU/LFU info to shared objects.

The main problem is, as I stated in redis/redis#13134, maxmemory-policy is modifiable at runtime, and 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.

May be I need to move that PR to Valkey.

[CharlesChen888]

Forgot to mention it here. I moved redis/redis#13134 to Valkey. The new PR is #250.

[hpatro]

Fixed by https://github.com/valkey-io/valkey/pull/250