Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Real LRU eviction algorithm #1459

Open
wants to merge 2 commits into from

5 participants

@fadimko

Patch that implements allkeys-real-lru maxmemory policy.

@antirez
Owner

This is very interesting, thanks! In the past this was never attempted for the fear of the additional memory usage, however there is to evaluate the cost with care and check if there are fields we could remove from the object structure to make room... checking what allocation class of jemalloc to target, and so forth.

Also I would love to write a simulation to check the real advantage of this feature over the imprecise sampling we do. The simulation should just write keys on a Redis instance configured with a given maxmemory setting. We could write the application so that it remembers what are the recently used objects, and verify what is inside the database. Your implementation should score 100% (100% of objects inside the database are the ones with the smallest access time), while the current implementation should not score so well.

I think it is an important step to actually measure what we gain exactly. Great contribution btw! Thanks again.

@1602

I think this is must have feature! Thanks for implementing this, @fadimko.

@leoromanovsky leoromanovsky commented on the diff
src/dictList.h
((9 lines not shown))
+} dictList;
+
+extern dictList *lruList;
+
+dictList* dlCreate ();
+void dlEmpty (dictList *dl);
+void dlFlushDb (dictList *dl, int dbid);
+
+#define dlGetLast(dl) ((dl)->last)
+
+void dlAdd (dictList *dl, struct dictEntry *de);
+void dlDelete (dictList *dl, struct dictEntry *de);
+void dlTouch (dictList *dl, struct dictEntry *de);
+
+#endif /* __DICTLIST_H */
+/* !LRU */

newline :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dustindoiron

Would be fantastic to get this added, :+1:.

@antirez
Owner

Hello, I'm starting a code review about this feature with the intention to merge it and/or work in the direction of fixing the issues if any in order to eventually merge it. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 9, 2013
  1. @fadimko
Commits on Dec 11, 2013
  1. @fadimko

    Fixed some function names.

    fadimko authored
This page is out of date. Refresh to see the latest.
View
2  src/Makefile
@@ -103,7 +103,7 @@ endif
REDIS_SERVER_NAME=redis-server
REDIS_SENTINEL_NAME=redis-sentinel
-REDIS_SERVER_OBJ=adlist.o ae.o anet.o dict.o redis.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o intset.o syncio.o migrate.o endianconv.o slowlog.o scripting.o bio.o rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o
+REDIS_SERVER_OBJ=adlist.o ae.o anet.o dict.o redis.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o intset.o syncio.o migrate.o endianconv.o slowlog.o scripting.o bio.o rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o dictList.o
REDIS_CLI_NAME=redis-cli
REDIS_CLI_OBJ=anet.o sds.o adlist.o redis-cli.o zmalloc.o release.o anet.o ae.o crc64.o
REDIS_BENCHMARK_NAME=redis-benchmark
View
6 src/config.c
@@ -232,6 +232,8 @@ void loadServerConfigFromString(char *config) {
server.maxmemory_policy = REDIS_MAXMEMORY_ALLKEYS_RANDOM;
} else if (!strcasecmp(argv[1],"noeviction")) {
server.maxmemory_policy = REDIS_MAXMEMORY_NO_EVICTION;
+ } else if (!strcasecmp(argv[1],"allkeys-real-lru")) {
+ server.maxmemory_policy = REDIS_MAXMEMORY_ALLKEYS_REAL_LRU;
} else {
err = "Invalid maxmemory policy";
goto loaderr;
@@ -604,6 +606,8 @@ void configSetCommand(redisClient *c) {
server.maxmemory_policy = REDIS_MAXMEMORY_ALLKEYS_RANDOM;
} else if (!strcasecmp(o->ptr,"noeviction")) {
server.maxmemory_policy = REDIS_MAXMEMORY_NO_EVICTION;
+ } else if (!strcasecmp(o->ptr,"allkeys-real-lru")) {
+ server.maxmemory_policy = REDIS_MAXMEMORY_ALLKEYS_REAL_LRU;
} else {
goto badfmt;
}
@@ -992,6 +996,7 @@ void configGetCommand(redisClient *c) {
case REDIS_MAXMEMORY_ALLKEYS_LRU: s = "allkeys-lru"; break;
case REDIS_MAXMEMORY_ALLKEYS_RANDOM: s = "allkeys-random"; break;
case REDIS_MAXMEMORY_NO_EVICTION: s = "noeviction"; break;
+ case REDIS_MAXMEMORY_ALLKEYS_REAL_LRU: s = "allkeys-real-lru"; break;
default: s = "unknown"; break; /* too harmless to panic */
}
addReplyBulkCString(c,"maxmemory-policy");
@@ -1662,6 +1667,7 @@ int rewriteConfig(char *path) {
"allkeys-random", REDIS_MAXMEMORY_ALLKEYS_RANDOM,
"volatile-ttl", REDIS_MAXMEMORY_VOLATILE_TTL,
"noeviction", REDIS_MAXMEMORY_NO_EVICTION,
+ "allkeys-real-lru", REDIS_MAXMEMORY_ALLKEYS_REAL_LRU,
NULL, REDIS_DEFAULT_MAXMEMORY_POLICY);
rewriteConfigNumericalOption(state,"maxmemory-samples",server.maxmemory_samples,REDIS_DEFAULT_MAXMEMORY_SAMPLES);
rewriteConfigAppendonlyOption(state);
View
28 src/db.c
@@ -47,8 +47,12 @@ robj *lookupKey(redisDb *db, robj *key) {
/* Update the access time for the ageing algorithm.
* Don't do it if we have a saving child, as this will trigger
* a copy on write madness. */
- if (server.rdb_child_pid == -1 && server.aof_child_pid == -1)
+ if (server.rdb_child_pid == -1 && server.aof_child_pid == -1) {
val->lru = server.lruclock;
+ /* LRU */
+ dlTouch(lruList, de);
+ /* !LRU */
+ }
return val;
} else {
return NULL;
@@ -90,9 +94,13 @@ robj *lookupKeyWriteOrReply(redisClient *c, robj *key, robj *reply) {
* The program is aborted if the key already exists. */
void dbAdd(redisDb *db, robj *key, robj *val) {
sds copy = sdsdup(key->ptr);
- int retval = dictAdd(db->dict, copy, val);
+ /* LRU */
+ dictEntry *de = dictAddReturnEntry(db->dict, copy, val);
- redisAssertWithInfo(NULL,key,retval == REDIS_OK);
+ redisAssertWithInfo(NULL,key,de != NULL);
+ de->dbid = db->id;
+ dlAdd(lruList, de);
+ /* !LRU */
}
/* Overwrite an existing key with a new value. Incrementing the reference
@@ -105,6 +113,10 @@ void dbOverwrite(redisDb *db, robj *key, robj *val) {
redisAssertWithInfo(NULL,key,de != NULL);
dictReplace(db->dict, key->ptr, val);
+ /* LRU */
+ de->dbid = db->id;
+ dlTouch(lruList, de);
+ /* !LRU */
}
/* High level Set operation. This function can be used in order to set
@@ -159,7 +171,9 @@ int dbDelete(redisDb *db, robj *key) {
/* Deleting an entry from the expires dict will not free the sds of
* the key, because it is shared with the main dictionary. */
if (dictSize(db->expires) > 0) dictDelete(db->expires,key->ptr);
- if (dictDelete(db->dict,key->ptr) == DICT_OK) {
+ /* LRU */
+ if (dictDeleteDictList(db->dict,key->ptr,lruList) == DICT_OK) {
+ /* !LRU */
return 1;
} else {
return 0;
@@ -175,6 +189,9 @@ long long emptyDb() {
dictEmpty(server.db[j].dict);
dictEmpty(server.db[j].expires);
}
+ /* LRU */
+ dlEmpty(lruList);
+ /* !LRU */
return removed;
}
@@ -211,6 +228,9 @@ void flushdbCommand(redisClient *c) {
signalFlushedDb(c->db->id);
dictEmpty(c->db->dict);
dictEmpty(c->db->expires);
+ /* LRU */
+ dlFlushDb(lruList, c->db->id);
+ /* !LRU */
addReply(c,shared.ok);
}
View
25 src/dict.c
@@ -320,6 +320,16 @@ int dictAdd(dict *d, void *key, void *val)
return DICT_OK;
}
+/* Add an element to the target hash table */
+dictEntry* dictAddReturnEntry(dict *d, void *key, void *val)
+{
+ dictEntry *entry = dictAddRaw(d,key);
+
+ if (!entry) return NULL;
+ dictSetVal(d, entry, val);
+ return entry;
+}
+
/* Low level add. This function adds the entry but instead of setting
* a value returns the dictEntry structure to the user, that will make
* sure to fill the value field as he wishes.
@@ -398,7 +408,7 @@ dictEntry *dictReplaceRaw(dict *d, void *key) {
}
/* Search and remove an element */
-static int dictGenericDelete(dict *d, const void *key, int nofree)
+static int dictGenericDelete(dict *d, const void *key, int nofree, dictList *dl)
{
unsigned int h, idx;
dictEntry *he, *prevHe;
@@ -423,6 +433,9 @@ static int dictGenericDelete(dict *d, const void *key, int nofree)
dictFreeKey(d, he);
dictFreeVal(d, he);
}
+ /* LRU */
+ if (dl) dlDelete(dl, he);
+ /* !LRU */
zfree(he);
d->ht[table].used--;
return DICT_OK;
@@ -436,12 +449,18 @@ static int dictGenericDelete(dict *d, const void *key, int nofree)
}
int dictDelete(dict *ht, const void *key) {
- return dictGenericDelete(ht,key,0);
+ return dictGenericDelete(ht,key,0,NULL);
}
int dictDeleteNoFree(dict *ht, const void *key) {
- return dictGenericDelete(ht,key,1);
+ return dictGenericDelete(ht,key,1,NULL);
+}
+
+/* LRU */
+int dictDeleteDictList(dict *ht, const void *key, struct dictList *dl) {
+ return dictGenericDelete(ht,key,0,dl);
}
+/* !LRU */
/* Destroy an entire dictionary */
int _dictClear(dict *d, dictht *ht)
View
9 src/dict.h
@@ -34,6 +34,9 @@
*/
#include <stdint.h>
+/* LRU */
+#include "dictList.h"
+/* !LRU */
#ifndef __DICT_H
#define __DICT_H
@@ -52,6 +55,10 @@ typedef struct dictEntry {
int64_t s64;
} v;
struct dictEntry *next;
+ /* LRU */
+ int dbid;
+ struct dictEntry *lruPrev, *lruNext;
+ /* !LRU */
} dictEntry;
typedef struct dictType {
@@ -143,11 +150,13 @@ typedef void (dictScanFunction)(void *privdata, const dictEntry *de);
dict *dictCreate(dictType *type, void *privDataPtr);
int dictExpand(dict *d, unsigned long size);
int dictAdd(dict *d, void *key, void *val);
+dictEntry* dictAddReturnEntry(dict *d, void *key, void *val);
dictEntry *dictAddRaw(dict *d, void *key);
int dictReplace(dict *d, void *key, void *val);
dictEntry *dictReplaceRaw(dict *d, void *key);
int dictDelete(dict *d, const void *key);
int dictDeleteNoFree(dict *d, const void *key);
+int dictDeleteDictList(dict *ht, const void *key, dictList *dl);
void dictRelease(dict *d);
dictEntry * dictFind(dict *d, const void *key);
void *dictFetchValue(dict *d, const void *key);
View
55 src/dictList.c
@@ -0,0 +1,55 @@
+/* LRU */
+#include "redis.h"
+
+dictList* dlCreate () {
+ dictList *dl = zmalloc (sizeof (dictList));
+
+ dl->first = dl->last = NULL;
+ return dl;
+}
+
+/* This function is called from emptyDb, which removes all data from db,
+ * so I don't need to worry about it here. */
+void dlEmpty (dictList *dl) {
+ dl->first = dl->last = NULL;
+}
+
+void dlFlushDb (dictList *dl, int dbid) {
+ dictEntry *de = dl->last,
+ *deNext;
+
+ while (de != NULL) {
+ deNext = de->lruNext;
+ if (de->dbid == dbid)
+ dlDelete (dl, de);
+ de = deNext;
+ }
+}
+
+void dlAdd (dictList *dl, dictEntry *de) {
+ de->lruNext = NULL;
+ de->lruPrev = dl->first;
+ if (dl->first) {
+ dl->first->lruNext = de;
+ dl->first = de;
+ } else {
+ dl->first = dl->last = de;
+ }
+}
+
+void dlDelete (dictList *dl, dictEntry *de) {
+ if (de->lruNext)
+ de->lruNext->lruPrev = de->lruPrev;
+ else
+ dl->first = de->lruPrev;
+ if (de->lruPrev)
+ de->lruPrev->lruNext = de->lruNext;
+ else
+ dl->last = de->lruNext;
+}
+
+void dlTouch (dictList *dl, dictEntry *de) {
+ dlDelete (dl, de);
+ dlAdd (dl, de);
+}
+/* !LRU */
View
24 src/dictList.h
@@ -0,0 +1,24 @@
+/* LRU */
+#ifndef __DICTLIST_H
+#define __DICTLIST_H
+
+struct dictEntry;
+
+typedef struct dictList {
+ struct dictEntry *first, *last;
+} dictList;
+
+extern dictList *lruList;
+
+dictList* dlCreate ();
+void dlEmpty (dictList *dl);
+void dlFlushDb (dictList *dl, int dbid);
+
+#define dlGetLast(dl) ((dl)->last)
+
+void dlAdd (dictList *dl, struct dictEntry *de);
+void dlDelete (dictList *dl, struct dictEntry *de);
+void dlTouch (dictList *dl, struct dictEntry *de);
+
+#endif /* __DICTLIST_H */
+/* !LRU */

newline :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
View
76 src/redis.c
@@ -66,6 +66,7 @@ double R_Zero, R_PosInf, R_NegInf, R_Nan;
/* Global vars */
struct redisServer server; /* server global state */
struct redisCommand *commandTable;
+dictList *lruList;
/* Our command table.
*
@@ -2594,6 +2595,36 @@ void monitorCommand(redisClient *c) {
/* ============================ Maxmemory directive ======================== */
+long long evict (redisDb *db, sds key, int slaves) {
+ long long delta;
+
+ robj *keyobj = createStringObject(key,sdslen(key));
+ propagateExpire(db,keyobj);
+ /* We compute the amount of memory freed by dbDelete() alone.
+ * It is possible that actually the memory needed to propagate
+ * the DEL in AOF and replication link is greater than the one
+ * we are freeing removing the key, but we can't account for
+ * that otherwise we would never exit the loop.
+ *
+ * AOF and Output buffer memory will be freed eventually so
+ * we only care about memory used by the key space. */
+ delta = (long long) zmalloc_used_memory();
+ dbDelete(db,keyobj);
+ delta -= (long long) zmalloc_used_memory();
+ server.stat_evictedkeys++;
+ notifyKeyspaceEvent(REDIS_NOTIFY_EVICTED, "evicted",
+ keyobj, db->id);
+ decrRefCount(keyobj);
+
+ /* When the memory to free starts to be big enough, we may
+ * start spending so much time here that is impossible to
+ * deliver data to the slaves fast enough, so we force the
+ * transmission here inside the loop. */
+ if (slaves) flushSlavesOutputBuffers();
+
+ return delta;
+}
+
/* This function gets called when 'maxmemory' is set on the config file to limit
* the max memory used by the server, before processing a command.
*
@@ -2647,6 +2678,20 @@ int freeMemoryIfNeeded(void) {
while (mem_freed < mem_tofree) {
int j, k, keys_freed = 0;
+ /* LRU */
+ if (server.maxmemory_policy == REDIS_MAXMEMORY_ALLKEYS_REAL_LRU) {
+ dictEntry *de = dlGetLast(lruList);
+
+ if (de) {
+ mem_freed += evict(server.db + de->dbid, de->key, slaves);
+ keys_freed++;
+ }
+ //TODO:
+ if (!keys_freed) return REDIS_ERR; /* nothing to free... */
+ continue;
+ }
+ /* !LRU */
+
for (j = 0; j < server.dbnum; j++) {
long bestval = 0; /* just to prevent warning */
sds bestkey = NULL;
@@ -2718,33 +2763,8 @@ int freeMemoryIfNeeded(void) {
/* Finally remove the selected key. */
if (bestkey) {
- long long delta;
-
- robj *keyobj = createStringObject(bestkey,sdslen(bestkey));
- propagateExpire(db,keyobj);
- /* We compute the amount of memory freed by dbDelete() alone.
- * It is possible that actually the memory needed to propagate
- * the DEL in AOF and replication link is greater than the one
- * we are freeing removing the key, but we can't account for
- * that otherwise we would never exit the loop.
- *
- * AOF and Output buffer memory will be freed eventually so
- * we only care about memory used by the key space. */
- delta = (long long) zmalloc_used_memory();
- dbDelete(db,keyobj);
- delta -= (long long) zmalloc_used_memory();
- mem_freed += delta;
- server.stat_evictedkeys++;
- notifyKeyspaceEvent(REDIS_NOTIFY_EVICTED, "evicted",
- keyobj, db->id);
- decrRefCount(keyobj);
+ mem_freed += evict(db, bestkey, slaves);
keys_freed++;
-
- /* When the memory to free starts to be big enough, we may
- * start spending so much time here that is impossible to
- * deliver data to the slaves fast enough, so we force the
- * transmission here inside the loop. */
- if (slaves) flushSlavesOutputBuffers();
}
}
if (!keys_freed) return REDIS_ERR; /* nothing to free... */
@@ -2941,6 +2961,10 @@ int main(int argc, char **argv) {
server.sentinel_mode = checkForSentinelMode(argc,argv);
initServerConfig();
+ /* LRU */
+ lruList = dlCreate();
+ /* !LRU */
+
/* We need to init sentinel right now as parsing the configuration file
* in sentinel mode will have the effect of populating the sentinel
* data structures with master nodes to monitor. */
View
7 src/redis.h
@@ -62,6 +62,10 @@
#include "version.h" /* Version macro */
#include "util.h" /* Misc functions useful in many places */
+/* LRU */
+#include "dictList.h"
+/* !LRU */
+
/* Error codes */
#define REDIS_OK 0
#define REDIS_ERR -1
@@ -312,6 +316,9 @@
#define REDIS_MAXMEMORY_ALLKEYS_LRU 3
#define REDIS_MAXMEMORY_ALLKEYS_RANDOM 4
#define REDIS_MAXMEMORY_NO_EVICTION 5
+/* LRU */
+#define REDIS_MAXMEMORY_ALLKEYS_REAL_LRU 6
+/* !LRU */
#define REDIS_DEFAULT_MAXMEMORY_POLICY REDIS_MAXMEMORY_VOLATILE_LRU
/* Scripting */
View
78 tests/unit/lru-test.tcl
@@ -0,0 +1,78 @@
+start_server {tags {"maxmemory"}} {
+ proc __consume_subscribe_messages {client type channels} {
+ set numsub -1
+ set counts {}
+
+ for {set i [llength $channels]} {$i > 0} {incr i -1} {
+ set msg [$client read]
+ assert_equal $type [lindex $msg 0]
+
+ # when receiving subscribe messages the channels names
+ # are ordered. when receiving unsubscribe messages
+ # they are unordered
+ set idx [lsearch -exact $channels [lindex $msg 1]]
+ if {[string match "*unsubscribe" $type]} {
+ assert {$idx >= 0}
+ } else {
+ assert {$idx == 0}
+ }
+ set channels [lreplace $channels $idx $idx]
+
+ # aggregate the subscription count to return to the caller
+ lappend counts [lindex $msg 2]
+ }
+
+ # we should have received messages for channels
+ assert {[llength $channels] == 0}
+ return $counts
+ }
+
+ proc subscribe {client channels} {
+ $client subscribe {*}$channels
+ __consume_subscribe_messages $client subscribe $channels
+ }
+
+ proc unsubscribe {client {channels {}}} {
+ $client unsubscribe {*}$channels
+ __consume_subscribe_messages $client unsubscribe $channels
+ }
+
+ proc psubscribe {client channels} {
+ $client psubscribe {*}$channels
+ __consume_subscribe_messages $client psubscribe $channels
+ }
+
+ proc punsubscribe {client {channels {}}} {
+ $client punsubscribe {*}$channels
+ __consume_subscribe_messages $client punsubscribe $channels
+ }
+
+
+ test "lru test - real lru eviction algorithm" {
+ r config set notify-keyspace-events Ee
+ r config set maxmemory-policy allkeys-real-lru
+ r flushdb
+ set rd1 [redis_deferring_client]
+ assert_equal {1} [psubscribe $rd1 *]
+
+ r config set maxmemory 10000000;
+ for {set name 1} {$name <= 100} {incr name} {
+ r set $name "hello world"
+ }
+ for {set name 1} {$name <= 25} {incr name} {
+ r get $name
+ }
+
+ r config set maxmemory 1
+ for {set name 26} {$name <= 100} {incr name} {
+ set s [$rd1 read]
+ assert {$s == "pmessage * __keyevent@9__:evicted $name"}
+ }
+ for {set name 1} {$name <= 25} {incr name} {
+ set s [$rd1 read]
+ assert {$s == "pmessage * __keyevent@9__:evicted $name"}
+ }
+ r config set maxmemory 0
+ $rd1 close
+ }
+}
View
4 tests/unit/maxmemory.tcl
@@ -1,6 +1,6 @@
start_server {tags {"maxmemory"}} {
foreach policy {
- allkeys-random allkeys-lru volatile-lru volatile-random volatile-ttl
+ allkeys-random allkeys-lru allkeys-real-lru volatile-lru volatile-random volatile-ttl
} {
test "maxmemory - is the memory limit honoured? (policy $policy)" {
# make sure to start with a blank instance
@@ -32,7 +32,7 @@ start_server {tags {"maxmemory"}} {
}
foreach policy {
- allkeys-random allkeys-lru volatile-lru volatile-random volatile-ttl
+ allkeys-random allkeys-lru allkeys-real-lru volatile-lru volatile-random volatile-ttl
} {
test "maxmemory - only allkeys-* should remove non-volatile keys ($policy)" {
# make sure to start with a blank instance
View
13 tests/unit/pubsub.tcl
@@ -356,4 +356,17 @@ start_server {tags {"pubsub"}} {
r config set maxmemory 0
$rd1 close
}
+
+ test "Keyspace notifications: evicted events" {
+ r config set notify-keyspace-events Ee
+ r config set maxmemory-policy allkeys-real-lru
+ r flushdb
+ set rd1 [redis_deferring_client]
+ assert_equal {1} [psubscribe $rd1 *]
+ r set foo bar
+ r config set maxmemory 1
+ assert_equal {pmessage * __keyevent@9__:evicted foo} [$rd1 read]
+ r config set maxmemory 0
+ $rd1 close
+ }
}
Something went wrong with that request. Please try again.