Merge pull request #6557 from oranagra/rm_lru_lfu_revized

rename RN_SetLRUOrLFU -> RM_SetLRU and RN_SetLFU

src/cluster.c

@@ -4966,7 +4966,7 @@ void restoreCommand(client *c) {
         if (!absttl) ttl+=mstime();
         setExpire(c,c->db,c->argv[1],ttl);
     }
-    objectSetLRUOrLFU(obj,lfu_freq,lru_idle,lru_clock);
+    objectSetLRUOrLFU(obj,lfu_freq,lru_idle,lru_clock,1000);
     signalModifiedKey(c->db,c->argv[1]);
     addReply(c,shared.ok);
     server.dirty++;

src/module.c

@@ -7035,35 +7035,53 @@ size_t moduleCount(void) {
     return dictSize(modules);
 }
 
-/* Set the key LRU/LFU depending on server.maxmemory_policy.
- * The lru_idle arg is idle time in seconds, and is only relevant if the
- * eviction policy is LRU based.
- * The lfu_freq arg is a logarithmic counter that provides an indication of
- * the access frequencyonly (must be <= 255) and is only relevant if the
- * eviction policy is LFU based.
- * Either or both of them may be <0, in that case, nothing is set. */
-/* return value is an indication if the lru field was updated or not. */
-int RM_SetLRUOrLFU(RedisModuleKey *key, long long lfu_freq, long long lru_idle) {
+/* Set the key last access time for LRU based eviction. not relevent if the
+ * servers's maxmemory policy is LFU based. Value is idle time in milliseconds.
+ * returns REDISMODULE_OK if the LRU was updated, REDISMODULE_ERR otherwise. */
+int RM_SetLRU(RedisModuleKey *key, mstime_t lru_idle) {
     if (!key->value)
         return REDISMODULE_ERR;
-    if (objectSetLRUOrLFU(key->value, lfu_freq, lru_idle, lru_idle>=0 ? LRU_CLOCK() : 0))
+    if (objectSetLRUOrLFU(key->value, -1, lru_idle, lru_idle>=0 ? LRU_CLOCK() : 0, 1))
         return REDISMODULE_OK;
     return REDISMODULE_ERR;
 }
 
-/* Gets the key LRU or LFU (depending on the current eviction policy).
- * One will be set to the appropiate return value, and the other will be set to -1.
- * see RedisModule_SetLRUOrLFU for units and ranges.
- * return value is an indication of success. */
-int RM_GetLRUOrLFU(RedisModuleKey *key, long long *lfu_freq, long long *lru_idle) {
-    *lru_idle = *lfu_freq = -1;
+/* Gets the key last access time.
+ * Value is idletime in milliseconds or -1 if the server's eviction policy is
+ * LFU based.
+ * returns REDISMODULE_OK if when key is valid. */
+int RM_GetLRU(RedisModuleKey *key, mstime_t *lru_idle) {
+    *lru_idle = -1;
     if (!key->value)
         return REDISMODULE_ERR;
-    if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU) {
-        *lfu_freq = LFUDecrAndReturn(key->value);
-    } else {
-        *lru_idle = estimateObjectIdleTime(key->value)/1000;
+    if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU)
+        return REDISMODULE_OK;
+    *lru_idle = estimateObjectIdleTime(key->value);
+    return REDISMODULE_OK;
 }
+
+/* Set the key access frequency. only relevant if the server's maxmemory policy
+ * is LFU based.
+ * The frequency is a logarithmic counter that provides an indication of
+ * the access frequencyonly (must be <= 255).
+ * returns REDISMODULE_OK if the LFU was updated, REDISMODULE_ERR otherwise. */
+int RM_SetLFU(RedisModuleKey *key, long long lfu_freq) {
+    if (!key->value)
+        return REDISMODULE_ERR;
+    if (objectSetLRUOrLFU(key->value, lfu_freq, -1, 0, 1))
+        return REDISMODULE_OK;
+    return REDISMODULE_ERR;
+}
+
+/* Gets the key access frequency or -1 if the server's eviction policy is not
+ * LFU based.
+ * returns REDISMODULE_OK if when key is valid. */
+int RM_GetLFU(RedisModuleKey *key, long long *lfu_freq) {
+    *lfu_freq = -1;
+    if (!key->value)
+        return REDISMODULE_ERR;
+    if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU)
+        *lfu_freq = LFUDecrAndReturn(key->value);
     return REDISMODULE_OK;
 }
 
@@ -7294,8 +7312,10 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(GetClientInfoById);
     REGISTER_API(PublishMessage);
     REGISTER_API(SubscribeToServerEvent);
-    REGISTER_API(SetLRUOrLFU);
-    REGISTER_API(GetLRUOrLFU);
+    REGISTER_API(SetLRU);
+    REGISTER_API(GetLRU);
+    REGISTER_API(SetLFU);
+    REGISTER_API(GetLFU);
     REGISTER_API(BlockClientOnKeys);
     REGISTER_API(SignalKeyAsReady);
     REGISTER_API(GetBlockedClientReadyKey);

src/object.c

@@ -1210,7 +1210,7 @@ sds getMemoryDoctorReport(void) {
  * is MAXMEMORY_FLAG_LRU.
  * Either or both of them may be <0, in that case, nothing is set. */
 int objectSetLRUOrLFU(robj *val, long long lfu_freq, long long lru_idle,
-                       long long lru_clock) {
+                       long long lru_clock, int lru_multiplier) {
     if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU) {
         if (lfu_freq >= 0) {
             serverAssert(lfu_freq <= 255);
@@ -1222,7 +1222,7 @@ int objectSetLRUOrLFU(robj *val, long long lfu_freq, long long lru_idle,
          * according to the LRU clock resolution this Redis
          * instance was compiled with (normally 1000 ms, so the
          * below statement will expand to lru_idle*1000/1000. */
-        lru_idle = lru_idle*1000/LRU_CLOCK_RESOLUTION;
+        lru_idle = lru_idle*lru_multiplier/LRU_CLOCK_RESOLUTION;
         long lru_abs = lru_clock - lru_idle; /* Absolute access time. */
         /* If the LRU field underflows (since LRU it is a wrapping
          * clock), the best we can do is to provide a large enough LRU

src/rdb.c

@@ -2239,7 +2239,7 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) {
             if (expiretime != -1) setExpire(NULL,db,key,expiretime);
 
             /* Set usage information (for eviction). */
-            objectSetLRUOrLFU(val,lfu_freq,lru_idle,lru_clock);
+            objectSetLRUOrLFU(val,lfu_freq,lru_idle,lru_clock,1000);
 
             /* Decrement the key refcount since dbAdd() will take its
              * own reference. */

src/redismodule.h

@@ -589,8 +589,10 @@ int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldDouble)(RedisModuleInfoCtx *ctx
 int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldLongLong)(RedisModuleInfoCtx *ctx, char *field, long long value);
 int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldULongLong)(RedisModuleInfoCtx *ctx, char *field, unsigned long long value);
 int REDISMODULE_API_FUNC(RedisModule_SubscribeToServerEvent)(RedisModuleCtx *ctx, RedisModuleEvent event, RedisModuleEventCallback callback);
-int REDISMODULE_API_FUNC(RedisModule_SetLRUOrLFU)(RedisModuleKey *key, long long lfu_freq, long long lru_idle);
-int REDISMODULE_API_FUNC(RedisModule_GetLRUOrLFU)(RedisModuleKey *key, long long *lfu_freq, long long *lru_idle);
+int REDISMODULE_API_FUNC(RedisModule_SetLRU)(RedisModuleKey *key, mstime_t lru_idle);
+int REDISMODULE_API_FUNC(RedisModule_GetLRU)(RedisModuleKey *key, mstime_t *lru_idle);
+int REDISMODULE_API_FUNC(RedisModule_SetLFU)(RedisModuleKey *key, long long lfu_freq);
+int REDISMODULE_API_FUNC(RedisModule_GetLFU)(RedisModuleKey *key, long long *lfu_freq);
 RedisModuleBlockedClient *REDISMODULE_API_FUNC(RedisModule_BlockClientOnKeys)(RedisModuleCtx *ctx, RedisModuleCmdFunc reply_callback, RedisModuleCmdFunc timeout_callback, void (*free_privdata)(RedisModuleCtx*,void*), long long timeout_ms, RedisModuleString **keys, int numkeys, void *privdata);
 void REDISMODULE_API_FUNC(RedisModule_SignalKeyAsReady)(RedisModuleCtx *ctx, RedisModuleString *key);
 RedisModuleString *REDISMODULE_API_FUNC(RedisModule_GetBlockedClientReadyKey)(RedisModuleCtx *ctx);
@@ -808,8 +810,10 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int
     REDISMODULE_GET_API(GetClientInfoById);
     REDISMODULE_GET_API(PublishMessage);
     REDISMODULE_GET_API(SubscribeToServerEvent);
-    REDISMODULE_GET_API(SetLRUOrLFU);
-    REDISMODULE_GET_API(GetLRUOrLFU);
+    REDISMODULE_GET_API(SetLRU);
+    REDISMODULE_GET_API(GetLRU);
+    REDISMODULE_GET_API(SetLFU);
+    REDISMODULE_GET_API(GetLFU);
     REDISMODULE_GET_API(BlockClientOnKeys);
     REDISMODULE_GET_API(SignalKeyAsReady);
     REDISMODULE_GET_API(GetBlockedClientReadyKey);

src/server.h

@@ -2093,7 +2093,7 @@ robj *lookupKeyWriteWithFlags(redisDb *db, robj *key, int flags);
 robj *objectCommandLookup(client *c, robj *key);
 robj *objectCommandLookupOrReply(client *c, robj *key, robj *reply);
 int objectSetLRUOrLFU(robj *val, long long lfu_freq, long long lru_idle,
-                       long long lru_clock);
+                       long long lru_clock, int lru_multiplier);
 #define LOOKUP_NONE 0
 #define LOOKUP_NOTOUCH (1<<0)
 void dbAdd(redisDb *db, robj *key, robj *val);

tests/modules/misc.c

@@ -68,16 +68,24 @@ int test_randomkey(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
     return REDISMODULE_OK;
 }
 
+RedisModuleKey *open_key_or_reply(RedisModuleCtx *ctx, RedisModuleString *keyname, int mode) {
+    RedisModuleKey *key = RedisModule_OpenKey(ctx, keyname, mode);
+    if (!key) {
+        RedisModule_ReplyWithError(ctx, "key not found");
+        return NULL;
+    }
+    return key;
+}
+
 int test_getlru(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 {
     if (argc<2) {
         RedisModule_WrongArity(ctx);
         return REDISMODULE_OK;
     }
-    RedisModuleString *keyname = argv[1];
-    RedisModuleKey *key = RedisModule_OpenKey(ctx, keyname, REDISMODULE_READ|REDISMODULE_OPEN_KEY_NOTOUCH);
-    long long lru, lfu;
-    RedisModule_GetLRUOrLFU(key, &lfu, &lru);
+    RedisModuleKey *key = open_key_or_reply(ctx, argv[1], REDISMODULE_READ|REDISMODULE_OPEN_KEY_NOTOUCH);
+    mstime_t lru;
+    RedisModule_GetLRU(key, &lru);
     RedisModule_ReplyWithLongLong(ctx, lru);
     RedisModule_CloseKey(key);
     return REDISMODULE_OK;
@@ -89,12 +97,46 @@ int test_setlru(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
         RedisModule_WrongArity(ctx);
         return REDISMODULE_OK;
     }
-    RedisModuleString *keyname = argv[1];
-    RedisModuleKey *key = RedisModule_OpenKey(ctx, keyname, REDISMODULE_WRITE|REDISMODULE_OPEN_KEY_NOTOUCH);
-    long long lru;
-    RedisModule_StringToLongLong(argv[2], &lru);
-    RedisModule_SetLRUOrLFU(key, -1, lru);
-    RedisModule_ReplyWithCString(ctx, "Ok");
+    RedisModuleKey *key = open_key_or_reply(ctx, argv[1], REDISMODULE_READ|REDISMODULE_OPEN_KEY_NOTOUCH);
+    mstime_t lru;
+    if (RedisModule_StringToLongLong(argv[2], &lru) != REDISMODULE_OK) {
+        RedisModule_ReplyWithError(ctx, "invalid idle time");
+        return REDISMODULE_OK;
+    }
+    int was_set = RedisModule_SetLRU(key, lru)==REDISMODULE_OK;
+    RedisModule_ReplyWithLongLong(ctx, was_set);
+    RedisModule_CloseKey(key);
+    return REDISMODULE_OK;
+}
+
+int test_getlfu(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
+{
+    if (argc<2) {
+        RedisModule_WrongArity(ctx);
+        return REDISMODULE_OK;
+    }
+    RedisModuleKey *key = open_key_or_reply(ctx, argv[1], REDISMODULE_READ|REDISMODULE_OPEN_KEY_NOTOUCH);
+    mstime_t lfu;
+    RedisModule_GetLFU(key, &lfu);
+    RedisModule_ReplyWithLongLong(ctx, lfu);
+    RedisModule_CloseKey(key);
+    return REDISMODULE_OK;
+}
+
+int test_setlfu(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
+{
+    if (argc<3) {
+        RedisModule_WrongArity(ctx);
+        return REDISMODULE_OK;
+    }
+    RedisModuleKey *key = open_key_or_reply(ctx, argv[1], REDISMODULE_READ|REDISMODULE_OPEN_KEY_NOTOUCH);
+    mstime_t lfu;
+    if (RedisModule_StringToLongLong(argv[2], &lfu) != REDISMODULE_OK) {
+        RedisModule_ReplyWithError(ctx, "invalid freq");
+        return REDISMODULE_OK;
+    }
+    int was_set = RedisModule_SetLFU(key, lfu)==REDISMODULE_OK;
+    RedisModule_ReplyWithLongLong(ctx, was_set);
     RedisModule_CloseKey(key);
     return REDISMODULE_OK;
 }
@@ -119,6 +161,10 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
         return REDISMODULE_ERR;
     if (RedisModule_CreateCommand(ctx,"test.getlru", test_getlru,"",0,0,0) == REDISMODULE_ERR)
         return REDISMODULE_ERR;
+    if (RedisModule_CreateCommand(ctx,"test.setlfu", test_setlfu,"",0,0,0) == REDISMODULE_ERR)
+        return REDISMODULE_ERR;
+    if (RedisModule_CreateCommand(ctx,"test.getlfu", test_getlfu,"",0,0,0) == REDISMODULE_ERR)
+        return REDISMODULE_ERR;
 
     return REDISMODULE_OK;
 }

tests/unit/moduleapi/misc.tcl

@@ -26,13 +26,40 @@ start_server {tags {"modules"}} {
     }
 
     test {test modle lru api} {
+        r config set maxmemory-policy allkeys-lru
         r set x foo
         set lru [r test.getlru x]
-        assert { $lru <= 1 }
-        r test.setlru x 100
+        assert { $lru <= 1000 }
+        set was_set [r test.setlru x 100000]
+        assert { $was_set == 1 }
         set idle [r object idletime x]
         assert { $idle >= 100 }
         set lru [r test.getlru x]
-        assert { $lru >= 100 }
+        assert { $lru >= 100000 }
+        r config set maxmemory-policy allkeys-lfu
+        set lru [r test.getlru x]
+        assert { $lru == -1 }
+        set was_set [r test.setlru x 100000]
+        assert { $was_set == 0 }
     }
+    r config set maxmemory-policy allkeys-lru
+
+    test {test modle lfu api} {
+        r config set maxmemory-policy allkeys-lfu
+        r set x foo
+        set lfu [r test.getlfu x]
+        assert { $lfu >= 1 }
+        set was_set [r test.setlfu x 100]
+        assert { $was_set == 1 }
+        set freq [r object freq x]
+        assert { $freq <= 100 }
+        set lfu [r test.getlfu x]
+        assert { $lfu <= 100 }
+        r config set maxmemory-policy allkeys-lru
+        set lfu [r test.getlfu x]
+        assert { $lfu == -1 }
+        set was_set [r test.setlfu x 100]
+        assert { $was_set == 0 }
+    }
+
 }