Hi,

void dbOverwrite(redisDb *db, robj *key, robj *val) {
    ...
    if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU) {
        val->lru = old->lru;
    }
    ...
}

I noticed that dbOverwrite will update the LFU, but when the old key and new key types are different, is this correct?

E.g: georadius with STORE key option, when the key exists, it will call setKey() -> dbOverwrite(), see here, if the key is not a GEO type before(maybe string), it makes sense to update the LFU of the new key ?

Unlike it, sunstorestore will dbDelete the old key first, and then dbAdd the new key, see here, there will be no LFU updates.

So, my questions are: 1. Does dbOverwrite need to decide whether to update LFU based on whether the key types are consistent? For example：

void dbOverwrite(redisDb *db, robj *key, robj *val, robj *oldval) { // pass oldval
    ...
    if (oldval->type == val->type) {  // judge type
        if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU) {
            val->lru = old->lru;
        }
    }
    ...
}
  1. Related to the answer to the first question, which of the current code, georadius and sunstorestore, is correct?

Comment From: oranagra

@yangbodong22011 thanks for raising this. I do not think the type change should make a difference. One can argue that even if you're overriding a key of the same type, you need to start over with a fresh LFU. I do see that retaining the old LFU is something that exists since the day LFU was implemented. Arguably the LFU is a property of the keyname not the value.. maybe if someone comes with a strong argument against it we should consider if we wanna change that.

regarding the differences between SUNIONSTORE and GEORADIUS STORE, the inconsistencies between these (including ZUNIONSTORE, and SORT) are a pain point that we need to address. they have several other inconsistencies between these.

with regards to your question, i think GEORADIUS is more correct (using the dbOverwrite mechanism).

Comment From: soloestoy

Arguably the LFU is a property of the keyname not the value..

Agree, so I think we need replace dbDelete/dbAdd with setKey in SUNIONSTORE..

Comment From: yangbodong22011

@oranagra @soloestoy Thanks for your reply I will try to use dbOverwrite to fix this kind of problem like SUNIONSTORE (ZUNIONSTORE, etc.).

Comment From: oranagra

@yangbodong22011 thank you. #7489 merged.