From b7a8daef6070816e08438fd64b2b9a41571eb333 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 5 Jul 2010 19:38:12 +0200 Subject: [PATCH 01/49] WATCH will now consider touched keys target of EXPIRE command after the WATCH is performed, but not before --- src/db.c | 2 ++ tests/unit/cas.tcl | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/db.c b/src/db.c index e1e82cb2..d5e0d1e8 100644 --- a/src/db.c +++ b/src/db.c @@ -472,11 +472,13 @@ void expireGenericCommand(redisClient *c, robj *key, robj *param, long offset) { if (seconds <= 0) { if (dbDelete(c->db,key)) server.dirty++; addReply(c, shared.cone); + touchWatchedKey(c->db,key); return; } else { time_t when = time(NULL)+seconds; if (setExpire(c->db,key,when)) { addReply(c,shared.cone); + touchWatchedKey(c->db,key); server.dirty++; } else { addReply(c,shared.czero); diff --git a/tests/unit/cas.tcl b/tests/unit/cas.tcl index dc6a5ef7..d420d9e2 100644 --- a/tests/unit/cas.tcl +++ b/tests/unit/cas.tcl @@ -111,4 +111,25 @@ start_server {tags {"cas"}} { r ping r exec } {PONG} + + test {WATCH will consider touched keys target of EXPIRE} { + r del x + r set x foo + r watch x + r expire x 10 + r multi + r ping + r exec + } {} + + test {WATCH will not consider touched expired keys} { + r del x + r set x foo + r expire x 2 + r watch x + after 3000 + r multi + r ping + r exec + } {PONG} } From b67d234563e03ceb9325b39a78cca11bec28569d Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 5 Jul 2010 20:06:54 +0200 Subject: [PATCH 02/49] Fixed a crash loading the AOF file containing MULTI/EXEC, a result of WATCH implementation. Test needed... --- src/aof.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/aof.c b/src/aof.c index c92798c5..f8b92d2d 100644 --- a/src/aof.c +++ b/src/aof.c @@ -194,6 +194,7 @@ struct redisClient *createFakeClient(void) { * so that Redis will not try to send replies to this client. */ c->replstate = REDIS_REPL_WAIT_BGSAVE_START; c->reply = listCreate(); + c->watched_keys = listCreate(); listSetFreeMethod(c->reply,decrRefCount); listSetDupMethod(c->reply,dupClientReplyValue); initClientMultiState(c); @@ -203,6 +204,7 @@ struct redisClient *createFakeClient(void) { void freeFakeClient(struct redisClient *c) { sdsfree(c->querybuf); listRelease(c->reply); + listRelease(c->watched_keys); freeClientMultiState(c); zfree(c); } From d06a5b23c85d615310ce199c04bc2d122d16526a Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 5 Jul 2010 20:14:48 +0200 Subject: [PATCH 03/49] Fixed compilation on *BSD systems --- TODO | 1 + src/redis.h | 1 + 2 files changed, 2 insertions(+) diff --git a/TODO b/TODO index 4fcdb18f..5d9736a9 100644 --- a/TODO +++ b/TODO @@ -6,6 +6,7 @@ VERSION 2.2 TODO (Optimizations and latency) * Support for syslog(3). * Change the implementation of ZCOUNT to use the augmented skiplist in order to be much faster. +* Add an explicit test for MULTI/EXEC reloaded in the AOF. VM TODO ======= diff --git a/src/redis.h b/src/redis.h index 545a3e0f..4ff83620 100644 --- a/src/redis.h +++ b/src/redis.h @@ -16,6 +16,7 @@ #include #include #include +#include #include "ae.h" /* Event driven programming library */ #include "sds.h" /* Dynamic safe strings */ From 70a214c46d00f37b83a4cb944e09a754072a28c9 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 5 Jul 2010 20:37:20 +0200 Subject: [PATCH 04/49] INSTALL file added BETATESTING.txt removed --- BETATESTING.txt | 9 --------- INSTALL | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 9 deletions(-) delete mode 100644 BETATESTING.txt create mode 100644 INSTALL diff --git a/BETATESTING.txt b/BETATESTING.txt deleted file mode 100644 index 10fc9413..00000000 --- a/BETATESTING.txt +++ /dev/null @@ -1,9 +0,0 @@ -This is a stable release, for beta testing make sure to download the latest source code from Git: - - git clone git://github.com/antirez/redis.git - -It's also possibe to download the latest source code as a tarball: - - http://github.com/antirez/redis/tree/master - -(use the download button) diff --git a/INSTALL b/INSTALL new file mode 100644 index 00000000..7c6635aa --- /dev/null +++ b/INSTALL @@ -0,0 +1,19 @@ +To compile Redis, do the following: + + cd src; make + +The compilation will produce a redis-server binary. +Copy this file where you want. + +Run the server using the following command line: + + /path/to/redis-server + +This will start a Redis server with the default configuration. + +Otherwise if you want to provide your configuration use: + + /path/to/redis-server /path/to/redis.conf + +You can find an example redis.conf file in the root directory +of this source distribution. From d0a4e24e321ced2324b0ad4b6be34f13f90e9b90 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 5 Jul 2010 15:16:33 -0400 Subject: [PATCH 05/49] merged code from 184d74ab, 4774a53b, f483ce5f to new file structure --- src/t_list.c | 9 ++++----- src/t_zset.c | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/t_list.c b/src/t_list.c index ec8b30c3..0568fff1 100644 --- a/src/t_list.c +++ b/src/t_list.c @@ -476,11 +476,10 @@ void lrangeCommand(redisClient *c) { if (start < 0) start = llen+start; if (end < 0) end = llen+end; if (start < 0) start = 0; - if (end < 0) end = 0; - /* indexes sanity checks */ + /* Invariant: start >= 0, so this test will be true when end < 0. + * The range is empty when start > end or start >= length. */ if (start > end || start >= llen) { - /* Out of range start or start > end result in empty list */ addReply(c,shared.emptymultibulk); return; } @@ -516,9 +515,9 @@ void ltrimCommand(redisClient *c) { if (start < 0) start = llen+start; if (end < 0) end = llen+end; if (start < 0) start = 0; - if (end < 0) end = 0; - /* indexes sanity checks */ + /* Invariant: start >= 0, so this test will be true when end < 0. + * The range is empty when start > end or start >= length. */ if (start > end || start >= llen) { /* Out of range start or start > end result in empty list */ ltrim = llen; diff --git a/src/t_zset.c b/src/t_zset.c index de32a8ee..26a80e99 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -497,9 +497,9 @@ void zremrangebyrankCommand(redisClient *c) { if (start < 0) start = llen+start; if (end < 0) end = llen+end; if (start < 0) start = 0; - if (end < 0) end = 0; - /* indexes sanity checks */ + /* Invariant: start >= 0, so this test will be true when end < 0. + * The range is empty when start > end or start >= length. */ if (start > end || start >= llen) { addReply(c,shared.czero); return; @@ -750,11 +750,10 @@ void zrangeGenericCommand(redisClient *c, int reverse) { if (start < 0) start = llen+start; if (end < 0) end = llen+end; if (start < 0) start = 0; - if (end < 0) end = 0; - /* indexes sanity checks */ + /* Invariant: start >= 0, so this test will be true when end < 0. + * The range is empty when start > end or start >= length. */ if (start > end || start >= llen) { - /* Out of range start or start > end result in empty list */ addReply(c,shared.emptymultibulk); return; } From 8b654e54c490161983e42a328675e484e404e2d2 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 6 Jul 2010 17:24:00 +0200 Subject: [PATCH 06/49] First implementation of a replication consistency test --- tests/integration/replication.tcl | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/integration/replication.tcl b/tests/integration/replication.tcl index 0f5d496d..39d77c8f 100644 --- a/tests/integration/replication.tcl +++ b/tests/integration/replication.tcl @@ -1,3 +1,18 @@ +start_server {tags {"repl"}} { + start_server {} { + test {First server should have role slave after SLAVEOF} { + r -1 slaveof [srv 0 host] [srv 0 port] + after 1000 + s -1 role + } {slave} + + test {MASTER and SLAVE dataset should be identical after complex ops} { + createComplexDataset r 10000 + assert_equal [r debug digest] [r -1 debug digest] + } + } +} + start_server {tags {"repl"}} { r set mykey foo From b056ca39f2d37957164c5bfc04c466b64aa6087e Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 6 Jul 2010 18:30:38 +0200 Subject: [PATCH 07/49] improved random dataset creation in test: del, sunionstore, zunionstore --- tests/support/util.tcl | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/support/util.tcl b/tests/support/util.tcl index 36bc90d9..ae758f4c 100644 --- a/tests/support/util.tcl +++ b/tests/support/util.tcl @@ -127,9 +127,23 @@ proc randomKey {} { } } +proc findKeyWithType {r type} { + for {set j 0} {$j < 20} {incr j} { + set k [$r randomkey] + if {$k eq {}} { + return {} + } + if {[$r type $k] eq $type} { + return $k + } + } + return {} +} + proc createComplexDataset {r ops} { for {set j 0} {$j < $ops} {incr j} { set k [randomKey] + set k2 [randomKey] set f [randomValue] set v [randomValue] randpath { @@ -158,6 +172,8 @@ proc createComplexDataset {r ops} { $r zadd $k $d $v } { $r hset $k $f $v + } { + $r del $k } set t [$r type $k] } @@ -175,11 +191,23 @@ proc createComplexDataset {r ops} { } {set} { randpath {$r sadd $k $v} \ - {$r srem $k $v} + {$r srem $k $v} \ + { + set otherset [findKeyWithType r set] + if {$otherset ne {}} { + $r sunionstore $k2 $k $otherset + } + } } {zset} { randpath {$r zadd $k $d $v} \ - {$r zrem $k $v} + {$r zrem $k $v} \ + { + set otherzset [findKeyWithType r zset] + if {$otherzset ne {}} { + $r zunionstore $k2 2 $k $otherzset + } + } } {hash} { randpath {$r hset $k $f $v} \ From f26dde8ca93e50068084cc2f5423b9244296148d Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 6 Jul 2010 18:54:54 +0200 Subject: [PATCH 08/49] top level Makefile added, so you do not need to cd src --- Makefile | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..711ef6ff --- /dev/null +++ b/Makefile @@ -0,0 +1,9 @@ +# Top level makefile, the real shit is at src/Makefile + +TARGETS=32bit noopt + +all: + cd src && $(MAKE) $@ + +$(TARGETS) clean: + cd src && $(MAKE) $@ From acc018549379dc2ac02a0b0484cfdb4d56122488 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 6 Jul 2010 19:07:16 +0200 Subject: [PATCH 09/49] make install target, finally ;) --- Makefile | 2 +- src/Makefile | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 711ef6ff..576c0e5e 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # Top level makefile, the real shit is at src/Makefile -TARGETS=32bit noopt +TARGETS=32bit noopt test install all: cd src && $(MAKE) $@ diff --git a/src/Makefile b/src/Makefile index a7cc6b9a..0af70f17 100644 --- a/src/Makefile +++ b/src/Makefile @@ -15,6 +15,10 @@ endif CCOPT= $(CFLAGS) $(CCLINK) $(ARCH) $(PROF) DEBUG?= -g -rdynamic -ggdb +INSTALL_TOP= /usr/local +INSTALL_BIN= $(INSTALL_TOP)/bin +INSTALL= cp -p + 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 vm.o pubsub.o multi.o debug.o sort.o BENCHOBJ = ae.o anet.o redis-benchmark.o sds.o adlist.o zmalloc.o CLIOBJ = anet.o sds.o adlist.o redis-cli.o zmalloc.o linenoise.o @@ -109,3 +113,10 @@ noopt: 32bitgprof: make PROF="-pg" ARCH="-arch i386" + +install: all + $(INSTALL) $(PRGNAME) $(INSTALL_BIN) + $(INSTALL) $(BENCHPRGNAME) $(INSTALL_BIN) + $(INSTALL) $(CLIPRGNAME) $(INSTALL_BIN) + $(INSTALL) $(CHECKDUMPPRGNAME) $(INSTALL_BIN) + $(INSTALL) $(CHECKAOFPRGNAME) $(INSTALL_BIN) From 443d1e9efedfcab7d67c10085b69a9b4cb1db6f5 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 6 Jul 2010 19:10:20 +0200 Subject: [PATCH 10/49] Make install fixed using a dummy taget --- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 576c0e5e..f6790945 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,14 @@ # Top level makefile, the real shit is at src/Makefile -TARGETS=32bit noopt test install +TARGETS=32bit noopt test all: cd src && $(MAKE) $@ +install: dummy + cd src && $(MAKE) $@ + $(TARGETS) clean: cd src && $(MAKE) $@ + +dummy: From 185cabda4524efc03c767243fb90083708a4aeeb Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 6 Jul 2010 19:17:09 +0200 Subject: [PATCH 11/49] redis-cli is now able to report version information using -v --- src/redis-cli.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index 2daa7c46..1d00fb87 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -29,6 +29,7 @@ */ #include "fmacros.h" +#include "version.h" #include #include @@ -315,6 +316,9 @@ static int parseOptions(int argc, char **argv) { config.interactive = 1; } else if (!strcmp(argv[i],"-c")) { config.argn_from_stdin = 1; + } else if (!strcmp(argv[i],"-v")) { + printf("redis-cli shipped with Redis verison %s\n", REDIS_VERSION); + exit(0); } else { break; } @@ -340,7 +344,7 @@ static sds readArgFromStdin(void) { } static void usage() { - fprintf(stderr, "usage: redis-cli [-h host] [-p port] [-a authpw] [-r repeat_times] [-n db_num] [-i] cmd arg1 arg2 arg3 ... argN\n"); + fprintf(stderr, "usage: redis-cli [-iv] [-h host] [-p port] [-a authpw] [-r repeat_times] [-n db_num] cmd arg1 arg2 arg3 ... argN\n"); fprintf(stderr, "usage: echo \"argN\" | redis-cli -c [-h host] [-p port] [-a authpw] [-r repeat_times] [-n db_num] cmd arg1 arg2 ... arg(N-1)\n"); fprintf(stderr, "\nIf a pipe from standard input is detected this data is used as last argument.\n\n"); fprintf(stderr, "example: cat /etc/passwd | redis-cli set my_passwd\n"); From 99628c1af8ab2a2af4f2baf7039c08acdf0974ce Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 7 Jul 2010 18:44:53 +0200 Subject: [PATCH 12/49] redis-cli history saved across sessions --- src/linenoise.c | 48 ++++++++++++++++++++++++++++++++++++++++++------ src/linenoise.h | 4 +++- src/redis-cli.c | 9 +++++++++ 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/linenoise.c b/src/linenoise.c index 0c04d03f..b7c6b732 100644 --- a/src/linenoise.c +++ b/src/linenoise.c @@ -69,7 +69,6 @@ * */ -#include "fmacros.h" #include #include #include @@ -81,13 +80,14 @@ #include #include +#define LINENOISE_DEFAULT_HISTORY_MAX_LEN 100 #define LINENOISE_MAX_LINE 4096 static char *unsupported_term[] = {"dumb","cons25",NULL}; static struct termios orig_termios; /* in order to restore at exit */ static int rawmode = 0; /* for atexit() function to check if restore is needed*/ static int atexit_registered = 0; /* register atexit just 1 time */ -static int history_max_len = 100; +static int history_max_len = LINENOISE_DEFAULT_HISTORY_MAX_LEN; static int history_len = 0; char **history = NULL; @@ -219,11 +219,10 @@ static int linenoisePrompt(int fd, char *buf, size_t buflen, const char *prompt) if (nread <= 0) return len; switch(c) { case 13: /* enter */ - history_len--; - return len; case 4: /* ctrl-d */ history_len--; - return (len == 0) ? -1 : (int)len; + free(history[history_len]); + return (len == 0 && c == 4) ? -1 : (int)len; case 3: /* ctrl-c */ errno = EAGAIN; return -1; @@ -396,7 +395,7 @@ int linenoiseHistoryAdd(const char *line) { char *linecopy; if (history_max_len == 0) return 0; - if (history == 0) { + if (history == NULL) { history = malloc(sizeof(char*)*history_max_len); if (history == NULL) return 0; memset(history,0,(sizeof(char*)*history_max_len)); @@ -404,6 +403,7 @@ int linenoiseHistoryAdd(const char *line) { linecopy = strdup(line); if (!linecopy) return 0; if (history_len == history_max_len) { + free(history[0]); memmove(history,history+1,sizeof(char*)*(history_max_len-1)); history_len--; } @@ -431,3 +431,39 @@ int linenoiseHistorySetMaxLen(int len) { history_len = history_max_len; return 1; } + +/* Save the history in the specified file. On success 0 is returned + * otherwise -1 is returned. */ +int linenoiseHistorySave(char *filename) { + FILE *fp = fopen(filename,"w"); + int j; + + if (fp == NULL) return -1; + for (j = 0; j < history_len; j++) + fprintf(fp,"%s\n",history[j]); + fclose(fp); + return 0; +} + +/* Load the history from the specified file. If the file does not exist + * zero is returned and no operation is performed. + * + * If the file exists and the operation succeeded 0 is returned, otherwise + * on error -1 is returned. */ +int linenoiseHistoryLoad(char *filename) { + FILE *fp = fopen(filename,"r"); + char buf[LINENOISE_MAX_LINE]; + + if (fp == NULL) return -1; + + while (fgets(buf,LINENOISE_MAX_LINE,fp) != NULL) { + char *p; + + p = strchr(buf,'\r'); + if (!p) p = strchr(buf,'\n'); + if (p) *p = '\0'; + linenoiseHistoryAdd(buf); + } + fclose(fp); + return 0; +} diff --git a/src/linenoise.h b/src/linenoise.h index ff45e2c4..0d76aea9 100644 --- a/src/linenoise.h +++ b/src/linenoise.h @@ -35,7 +35,9 @@ #define __LINENOISE_H char *linenoise(const char *prompt); -int linenoiseHistoryAdd(char *line); +int linenoiseHistoryAdd(const char *line); int linenoiseHistorySetMaxLen(int len); +int linenoiseHistorySave(char *filename); +int linenoiseHistoryLoad(char *filename); #endif /* __LINENOISE_H */ diff --git a/src/redis-cli.c b/src/redis-cli.c index 1d00fb87..dac82862 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -61,6 +61,7 @@ static struct config { int pubsub_mode; int raw_output; char *auth; + char *historyfile; } config; static int cliReadReply(int fd); @@ -439,6 +440,7 @@ static void repl() { if (line[0] != '\0') { argv = splitArguments(line,&argc); linenoiseHistoryAdd(line); + if (config.historyfile) linenoiseHistorySave(config.historyfile); if (argc > 0) { if (strcasecmp(argv[0],"quit") == 0 || strcasecmp(argv[0],"exit") == 0) @@ -472,6 +474,13 @@ int main(int argc, char **argv) { config.pubsub_mode = 0; config.raw_output = 0; config.auth = NULL; + config.historyfile = NULL; + + if (getenv("HOME") != NULL) { + config.historyfile = malloc(256); + snprintf(config.historyfile,256,"%s/.rediscli_history",getenv("HOME")); + linenoiseHistoryLoad(config.historyfile); + } firstarg = parseOptions(argc,argv); argc -= firstarg; From e51a74aa40136387615eec055975c63c803f5b30 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 9 Jul 2010 10:51:41 +0200 Subject: [PATCH 13/49] fmacro included in linenoise.c --- src/linenoise.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/linenoise.c b/src/linenoise.c index b7c6b732..54f5a27d 100644 --- a/src/linenoise.c +++ b/src/linenoise.c @@ -69,6 +69,8 @@ * */ +#include "fmacros.h" + #include #include #include From 5b4bff9c175a0189672d95eb953df5704c891d0c Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 12 Jul 2010 12:01:15 +0200 Subject: [PATCH 14/49] WATCH is now affected only when write commands actually modify the key content --- TODO | 2 ++ src/db.c | 4 +--- src/sort.c | 1 + src/t_hash.c | 5 +++++ src/t_list.c | 10 ++++++++++ src/t_set.c | 11 ++++++++++- src/t_string.c | 6 +++++- src/t_zset.c | 6 ++++++ 8 files changed, 40 insertions(+), 5 deletions(-) diff --git a/TODO b/TODO index 5d9736a9..dbde9bb5 100644 --- a/TODO +++ b/TODO @@ -4,7 +4,9 @@ Redis TODO and Roadmap VERSION 2.2 TODO (Optimizations and latency) ============================================ +* SETNX when failing should not affect WATCH * Support for syslog(3). +* Document ZCOUNT. * Change the implementation of ZCOUNT to use the augmented skiplist in order to be much faster. * Add an explicit test for MULTI/EXEC reloaded in the AOF. diff --git a/src/db.c b/src/db.c index d5e0d1e8..958a9f6b 100644 --- a/src/db.c +++ b/src/db.c @@ -46,7 +46,6 @@ robj *lookupKeyRead(redisDb *db, robj *key) { robj *lookupKeyWrite(redisDb *db, robj *key) { deleteIfVolatile(db,key); - touchWatchedKey(db,key); return lookupKey(db,key); } @@ -332,6 +331,7 @@ void renameGenericCommand(redisClient *c, int nx) { dbReplace(c->db,c->argv[2],o); } dbDelete(c->db,c->argv[1]); + touchWatchedKey(c->db,c->argv[1]); touchWatchedKey(c->db,c->argv[2]); server.dirty++; addReply(c,nx ? shared.cone : shared.ok); @@ -506,5 +506,3 @@ void ttlCommand(redisClient *c) { } addReplySds(c,sdscatprintf(sdsempty(),":%d\r\n",ttl)); } - - diff --git a/src/sort.c b/src/sort.c index 0bc86b47..4295a6ec 100644 --- a/src/sort.c +++ b/src/sort.c @@ -364,6 +364,7 @@ void sortCommand(redisClient *c) { * SORT result is empty a new key is set and maybe the old content * replaced. */ server.dirty += 1+outputlen; + touchWatchedKey(c->db,storekey); addReplySds(c,sdscatprintf(sdsempty(),":%d\r\n",outputlen)); } diff --git a/src/t_hash.c b/src/t_hash.c index 3f5fd6e1..b6be284f 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -224,6 +224,7 @@ void hsetCommand(redisClient *c) { hashTypeTryObjectEncoding(o,&c->argv[2], &c->argv[3]); update = hashTypeSet(o,c->argv[2],c->argv[3]); addReply(c, update ? shared.czero : shared.cone); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } @@ -238,6 +239,7 @@ void hsetnxCommand(redisClient *c) { hashTypeTryObjectEncoding(o,&c->argv[2], &c->argv[3]); hashTypeSet(o,c->argv[2],c->argv[3]); addReply(c, shared.cone); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } } @@ -258,6 +260,7 @@ void hmsetCommand(redisClient *c) { hashTypeSet(o,c->argv[i],c->argv[i+1]); } addReply(c, shared.ok); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } @@ -284,6 +287,7 @@ void hincrbyCommand(redisClient *c) { hashTypeSet(o,c->argv[2],new); decrRefCount(new); addReplyLongLong(c,value); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } @@ -330,6 +334,7 @@ void hdelCommand(redisClient *c) { if (hashTypeDelete(o,c->argv[2])) { if (hashTypeLength(o) == 0) dbDelete(c->db,c->argv[1]); addReply(c,shared.cone); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } else { addReply(c,shared.czero); diff --git a/src/t_list.c b/src/t_list.c index 0568fff1..2a981033 100644 --- a/src/t_list.c +++ b/src/t_list.c @@ -273,12 +273,14 @@ void pushGenericCommand(redisClient *c, int where) { return; } if (handleClientsWaitingListPush(c,c->argv[1],c->argv[2])) { + touchWatchedKey(c->db,c->argv[1]); addReply(c,shared.cone); return; } } listTypePush(lobj,c->argv[2],where); addReplyLongLong(c,listTypeLength(lobj)); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } @@ -327,6 +329,7 @@ void pushxGenericCommand(redisClient *c, robj *refval, robj *val, int where) { if (subject->encoding == REDIS_ENCODING_ZIPLIST && ziplistLen(subject->ptr) > server.list_max_ziplist_entries) listTypeConvert(subject,REDIS_ENCODING_LINKEDLIST); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } else { /* Notify client of a failed insert */ @@ -335,6 +338,7 @@ void pushxGenericCommand(redisClient *c, robj *refval, robj *val, int where) { } } else { listTypePush(subject,val,where); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } @@ -419,6 +423,7 @@ void lsetCommand(redisClient *c) { o->ptr = ziplistInsert(o->ptr,p,value->ptr,sdslen(value->ptr)); decrRefCount(value); addReply(c,shared.ok); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } } else if (o->encoding == REDIS_ENCODING_LINKEDLIST) { @@ -430,6 +435,7 @@ void lsetCommand(redisClient *c) { listNodeValue(ln) = value; incrRefCount(value); addReply(c,shared.ok); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } } else { @@ -448,6 +454,7 @@ void popGenericCommand(redisClient *c, int where) { addReplyBulk(c,value); decrRefCount(value); if (listTypeLength(o) == 0) dbDelete(c->db,c->argv[1]); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } } @@ -546,6 +553,7 @@ void ltrimCommand(redisClient *c) { redisPanic("Unknown list encoding"); } if (listTypeLength(o) == 0) dbDelete(c->db,c->argv[1]); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; addReply(c,shared.ok); } @@ -587,6 +595,7 @@ void lremCommand(redisClient *c) { if (listTypeLength(subject) == 0) dbDelete(c->db,c->argv[1]); addReplySds(c,sdscatprintf(sdsempty(),":%d\r\n",removed)); + if (removed) touchWatchedKey(c->db,c->argv[1]); } /* This is the semantic of this command: @@ -635,6 +644,7 @@ void rpoplpushcommand(redisClient *c) { /* Delete the source list when it is empty */ if (listTypeLength(sobj) == 0) dbDelete(c->db,c->argv[1]); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } } diff --git a/src/t_set.c b/src/t_set.c index 808ef268..94b97633 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -19,6 +19,7 @@ void saddCommand(redisClient *c) { } if (dictAdd(set->ptr,c->argv[2],NULL) == DICT_OK) { incrRefCount(c->argv[2]); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; addReply(c,shared.cone); } else { @@ -34,6 +35,7 @@ void sremCommand(redisClient *c) { if (dictDelete(set->ptr,c->argv[2]) == DICT_OK) { server.dirty++; + touchWatchedKey(c->db,c->argv[1]); if (htNeedsResize(set->ptr)) dictResize(set->ptr); if (dictSize((dict*)set->ptr) == 0) dbDelete(c->db,c->argv[1]); addReply(c,shared.cone); @@ -67,6 +69,8 @@ void smoveCommand(redisClient *c) { } if (dictSize((dict*)srcset->ptr) == 0 && srcset != dstset) dbDelete(c->db,c->argv[1]); + touchWatchedKey(c->db,c->argv[1]); + touchWatchedKey(c->db,c->argv[2]); server.dirty++; /* Add the element to the destination set */ if (!dstset) { @@ -118,6 +122,7 @@ void spopCommand(redisClient *c) { dictDelete(set->ptr,ele); if (htNeedsResize(set->ptr)) dictResize(set->ptr); if (dictSize((dict*)set->ptr) == 0) dbDelete(c->db,c->argv[1]); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } } @@ -161,8 +166,10 @@ void sinterGenericCommand(redisClient *c, robj **setskeys, unsigned long setsnum if (!setobj) { zfree(dv); if (dstkey) { - if (dbDelete(c->db,dstkey)) + if (dbDelete(c->db,dstkey)) { + touchWatchedKey(c->db,dstkey); server.dirty++; + } addReply(c,shared.czero); } else { addReply(c,shared.emptymultibulk); @@ -229,6 +236,7 @@ void sinterGenericCommand(redisClient *c, robj **setskeys, unsigned long setsnum decrRefCount(dstset); addReply(c,shared.czero); } + touchWatchedKey(c->db,dstkey); server.dirty++; } else { lenobj->ptr = sdscatprintf(sdsempty(),"*%lu\r\n",cardinality); @@ -327,6 +335,7 @@ void sunionDiffGenericCommand(redisClient *c, robj **setskeys, int setsnum, robj decrRefCount(dstset); addReply(c,shared.czero); } + touchWatchedKey(c->db,dstkey); server.dirty++; } zfree(dv); diff --git a/src/t_string.c b/src/t_string.c index eaaec05b..281bd6be 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -17,7 +17,6 @@ void setGenericCommand(redisClient *c, int nx, robj *key, robj *val, robj *expir } } - touchWatchedKey(c->db,key); if (nx) deleteIfVolatile(c->db,key); retval = dbAdd(c->db,key,val); if (retval == REDIS_ERR) { @@ -31,6 +30,7 @@ void setGenericCommand(redisClient *c, int nx, robj *key, robj *val, robj *expir } else { incrRefCount(val); } + touchWatchedKey(c->db,key); server.dirty++; removeExpire(c->db,key); if (expire) setExpire(c->db,key,time(NULL)+seconds); @@ -72,6 +72,7 @@ void getsetCommand(redisClient *c) { if (getGenericCommand(c) == REDIS_ERR) return; dbReplace(c->db,c->argv[1],c->argv[2]); incrRefCount(c->argv[2]); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; removeExpire(c->db,c->argv[1]); } @@ -120,6 +121,7 @@ void msetGenericCommand(redisClient *c, int nx) { dbReplace(c->db,c->argv[j],c->argv[j+1]); incrRefCount(c->argv[j+1]); removeExpire(c->db,c->argv[j]); + touchWatchedKey(c->db,c->argv[j]); } server.dirty += (c->argc-1)/2; addReply(c, nx ? shared.cone : shared.ok); @@ -144,6 +146,7 @@ void incrDecrCommand(redisClient *c, long long incr) { value += incr; o = createStringObjectFromLongLong(value); dbReplace(c->db,c->argv[1],o); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; addReply(c,shared.colon); addReply(c,o); @@ -207,6 +210,7 @@ void appendCommand(redisClient *c) { } totlen = sdslen(o->ptr); } + touchWatchedKey(c->db,c->argv[1]); server.dirty++; addReplySds(c,sdscatprintf(sdsempty(),":%lu\r\n",(unsigned long)totlen)); } diff --git a/src/t_zset.c b/src/t_zset.c index 26a80e99..0fcd6ea3 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -379,6 +379,7 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i incrRefCount(ele); /* added to hash */ zslInsert(zs->zsl,*score,ele); incrRefCount(ele); /* added to skiplist */ + touchWatchedKey(c->db,c->argv[1]); server.dirty++; if (doincrement) addReplyDouble(c,*score); @@ -402,6 +403,7 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i incrRefCount(ele); /* Update the score in the hash table */ dictReplace(zs->dict,ele,score); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; } else { zfree(score); @@ -452,6 +454,7 @@ void zremCommand(redisClient *c) { dictDelete(zs->dict,c->argv[2]); if (htNeedsResize(zs->dict)) dictResize(zs->dict); if (dictSize(zs->dict) == 0) dbDelete(c->db,c->argv[1]); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; addReply(c,shared.cone); } @@ -473,6 +476,7 @@ void zremrangebyscoreCommand(redisClient *c) { deleted = zslDeleteRangeByScore(zs->zsl,min,max,zs->dict); if (htNeedsResize(zs->dict)) dictResize(zs->dict); if (dictSize(zs->dict) == 0) dbDelete(c->db,c->argv[1]); + if (deleted) touchWatchedKey(c->db,c->argv[1]); server.dirty += deleted; addReplyLongLong(c,deleted); } @@ -511,6 +515,7 @@ void zremrangebyrankCommand(redisClient *c) { deleted = zslDeleteRangeByRank(zs->zsl,start+1,end+1,zs->dict); if (htNeedsResize(zs->dict)) dictResize(zs->dict); if (dictSize(zs->dict) == 0) dbDelete(c->db,c->argv[1]); + if (deleted) touchWatchedKey(c->db,c->argv[1]); server.dirty += deleted; addReplyLongLong(c, deleted); } @@ -702,6 +707,7 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { if (dstzset->zsl->length) { dbAdd(c->db,dstkey,dstobj); addReplyLongLong(c, dstzset->zsl->length); + touchWatchedKey(c->db,dstkey); server.dirty++; } else { decrRefCount(dstobj); From 2cffe2993b01600ab16e7e424b71db4f6327941c Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 16 Jul 2010 23:56:18 +0200 Subject: [PATCH 15/49] TODO list modified, trivial change to source code --- TODO | 9 +++++++-- src/object.c | 3 +-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/TODO b/TODO index dbde9bb5..830149b3 100644 --- a/TODO +++ b/TODO @@ -4,11 +4,10 @@ Redis TODO and Roadmap VERSION 2.2 TODO (Optimizations and latency) ============================================ -* SETNX when failing should not affect WATCH * Support for syslog(3). -* Document ZCOUNT. * Change the implementation of ZCOUNT to use the augmented skiplist in order to be much faster. * Add an explicit test for MULTI/EXEC reloaded in the AOF. +* Command table -> hash table, with support for command renaming VM TODO ======= @@ -59,3 +58,9 @@ KNOWN BUGS ========== * LRANGE and other commands are using 32 bit integers for ranges, and overflows are not detected. So LRANGE mylist 0 23498204823094823904823904 will have random effects. + +REDIS CLI TODO +============== + +* Computer parsable output generation +* Memoize return values so that they can be used later as arguments, like $1 diff --git a/src/object.c b/src/object.c index 4854909e..772637ce 100644 --- a/src/object.c +++ b/src/object.c @@ -11,8 +11,7 @@ robj *createObject(int type, void *ptr) { listDelNode(server.objfreelist,head); if (server.vm_enabled) pthread_mutex_unlock(&server.obj_freelist_mutex); } else { - if (server.vm_enabled) - pthread_mutex_unlock(&server.obj_freelist_mutex); + if (server.vm_enabled) pthread_mutex_unlock(&server.obj_freelist_mutex); o = zmalloc(sizeof(*o)); } o->type = type; From 1a71fb96697fc2af064fc221b21421ad6a8196fa Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 21 Jul 2010 13:16:26 +0200 Subject: [PATCH 16/49] vm_blocked_clients count fixed in INFO, thanks to Pietern Noordhuis --- src/networking.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/networking.c b/src/networking.c index 31844a09..e5a66984 100644 --- a/src/networking.c +++ b/src/networking.c @@ -235,19 +235,24 @@ void freeClient(redisClient *c) { ln = listSearchKey(server.clients,c); redisAssert(ln != NULL); listDelNode(server.clients,ln); - /* Remove from the list of clients that are now ready to be restarted - * after waiting for swapped keys */ - if (c->flags & REDIS_IO_WAIT && listLength(c->io_keys) == 0) { - ln = listSearchKey(server.io_ready_clients,c); - if (ln) { + /* Remove from the list of clients waiting for swapped keys, or ready + * to be restarted, but not yet woken up again. */ + if (c->flags & REDIS_IO_WAIT) { + redisAssert(server.vm_enabled); + if (listLength(c->io_keys) == 0) { + ln = listSearchKey(server.io_ready_clients,c); + + /* When this client is waiting to be woken up (REDIS_IO_WAIT), + * it should be present in the list io_ready_clients */ + redisAssert(ln != NULL); listDelNode(server.io_ready_clients,ln); - server.vm_blocked_clients--; + } else { + while (listLength(c->io_keys)) { + ln = listFirst(c->io_keys); + dontWaitForSwappedKey(c,ln->value); + } } - } - /* Remove from the list of clients waiting for swapped keys */ - while (server.vm_enabled && listLength(c->io_keys)) { - ln = listFirst(c->io_keys); - dontWaitForSwappedKey(c,ln->value); + server.vm_blocked_clients--; } listRelease(c->io_keys); /* Master/slave cleanup */ From 0e5441d8161042493fcfdacaeed2a770f1f7285c Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 22 Jul 2010 13:08:02 +0200 Subject: [PATCH 17/49] don't use object sharing inside I/O threads, as a fix for a well known instability of VM introduced with the new object sharing code --- src/object.c | 11 +++++++++-- src/redis.c | 1 + src/redis.h | 1 + 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/object.c b/src/object.c index 772637ce..25567e2a 100644 --- a/src/object.c +++ b/src/object.c @@ -213,8 +213,15 @@ robj *tryObjectEncoding(robj *o) { /* Check if we can represent this string as a long integer */ if (isStringRepresentableAsLong(s,&value) == REDIS_ERR) return o; - /* Ok, this object can be encoded */ - if (value >= 0 && value < REDIS_SHARED_INTEGERS) { + /* Ok, this object can be encoded... + * + * Can I use a shared object? Only if the object is inside a given + * range and if this is the main thread, sinc when VM is enabled we + * have the constraint that I/O thread should only handle non-shared + * objects, in order to avoid race conditions (we don't have per-object + * locking). */ + if (value >= 0 && value < REDIS_SHARED_INTEGERS && + pthread_equal(pthread_self(),server.mainthread)) { decrRefCount(o); incrRefCount(shared.integers[value]); return shared.integers[value]; diff --git a/src/redis.c b/src/redis.c index 86536b43..433eae37 100644 --- a/src/redis.c +++ b/src/redis.c @@ -760,6 +760,7 @@ void initServer() { signal(SIGPIPE, SIG_IGN); setupSigSegvAction(); + server.mainthread = pthread_self(); server.devnull = fopen("/dev/null","w"); if (server.devnull == NULL) { redisLog(REDIS_WARNING, "Can't open /dev/null: %s", server.neterr); diff --git a/src/redis.h b/src/redis.h index 4ff83620..d5fabc2d 100644 --- a/src/redis.h +++ b/src/redis.h @@ -327,6 +327,7 @@ struct sharedObjectsStruct { /* Global server state structure */ struct redisServer { + pthread_t mainthread; int port; int fd; redisDb *db; From cdbea20afb232a3603f988a1dc235f75d8eff981 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 22 Jul 2010 13:12:24 +0200 Subject: [PATCH 18/49] minor typo fixed in a comment --- src/object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/object.c b/src/object.c index 25567e2a..58f8f44c 100644 --- a/src/object.c +++ b/src/object.c @@ -216,7 +216,7 @@ robj *tryObjectEncoding(robj *o) { /* Ok, this object can be encoded... * * Can I use a shared object? Only if the object is inside a given - * range and if this is the main thread, sinc when VM is enabled we + * range and if this is the main thread, since when VM is enabled we * have the constraint that I/O thread should only handle non-shared * objects, in order to avoid race conditions (we don't have per-object * locking). */ From e002ec6801f41d29e1038f687b52dfe6c7fc9c8a Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 22 Jul 2010 14:48:45 +0200 Subject: [PATCH 19/49] other shared objects where created in the I/O thread in createStringObjectFromLongLong. Fixed as well. --- src/object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/object.c b/src/object.c index 58f8f44c..7abecfc6 100644 --- a/src/object.c +++ b/src/object.c @@ -35,7 +35,8 @@ robj *createStringObject(char *ptr, size_t len) { robj *createStringObjectFromLongLong(long long value) { robj *o; - if (value >= 0 && value < REDIS_SHARED_INTEGERS) { + if (value >= 0 && value < REDIS_SHARED_INTEGERS && + pthread_equal(pthread_self(),server.mainthread)) { incrRefCount(shared.integers[value]); o = shared.integers[value]; } else { From 2f996f02174343ce766710fc6871e2e6df8e73c6 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 22 Jul 2010 15:48:57 +0200 Subject: [PATCH 20/49] defensive programming: set o->ptr to NULL before freeing objects --- src/object.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/object.c b/src/object.c index 7abecfc6..51582619 100644 --- a/src/object.c +++ b/src/object.c @@ -179,6 +179,7 @@ void decrRefCount(void *obj) { case REDIS_HASH: freeHashObject(o); break; default: redisPanic("Unknown object type"); break; } + o->ptr = NULL; /* defensive programming. We'll see NULL in traces. */ if (server.vm_enabled) pthread_mutex_lock(&server.obj_freelist_mutex); if (listLength(server.objfreelist) > REDIS_OBJFREELIST_MAX || !listAddNodeHead(server.objfreelist,o)) From c8a10631d14dfad768b0b924e8dfb240af858dcb Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 22 Jul 2010 16:06:27 +0200 Subject: [PATCH 21/49] fix rare condition where 'key' would already be destroyed while is was needed later on --- src/vm.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/vm.c b/src/vm.c index 1aaa57eb..073cace2 100644 --- a/src/vm.c +++ b/src/vm.c @@ -1075,6 +1075,11 @@ int dontWaitForSwappedKey(redisClient *c, robj *key) { listIter li; struct dictEntry *de; + /* The key object might be destroyed when deleted from the c->io_keys + * list (and the "key" argument is physically the same object as the + * object inside the list), so we need to protect it. */ + incrRefCount(key); + /* Remove the key from the list of keys this client is waiting for. */ listRewind(c->io_keys,&li); while ((ln = listNext(&li)) != NULL) { @@ -1095,6 +1100,7 @@ int dontWaitForSwappedKey(redisClient *c, robj *key) { if (listLength(l) == 0) dictDelete(c->db->io_keys,key); + decrRefCount(key); return listLength(c->io_keys) == 0; } From 230729617dc6714858902da5520b4e2c1ecc711e Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 22 Jul 2010 23:31:40 +0200 Subject: [PATCH 22/49] don't open/close log file if log level is not matched --- src/redis.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/redis.c b/src/redis.c index 433eae37..9978eab5 100644 --- a/src/redis.c +++ b/src/redis.c @@ -186,23 +186,22 @@ struct redisCommand readonlyCommandTable[] = { void redisLog(int level, const char *fmt, ...) { va_list ap; FILE *fp; + char *c = ".-*#"; + char buf[64]; + time_t now; + + if (level < server.verbosity) return; fp = (server.logfile == NULL) ? stdout : fopen(server.logfile,"a"); if (!fp) return; va_start(ap, fmt); - if (level >= server.verbosity) { - char *c = ".-*#"; - char buf[64]; - time_t now; - - now = time(NULL); - strftime(buf,64,"%d %b %H:%M:%S",localtime(&now)); - fprintf(fp,"[%d] %s %c ",(int)getpid(),buf,c[level]); - vfprintf(fp, fmt, ap); - fprintf(fp,"\n"); - fflush(fp); - } + now = time(NULL); + strftime(buf,64,"%d %b %H:%M:%S",localtime(&now)); + fprintf(fp,"[%d] %s %c ",(int)getpid(),buf,c[level]); + vfprintf(fp, fmt, ap); + fprintf(fp,"\n"); + fflush(fp); va_end(ap); if (server.logfile) fclose(fp); From e39c8b5047c63336f67a7749e8f8cdd539c5eb22 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 23 Jul 2010 13:08:35 +0200 Subject: [PATCH 23/49] exit with non-zero status when there are failed tests --- tests/test_helper.tcl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_helper.tcl b/tests/test_helper.tcl index 59470e64..ef1f9923 100644 --- a/tests/test_helper.tcl +++ b/tests/test_helper.tcl @@ -102,13 +102,13 @@ proc main {} { execute_tests "unit/expire" execute_tests "unit/other" execute_tests "unit/cas" - + + cleanup puts "\n[expr $::passed+$::failed] tests, $::passed passed, $::failed failed" if {$::failed > 0} { puts "\n*** WARNING!!! $::failed FAILED TESTS ***\n" + exit 1 } - - cleanup } # parse arguments From b1e0bd4b9b5a9c203a4400bd52a06e377876f423 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Sat, 24 Jul 2010 22:37:01 +0200 Subject: [PATCH 24/49] Reduce code duplication --- src/dict.c | 60 +++++++++++++++++++----------------------------------- 1 file changed, 21 insertions(+), 39 deletions(-) diff --git a/src/dict.c b/src/dict.c index d5010708..79c005b6 100644 --- a/src/dict.c +++ b/src/dict.c @@ -651,7 +651,7 @@ static unsigned int _dictStringCopyHTHashFunction(const void *key) return dictGenHashFunction(key, strlen(key)); } -static void *_dictStringCopyHTKeyDup(void *privdata, const void *key) +static void *_dictStringDup(void *privdata, const void *key) { int len = strlen(key); char *copy = _dictAlloc(len+1); @@ -662,17 +662,6 @@ static void *_dictStringCopyHTKeyDup(void *privdata, const void *key) return copy; } -static void *_dictStringKeyValCopyHTValDup(void *privdata, const void *val) -{ - int len = strlen(val); - char *copy = _dictAlloc(len+1); - DICT_NOTUSED(privdata); - - memcpy(copy, val, len); - copy[len] = '\0'; - return copy; -} - static int _dictStringCopyHTKeyCompare(void *privdata, const void *key1, const void *key2) { @@ -681,47 +670,40 @@ static int _dictStringCopyHTKeyCompare(void *privdata, const void *key1, return strcmp(key1, key2) == 0; } -static void _dictStringCopyHTKeyDestructor(void *privdata, void *key) +static void _dictStringDestructor(void *privdata, void *key) { DICT_NOTUSED(privdata); - _dictFree((void*)key); /* ATTENTION: const cast */ -} - -static void _dictStringKeyValCopyHTValDestructor(void *privdata, void *val) -{ - DICT_NOTUSED(privdata); - - _dictFree((void*)val); /* ATTENTION: const cast */ + _dictFree(key); } dictType dictTypeHeapStringCopyKey = { - _dictStringCopyHTHashFunction, /* hash function */ - _dictStringCopyHTKeyDup, /* key dup */ - NULL, /* val dup */ - _dictStringCopyHTKeyCompare, /* key compare */ - _dictStringCopyHTKeyDestructor, /* key destructor */ - NULL /* val destructor */ + _dictStringCopyHTHashFunction, /* hash function */ + _dictStringDup, /* key dup */ + NULL, /* val dup */ + _dictStringCopyHTKeyCompare, /* key compare */ + _dictStringDestructor, /* key destructor */ + NULL /* val destructor */ }; /* This is like StringCopy but does not auto-duplicate the key. * It's used for intepreter's shared strings. */ dictType dictTypeHeapStrings = { - _dictStringCopyHTHashFunction, /* hash function */ - NULL, /* key dup */ - NULL, /* val dup */ - _dictStringCopyHTKeyCompare, /* key compare */ - _dictStringCopyHTKeyDestructor, /* key destructor */ - NULL /* val destructor */ + _dictStringCopyHTHashFunction, /* hash function */ + NULL, /* key dup */ + NULL, /* val dup */ + _dictStringCopyHTKeyCompare, /* key compare */ + _dictStringDestructor, /* key destructor */ + NULL /* val destructor */ }; /* This is like StringCopy but also automatically handle dynamic * allocated C strings as values. */ dictType dictTypeHeapStringCopyKeyValue = { - _dictStringCopyHTHashFunction, /* hash function */ - _dictStringCopyHTKeyDup, /* key dup */ - _dictStringKeyValCopyHTValDup, /* val dup */ - _dictStringCopyHTKeyCompare, /* key compare */ - _dictStringCopyHTKeyDestructor, /* key destructor */ - _dictStringKeyValCopyHTValDestructor, /* val destructor */ + _dictStringCopyHTHashFunction, /* hash function */ + _dictStringDup, /* key dup */ + _dictStringDup, /* val dup */ + _dictStringCopyHTKeyCompare, /* key compare */ + _dictStringDestructor, /* key destructor */ + _dictStringDestructor, /* val destructor */ }; From d9dd352b3693e1ad8ab2e0993b4f0a275d7fd512 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Sat, 24 Jul 2010 23:10:42 +0200 Subject: [PATCH 25/49] Remove _dictAlloc and friends zmalloc calls abort() so _dictPanic will never be called. --- src/dict.c | 51 ++++++++++++--------------------------------------- 1 file changed, 12 insertions(+), 39 deletions(-) diff --git a/src/dict.c b/src/dict.c index 79c005b6..0d4aacb7 100644 --- a/src/dict.c +++ b/src/dict.c @@ -52,33 +52,6 @@ * around when there is a child performing saving operations. */ static int dict_can_resize = 1; -/* ---------------------------- Utility funcitons --------------------------- */ - -static void _dictPanic(const char *fmt, ...) -{ - va_list ap; - - va_start(ap, fmt); - fprintf(stderr, "\nDICT LIBRARY PANIC: "); - vfprintf(stderr, fmt, ap); - fprintf(stderr, "\n\n"); - va_end(ap); -} - -/* ------------------------- Heap Management Wrappers------------------------ */ - -static void *_dictAlloc(size_t size) -{ - void *p = zmalloc(size); - if (p == NULL) - _dictPanic("Out of memory"); - return p; -} - -static void _dictFree(void *ptr) { - zfree(ptr); -} - /* -------------------------- private prototypes ---------------------------- */ static int _dictExpandIfNeeded(dict *ht); @@ -132,7 +105,7 @@ static void _dictReset(dictht *ht) dict *dictCreate(dictType *type, void *privDataPtr) { - dict *d = _dictAlloc(sizeof(*d)); + dict *d = zmalloc(sizeof(*d)); _dictInit(d,type,privDataPtr); return d; @@ -177,7 +150,7 @@ int dictExpand(dict *d, unsigned long size) n.size = realsize; n.sizemask = realsize-1; - n.table = _dictAlloc(realsize*sizeof(dictEntry*)); + n.table = zmalloc(realsize*sizeof(dictEntry*)); n.used = 0; /* Initialize all the pointers to NULL */ @@ -208,7 +181,7 @@ int dictRehash(dict *d, int n) { /* Check if we already rehashed the whole table... */ if (d->ht[0].used == 0) { - _dictFree(d->ht[0].table); + zfree(d->ht[0].table); d->ht[0] = d->ht[1]; _dictReset(&d->ht[1]); d->rehashidx = -1; @@ -285,7 +258,7 @@ int dictAdd(dict *d, void *key, void *val) /* Allocates the memory and stores key */ ht = dictIsRehashing(d) ? &d->ht[1] : &d->ht[0]; - entry = _dictAlloc(sizeof(*entry)); + entry = zmalloc(sizeof(*entry)); entry->next = ht->table[index]; ht->table[index] = entry; ht->used++; @@ -348,7 +321,7 @@ static int dictGenericDelete(dict *d, const void *key, int nofree) dictFreeEntryKey(d, he); dictFreeEntryVal(d, he); } - _dictFree(he); + zfree(he); d->ht[table].used--; return DICT_OK; } @@ -382,13 +355,13 @@ int _dictClear(dict *d, dictht *ht) nextHe = he->next; dictFreeEntryKey(d, he); dictFreeEntryVal(d, he); - _dictFree(he); + zfree(he); ht->used--; he = nextHe; } } /* Free the table and the allocated cache structure */ - _dictFree(ht->table); + zfree(ht->table); /* Re-initialize the table */ _dictReset(ht); return DICT_OK; /* never fails */ @@ -399,7 +372,7 @@ void dictRelease(dict *d) { _dictClear(d,&d->ht[0]); _dictClear(d,&d->ht[1]); - _dictFree(d); + zfree(d); } dictEntry *dictFind(dict *d, const void *key) @@ -432,7 +405,7 @@ void *dictFetchValue(dict *d, const void *key) { dictIterator *dictGetIterator(dict *d) { - dictIterator *iter = _dictAlloc(sizeof(*iter)); + dictIterator *iter = zmalloc(sizeof(*iter)); iter->d = d; iter->table = 0; @@ -475,7 +448,7 @@ dictEntry *dictNext(dictIterator *iter) void dictReleaseIterator(dictIterator *iter) { if (!(iter->index == -1 && iter->table == 0)) iter->d->iterators--; - _dictFree(iter); + zfree(iter); } /* Return a random entry from the hash table. Useful to @@ -654,7 +627,7 @@ static unsigned int _dictStringCopyHTHashFunction(const void *key) static void *_dictStringDup(void *privdata, const void *key) { int len = strlen(key); - char *copy = _dictAlloc(len+1); + char *copy = zmalloc(len+1); DICT_NOTUSED(privdata); memcpy(copy, key, len); @@ -674,7 +647,7 @@ static void _dictStringDestructor(void *privdata, void *key) { DICT_NOTUSED(privdata); - _dictFree(key); + zfree(key); } dictType dictTypeHeapStringCopyKey = { From 399f2f401c6fc3d489e2e40ffc78638425e3a09e Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Sat, 24 Jul 2010 23:20:00 +0200 Subject: [PATCH 26/49] Add zcalloc and use it where appropriate calloc is more effecient than malloc+memset when the system uses mmap to allocate memory. mmap always returns zeroed memory so the memset can be avoided. The threshold to use mmap is 16k in osx libc and 128k in bsd libc and glibc. The kernel can lazily allocate the pages, this reduces memory usage when we have a page table or hash table that is mostly empty. This change is most visible when you start a new redis instance with vm enabled. You'll see no increased memory usage no matter how big your page table is. --- src/dict.c | 6 ++---- src/vm.c | 3 +-- src/zmalloc.c | 14 ++++++++++++++ src/zmalloc.h | 1 + 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/dict.c b/src/dict.c index 0d4aacb7..77ce90cb 100644 --- a/src/dict.c +++ b/src/dict.c @@ -148,14 +148,12 @@ int dictExpand(dict *d, unsigned long size) if (dictIsRehashing(d) || d->ht[0].used > size) return DICT_ERR; + /* Allocate the new hashtable and initialize all pointers to NULL */ n.size = realsize; n.sizemask = realsize-1; - n.table = zmalloc(realsize*sizeof(dictEntry*)); + n.table = zcalloc(realsize*sizeof(dictEntry*)); n.used = 0; - /* Initialize all the pointers to NULL */ - memset(n.table, 0, realsize*sizeof(dictEntry*)); - /* Is this the first initialization? If so it's not really a rehashing * we just set the first hash table so that it can accept keys. */ if (d->ht[0].table == NULL) { diff --git a/src/vm.c b/src/vm.c index 073cace2..0ccc5fe2 100644 --- a/src/vm.c +++ b/src/vm.c @@ -86,10 +86,9 @@ void vmInit(void) { } else { redisLog(REDIS_NOTICE,"Swap file allocated with success"); } - server.vm_bitmap = zmalloc((server.vm_pages+7)/8); + server.vm_bitmap = zcalloc((server.vm_pages+7)/8); redisLog(REDIS_VERBOSE,"Allocated %lld bytes page table for %lld pages", (long long) (server.vm_pages+7)/8, server.vm_pages); - memset(server.vm_bitmap,0,(server.vm_pages+7)/8); /* Initialize threaded I/O (used by Virtual Memory) */ server.io_newjobs = listCreate(); diff --git a/src/zmalloc.c b/src/zmalloc.c index 8658376a..5c1b5e9a 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -89,6 +89,20 @@ void *zmalloc(size_t size) { #endif } +void *zcalloc(size_t size) { + void *ptr = calloc(1, size+PREFIX_SIZE); + + if (!ptr) zmalloc_oom(size); +#ifdef HAVE_MALLOC_SIZE + increment_used_memory(redis_malloc_size(ptr)); + return ptr; +#else + *((size_t*)ptr) = size; + increment_used_memory(size+PREFIX_SIZE); + return (char*)ptr+PREFIX_SIZE; +#endif +} + void *zrealloc(void *ptr, size_t size) { #ifndef HAVE_MALLOC_SIZE void *realptr; diff --git a/src/zmalloc.h b/src/zmalloc.h index 193e7eda..db858bba 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -32,6 +32,7 @@ #define _ZMALLOC_H void *zmalloc(size_t size); +void *zcalloc(size_t size); void *zrealloc(void *ptr, size_t size); void zfree(void *ptr); char *zstrdup(const char *s); From b3aa6d712e1345a57696e4d260ce49ccac253ba7 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Jul 2010 09:36:42 +0200 Subject: [PATCH 27/49] use the function deprecated attribute if compiling with GCC to get warnings for malloc/free usages. We always want to use our zmalloc/zfree versions for memory usage tracking --- src/redis.c | 2 +- src/redis.h | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/redis.c b/src/redis.c index 9978eab5..f6040dfe 100644 --- a/src/redis.c +++ b/src/redis.c @@ -826,7 +826,7 @@ int qsortRedisCommands(const void *r1, const void *r2) { void sortCommandTable() { /* Copy and sort the read-only version of the command table */ - commandTable = (struct redisCommand*)malloc(sizeof(readonlyCommandTable)); + commandTable = (struct redisCommand*)zmalloc(sizeof(readonlyCommandTable)); memcpy(commandTable,readonlyCommandTable,sizeof(readonlyCommandTable)); qsort(commandTable, sizeof(readonlyCommandTable)/sizeof(struct redisCommand), diff --git a/src/redis.h b/src/redis.h index d5fabc2d..7aca2abc 100644 --- a/src/redis.h +++ b/src/redis.h @@ -885,4 +885,12 @@ void publishCommand(redisClient *c); void watchCommand(redisClient *c); void unwatchCommand(redisClient *c); +#if defined(__GNUC__) +void *malloc(size_t size) __attribute__ ((deprecated)); +void *calloc(size_t count, size_t size) __attribute__ ((deprecated)); +void free(void *ptr) __attribute__ ((deprecated)); +void *malloc(size_t size) __attribute__ ((deprecated)); +void *realloc(void *ptr, size_t size) __attribute__ ((deprecated)); +#endif + #endif From e0be2289e9582423495c5b36a413c02765cde3ea Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Jul 2010 10:00:38 +0200 Subject: [PATCH 28/49] hash table example commented out in dict.c --- src/dict.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/dict.c b/src/dict.c index 77ce90cb..2d1e752b 100644 --- a/src/dict.c +++ b/src/dict.c @@ -615,6 +615,12 @@ void dictDisableResize(void) { dict_can_resize = 0; } +#if 0 + +/* The following are just example hash table types implementations. + * Not useful for Redis so they are commented out. + */ + /* ----------------------- StringCopy Hash Table Type ------------------------*/ static unsigned int _dictStringCopyHTHashFunction(const void *key) @@ -678,3 +684,4 @@ dictType dictTypeHeapStringCopyKeyValue = { _dictStringDestructor, /* key destructor */ _dictStringDestructor, /* val destructor */ }; +#endif From 80091bbaacf83d82635f3202c1db3f61f56dc0d0 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Jul 2010 10:09:26 +0200 Subject: [PATCH 29/49] STRLEN command implemented --- src/redis.c | 1 + src/redis.h | 1 + src/t_string.c | 9 +++++++++ tests/unit/basic.tcl | 14 ++++++++++++++ 4 files changed, 25 insertions(+) diff --git a/src/redis.c b/src/redis.c index f6040dfe..c8b1c781 100644 --- a/src/redis.c +++ b/src/redis.c @@ -74,6 +74,7 @@ struct redisCommand readonlyCommandTable[] = { {"setex",setexCommand,4,REDIS_CMD_BULK|REDIS_CMD_DENYOOM,NULL,0,0,0}, {"append",appendCommand,3,REDIS_CMD_BULK|REDIS_CMD_DENYOOM,NULL,1,1,1}, {"substr",substrCommand,4,REDIS_CMD_INLINE,NULL,1,1,1}, + {"strlen",strlenCommand,2,REDIS_CMD_INLINE,NULL,1,1,1}, {"del",delCommand,-2,REDIS_CMD_INLINE,NULL,0,0,0}, {"exists",existsCommand,2,REDIS_CMD_INLINE,NULL,1,1,1}, {"incr",incrCommand,2,REDIS_CMD_INLINE|REDIS_CMD_DENYOOM,NULL,1,1,1}, diff --git a/src/redis.h b/src/redis.h index 7aca2abc..63041860 100644 --- a/src/redis.h +++ b/src/redis.h @@ -859,6 +859,7 @@ void blpopCommand(redisClient *c); void brpopCommand(redisClient *c); void appendCommand(redisClient *c); void substrCommand(redisClient *c); +void strlenCommand(redisClient *c); void zrankCommand(redisClient *c); void zrevrankCommand(redisClient *c); void hsetCommand(redisClient *c); diff --git a/src/t_string.c b/src/t_string.c index 281bd6be..f55595c2 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -252,4 +252,13 @@ void substrCommand(redisClient *c) { decrRefCount(o); } +void strlenCommand(redisClient *c) { + robj *o; + if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL || + checkType(c,o,REDIS_STRING)) return; + + o = getDecodedObject(o); + addReplyLongLong(c,sdslen(o->ptr)); + decrRefCount(o); +} diff --git a/tests/unit/basic.tcl b/tests/unit/basic.tcl index 0d50fa1b..f888cabc 100644 --- a/tests/unit/basic.tcl +++ b/tests/unit/basic.tcl @@ -368,4 +368,18 @@ start_server {tags {"basic"}} { r expire z 10000 list [r msetnx x A y B z C] [r mget x y z] } {0 {1 {} {}}} + + test {STRLEN against non existing key} { + r strlen notakey + } {0} + + test {STRLEN against integer} { + r set myinteger -555 + r strlen myinteger + } {4} + + test {STRLEN against plain string} { + r set mystring "foozzz0123456789 baz" + r strlen mystring + } } From f99e660b44281cd6bd6c24c60f1ebfb5e0e994e1 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Jul 2010 14:30:02 +0200 Subject: [PATCH 30/49] malloc definition with deprecated attribute was duplicated, one removed --- src/redis.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/redis.h b/src/redis.h index 63041860..fb051f8e 100644 --- a/src/redis.h +++ b/src/redis.h @@ -887,7 +887,6 @@ void watchCommand(redisClient *c); void unwatchCommand(redisClient *c); #if defined(__GNUC__) -void *malloc(size_t size) __attribute__ ((deprecated)); void *calloc(size_t count, size_t size) __attribute__ ((deprecated)); void free(void *ptr) __attribute__ ((deprecated)); void *malloc(size_t size) __attribute__ ((deprecated)); From dd3f505ff527be62f422bf164d5ef62932d0f06a Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Jul 2010 14:42:11 +0200 Subject: [PATCH 31/49] Consistency test improved --- tests/support/util.tcl | 49 +++++++++++++++++++++++++++++++ tests/unit/other.tcl | 66 ++++++++++++++++++++++++++++++++---------- 2 files changed, 99 insertions(+), 16 deletions(-) diff --git a/tests/support/util.tcl b/tests/support/util.tcl index ae758f4c..26cf1dc1 100644 --- a/tests/support/util.tcl +++ b/tests/support/util.tcl @@ -224,3 +224,52 @@ proc formatCommand {args} { } set _ $cmd } + +proc csvdump r { + set o {} + foreach k [lsort [$r keys *]] { + set type [$r type $k] + append o [csvstring $k] , [csvstring $type] , + switch $type { + string { + append o [csvstring [$r get $k]] "\n" + } + list { + foreach e [$r lrange $k 0 -1] { + append o [csvstring $e] , + } + append o "\n" + } + set { + foreach e [lsort [$r smembers $k]] { + append o [csvstring $e] , + } + append o "\n" + } + zset { + foreach e [$r zrange $k 0 -1 withscores] { + append o [csvstring $e] , + } + append o "\n" + } + hash { + set fields [$r hgetall $k] + set newfields {} + foreach {k v} $fields { + lappend newfields [list $k $v] + } + set fields [lsort -index 0 $newfields] + foreach kv $fields { + append o [csvstring [lindex $kv 0]] , + append o [csvstring [lindex $kv 1]] , + } + append o "\n" + } + } + } + return $o +} + +proc csvstring s { + return "\"$s\"" +} diff --git a/tests/unit/other.tcl b/tests/unit/other.tcl index a2e8ba9e..024adace 100644 --- a/tests/unit/other.tcl +++ b/tests/unit/other.tcl @@ -46,23 +46,57 @@ start_server {} { set _ $err } {*invalid*} - if {![catch {package require sha1}]} { - test {Check consistency of different data types after a reload} { - r flushdb - createComplexDataset r 10000 - set sha1 [r debug digest] - r debug reload - set sha1_after [r debug digest] - expr {$sha1 eq $sha1_after} - } {1} + tags {consistency} { + if {![catch {package require sha1}]} { + test {Check consistency of different data types after a reload} { + r flushdb + createComplexDataset r 10000 + set dump [csvdump r] + set sha1 [r debug digest] + r debug reload + r set baubau x + set sha1_after [r debug digest] + if {$sha1 eq $sha1_after} { + set _ 1 + } else { + set newdump [csvdump r] + puts "Consistency test failed!" + puts "You can inspect the two dumps in /tmp/repldump*.txt" - test {Same dataset digest if saving/reloading as AOF?} { - r bgrewriteaof - waitForBgrewriteaof r - r debug loadaof - set sha1_after [r debug digest] - expr {$sha1 eq $sha1_after} - } {1} + set fd [open /tmp/repldump1.txt w] + puts $fd $dump + close $fd + set fd [open /tmp/repldump2.txt w] + puts $fd $newdump + close $fd + + set _ 0 + } + } {1} + + test {Same dataset digest if saving/reloading as AOF?} { + r bgrewriteaof + waitForBgrewriteaof r + r debug loadaof + set sha1_after [r debug digest] + if {$sha1 eq $sha1_after} { + set _ 1 + } else { + set newdump [csvdump r] + puts "Consistency test failed!" + puts "You can inspect the two dumps in /tmp/aofdump*.txt" + + set fd [open /tmp/aofdump1.txt w] + puts $fd $dump + close $fd + set fd [open /tmp/aofdump2.txt w] + puts $fd $newdump + close $fd + + set _ 0 + } + } {1} + } } test {EXPIRES after a reload (snapshot + append only file)} { From db0c43a70c5da44642609414c12c0efbd2ef9746 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Jul 2010 14:46:39 +0200 Subject: [PATCH 32/49] removed test code having bad effects... --- tests/unit/other.tcl | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/other.tcl b/tests/unit/other.tcl index 024adace..f0497b62 100644 --- a/tests/unit/other.tcl +++ b/tests/unit/other.tcl @@ -54,7 +54,6 @@ start_server {} { set dump [csvdump r] set sha1 [r debug digest] r debug reload - r set baubau x set sha1_after [r debug digest] if {$sha1 eq $sha1_after} { set _ 1 From 6171250871e408928a172c09d5fdf41961720fbc Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Jul 2010 15:26:08 +0200 Subject: [PATCH 33/49] fixed a ziplist bug about encoding of integer values overflowing 64 bit --- src/ziplist.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/ziplist.c b/src/ziplist.c index 6c5827b9..7a3a8b01 100644 --- a/src/ziplist.c +++ b/src/ziplist.c @@ -23,6 +23,8 @@ #include "zmalloc.h" #include "ziplist.h" +int ll2string(char *s, size_t len, long long value); + /* Important note: the ZIP_END value is used to depict the end of the * ziplist structure. When a pointer contains an entry, the first couple * of bytes contain the encoded length of the previous entry. This length @@ -174,15 +176,27 @@ static int zipPrevLenByteDiff(unsigned char *p, unsigned int len) { } /* Check if string pointed to by 'entry' can be encoded as an integer. - * Stores the integer value in 'v' and its encoding in 'encoding'. - * Warning: this function requires a NULL-terminated string! */ -static int zipTryEncoding(unsigned char *entry, long long *v, unsigned char *encoding) { + * Stores the integer value in 'v' and its encoding in 'encoding'. */ +static int zipTryEncoding(unsigned char *entry, unsigned int entrylen, long long *v, unsigned char *encoding) { long long value; char *eptr; + char buf[32]; + if (entrylen >= 32 || entrylen == 0) return 0; if (entry[0] == '-' || (entry[0] >= '0' && entry[0] <= '9')) { - value = strtoll((char*)entry,&eptr,10); + int slen; + + /* Perform a back-and-forth conversion to make sure that + * the string turned into an integer is not losing any info. */ + memcpy(buf,entry,entrylen); + buf[entrylen] = '\0'; + value = strtoll(buf,&eptr,10); if (eptr[0] != '\0') return 0; + slen = ll2string(buf,32,value); + if (entrylen != (unsigned)slen || memcmp(buf,entry,slen)) return 0; + + /* Great, the string can be encoded. Check what's the smallest + * of our encoding types that can hold this value. */ if (value >= INT16_MIN && value <= INT16_MAX) { *encoding = ZIP_ENC_INT16; } else if (value >= INT32_MIN && value <= INT32_MAX) { @@ -329,7 +343,7 @@ static unsigned char *__ziplistInsert(unsigned char *zl, unsigned char *p, unsig } /* See if the entry can be encoded */ - if (zipTryEncoding(s,&value,&encoding)) { + if (zipTryEncoding(s,slen,&value,&encoding)) { reqlen = zipEncodingSize(encoding); } else { reqlen = slen; @@ -505,7 +519,7 @@ unsigned int ziplistCompare(unsigned char *p, unsigned char *sstr, unsigned int } } else { /* Try to compare encoded values */ - if (zipTryEncoding(sstr,&sval,&sencoding)) { + if (zipTryEncoding(sstr,slen,&sval,&sencoding)) { if (entry.encoding == sencoding) { zval = zipLoadInteger(p+entry.headersize,entry.encoding); return zval == sval; From a0573260b0928170d66268eb22be1a5699615275 Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 28 Jul 2010 14:08:46 +0200 Subject: [PATCH 34/49] better random dataset creation function in test. master-slave replication test now is able to save the two datasets in CSV when an inconsistency is detected. --- tests/integration/replication.tcl | 13 ++++++ tests/support/util.tcl | 72 ++++++++++++++++++------------- 2 files changed, 54 insertions(+), 31 deletions(-) diff --git a/tests/integration/replication.tcl b/tests/integration/replication.tcl index 39d77c8f..4b258825 100644 --- a/tests/integration/replication.tcl +++ b/tests/integration/replication.tcl @@ -8,6 +8,19 @@ start_server {tags {"repl"}} { test {MASTER and SLAVE dataset should be identical after complex ops} { createComplexDataset r 10000 + after 500 + if {[r debug digest] ne [r -1 debug digest]} { + set csv1 [csvdump r] + set csv2 [csvdump {r -1}] + set fd [open /tmp/repldump1.txt w] + puts -nonewline $fd $csv1 + close $fd + set fd [open /tmp/repldump2.txt w] + puts -nonewline $fd $csv2 + close $fd + puts "Master - Slave inconsistency" + puts "Run diff -u against /tmp/repldump*.txt for more info" + } assert_equal [r debug digest] [r -1 debug digest] } } diff --git a/tests/support/util.tcl b/tests/support/util.tcl index 26cf1dc1..b9c89aa8 100644 --- a/tests/support/util.tcl +++ b/tests/support/util.tcl @@ -44,7 +44,7 @@ proc warnings_from_file {filename} { # Return value for INFO property proc status {r property} { - if {[regexp "\r\n$property:(.*?)\r\n" [$r info] _ value]} { + if {[regexp "\r\n$property:(.*?)\r\n" [{*}$r info] _ value]} { set _ $value } } @@ -129,11 +129,11 @@ proc randomKey {} { proc findKeyWithType {r type} { for {set j 0} {$j < 20} {incr j} { - set k [$r randomkey] + set k [{*}$r randomkey] if {$k eq {}} { return {} } - if {[$r type $k] eq $type} { + if {[{*}$r type $k] eq $type} { return $k } } @@ -159,23 +159,23 @@ proc createComplexDataset {r ops} { } { randpath {set d +inf} {set d -inf} } - set t [$r type $k] + set t [{*}$r type $k] if {$t eq {none}} { randpath { - $r set $k $v + {*}$r set $k $v } { - $r lpush $k $v + {*}$r lpush $k $v } { - $r sadd $k $v + {*}$r sadd $k $v } { - $r zadd $k $d $v + {*}$r zadd $k $d $v } { - $r hset $k $f $v + {*}$r hset $k $f $v } { - $r del $k + {*}$r del $k } - set t [$r type $k] + set t [{*}$r type $k] } switch $t { @@ -183,35 +183,45 @@ proc createComplexDataset {r ops} { # Nothing to do } {list} { - randpath {$r lpush $k $v} \ - {$r rpush $k $v} \ - {$r lrem $k 0 $v} \ - {$r rpop $k} \ - {$r lpop $k} + randpath {{*}$r lpush $k $v} \ + {{*}$r rpush $k $v} \ + {{*}$r lrem $k 0 $v} \ + {{*}$r rpop $k} \ + {{*}$r lpop $k} } {set} { - randpath {$r sadd $k $v} \ - {$r srem $k $v} \ + randpath {{*}$r sadd $k $v} \ + {{*}$r srem $k $v} \ { set otherset [findKeyWithType r set] if {$otherset ne {}} { - $r sunionstore $k2 $k $otherset + randpath { + {*}$r sunionstore $k2 $k $otherset + } { + {*}$r sinterstore $k2 $k $otherset + } { + {*}$r sdiffstore $k2 $k $otherset + } } } } {zset} { - randpath {$r zadd $k $d $v} \ - {$r zrem $k $v} \ + randpath {{*}$r zadd $k $d $v} \ + {{*}$r zrem $k $v} \ { set otherzset [findKeyWithType r zset] if {$otherzset ne {}} { - $r zunionstore $k2 2 $k $otherzset + randpath { + {*}$r zunionstore $k2 2 $k $otherzset + } { + {*}$r zinterstore $k2 2 $k $otherzset + } } } } {hash} { - randpath {$r hset $k $f $v} \ - {$r hdel $k $f} + randpath {{*}$r hset $k $f $v} \ + {{*}$r hdel $k $f} } } } @@ -227,33 +237,33 @@ proc formatCommand {args} { proc csvdump r { set o {} - foreach k [lsort [$r keys *]] { - set type [$r type $k] + foreach k [lsort [{*}$r keys *]] { + set type [{*}$r type $k] append o [csvstring $k] , [csvstring $type] , switch $type { string { - append o [csvstring [$r get $k]] "\n" + append o [csvstring [{*}$r get $k]] "\n" } list { - foreach e [$r lrange $k 0 -1] { + foreach e [{*}$r lrange $k 0 -1] { append o [csvstring $e] , } append o "\n" } set { - foreach e [lsort [$r smembers $k]] { + foreach e [lsort [{*}$r smembers $k]] { append o [csvstring $e] , } append o "\n" } zset { - foreach e [$r zrange $k 0 -1 withscores] { + foreach e [{*}$r zrange $k 0 -1 withscores] { append o [csvstring $e] , } append o "\n" } hash { - set fields [$r hgetall $k] + set fields [{*}$r hgetall $k] set newfields {} foreach {k v} $fields { lappend newfields [list $k $v] From 8c1420ff2a2a9e68ab3faa98bb82d682301fa66b Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 28 Jul 2010 18:42:02 +0200 Subject: [PATCH 35/49] Fixed a replication bug in ZINTERSTORE. In order to trigger the bug what's needed is to call ZINTERSTORE resulting into an empty set created, bug against a key that already existed. The command was not propagated, so the replica ended with the key that the master removed. Sequence of command to reproduce: redis-cli hset 446 34 905 redis-cli hset 446 393 911 redis-cli zadd 966 0.085412045980529885 652 redis-cli zadd 645 0.25081839284432045 280 redis-cli zinterstore 446 2 966 645 --- src/t_zset.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/t_zset.c b/src/t_zset.c index 0fcd6ea3..9b59ca9a 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -559,6 +559,7 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { zset *dstzset; dictIterator *di; dictEntry *de; + int touched = 0; /* expect setnum input keys to be given */ setnum = atoi(c->argv[2]->ptr); @@ -703,12 +704,15 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { redisAssert(op == REDIS_OP_INTER || op == REDIS_OP_UNION); } - dbDelete(c->db,dstkey); + if (dbDelete(c->db,dstkey)) { + touchWatchedKey(c->db,dstkey); + touched = 1; + server.dirty++; + } if (dstzset->zsl->length) { dbAdd(c->db,dstkey,dstobj); addReplyLongLong(c, dstzset->zsl->length); - touchWatchedKey(c->db,dstkey); - server.dirty++; + if (!touched) touchWatchedKey(c->db,dstkey); } else { decrRefCount(dstobj); addReply(c, shared.czero); From cbf7e1070a5f3bcd8024dff481a2f729a2b5cf2f Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 28 Jul 2010 18:56:52 +0200 Subject: [PATCH 36/49] fix of the fix for the replication bug --- src/t_zset.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/t_zset.c b/src/t_zset.c index 9b59ca9a..8efe3c2a 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -713,6 +713,7 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { dbAdd(c->db,dstkey,dstobj); addReplyLongLong(c, dstzset->zsl->length); if (!touched) touchWatchedKey(c->db,dstkey); + server.dirty++; } else { decrRefCount(dstobj); addReply(c, shared.czero); From 86d392498ba41501f9508e03f6c830d7a5601631 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 29 Jul 2010 13:31:24 +0200 Subject: [PATCH 37/49] ensure the value is swapped in before testing its encoding --- tests/support/test.tcl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/support/test.tcl b/tests/support/test.tcl index 2c1fc164..298e4c77 100644 --- a/tests/support/test.tcl +++ b/tests/support/test.tcl @@ -33,9 +33,14 @@ proc assert_error {pattern code} { } proc assert_encoding {enc key} { - # swapped out value doesn't have encoding, so swap in first - r debug swapin $key - assert_match "* encoding:$enc *" [r debug object $key] + # Swapped out values don't have an encoding, so make sure that + # the value is swapped in before checking the encoding. + set dbg [r debug object $key] + while {[string match "* swapped:*" $dbg]} { + [r debug swapin $key] + set dbg [r debug object $key] + } + assert_match "* encoding:$enc *" $dbg } proc assert_type {type key} { From 715c801a07f9157963dc629386eaaf2406ad7572 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 29 Jul 2010 13:53:52 +0200 Subject: [PATCH 38/49] Use a large value to consistently trigger a list encoding, even when the list is swapped out and in again. --- tests/unit/type/list.tcl | 181 ++++++++++++++++++++------------------- 1 file changed, 94 insertions(+), 87 deletions(-) diff --git a/tests/unit/type/list.tcl b/tests/unit/type/list.tcl index 4636cc5b..d3ed90ec 100644 --- a/tests/unit/type/list.tcl +++ b/tests/unit/type/list.tcl @@ -5,6 +5,12 @@ start_server { "list-max-ziplist-entries" 256 } } { + # We need a value larger than list-max-ziplist-value to make sure + # the list has the right encoding when it is swapped in again. + array set largevalue {} + set largevalue(ziplist) "hello" + set largevalue(linkedlist) [string repeat "hello" 4] + test {LPUSH, RPUSH, LLENGTH, LINDEX - ziplist} { # first lpush then rpush assert_equal 1 [r lpush myziplist1 a] @@ -28,28 +34,25 @@ start_server { } test {LPUSH, RPUSH, LLENGTH, LINDEX - regular list} { - # use a string of length 17 to ensure a regular list is used - set large_value "aaaaaaaaaaaaaaaaa" - # first lpush then rpush - assert_equal 1 [r lpush mylist1 $large_value] + assert_equal 1 [r lpush mylist1 $largevalue(linkedlist)] assert_encoding linkedlist mylist1 assert_equal 2 [r rpush mylist1 b] assert_equal 3 [r rpush mylist1 c] assert_equal 3 [r llen mylist1] - assert_equal $large_value [r lindex mylist1 0] + assert_equal $largevalue(linkedlist) [r lindex mylist1 0] assert_equal b [r lindex mylist1 1] assert_equal c [r lindex mylist1 2] # first rpush then lpush - assert_equal 1 [r rpush mylist2 $large_value] + assert_equal 1 [r rpush mylist2 $largevalue(linkedlist)] assert_encoding linkedlist mylist2 assert_equal 2 [r lpush mylist2 b] assert_equal 3 [r lpush mylist2 c] assert_equal 3 [r llen mylist2] assert_equal c [r lindex mylist2 0] assert_equal b [r lindex mylist2 1] - assert_equal $large_value [r lindex mylist2 2] + assert_equal $largevalue(linkedlist) [r lindex mylist2 2] } test {DEL a list - ziplist} { @@ -72,16 +75,14 @@ start_server { proc create_linkedlist {key entries} { r del $key - r rpush $key "aaaaaaaaaaaaaaaaa" foreach entry $entries { r rpush $key $entry } - assert_equal "aaaaaaaaaaaaaaaaa" [r lpop $key] assert_encoding linkedlist $key } - foreach type {ziplist linkedlist} { + foreach {type large} [array get largevalue] { test "BLPOP, BRPOP: single existing list - $type" { set rd [redis_deferring_client] - create_$type blist {a b c d} + create_$type blist "a b $large c d" $rd blpop blist 1 assert_equal {blist a} [$rd read] @@ -96,8 +97,8 @@ start_server { test "BLPOP, BRPOP: multiple existing lists - $type" { set rd [redis_deferring_client] - create_$type blist1 {a b c} - create_$type blist2 {d e f} + create_$type blist1 "a $large c" + create_$type blist2 "d $large f" $rd blpop blist1 blist2 1 assert_equal {blist1 a} [$rd read] @@ -117,7 +118,7 @@ start_server { test "BLPOP, BRPOP: second list has an entry - $type" { set rd [redis_deferring_client] r del blist1 - create_$type blist2 {d e f} + create_$type blist2 "d $large f" $rd blpop blist1 blist2 1 assert_equal {blist2 d} [$rd read] @@ -179,26 +180,26 @@ start_server { assert_equal 0 [r llen xlist] } - foreach type {ziplist linkedlist} { + foreach {type large} [array get largevalue] { test "LPUSHX, RPUSHX - $type" { - create_$type xlist {b c} + create_$type xlist "$large c" assert_equal 3 [r rpushx xlist d] assert_equal 4 [r lpushx xlist a] - assert_equal {a b c d} [r lrange xlist 0 -1] + assert_equal "a $large c d" [r lrange xlist 0 -1] } test "LINSERT - $type" { - create_$type xlist {a b c d} + create_$type xlist "a $large c d" assert_equal 5 [r linsert xlist before c zz] - assert_equal {a b zz c d} [r lrange xlist 0 10] + assert_equal "a $large zz c d" [r lrange xlist 0 10] assert_equal 6 [r linsert xlist after c yy] - assert_equal {a b zz c yy d} [r lrange xlist 0 10] + assert_equal "a $large zz c yy d" [r lrange xlist 0 10] assert_equal 7 [r linsert xlist after d dd] assert_equal -1 [r linsert xlist after bad ddd] - assert_equal {a b zz c yy d dd} [r lrange xlist 0 10] + assert_equal "a $large zz c yy d dd" [r lrange xlist 0 10] assert_equal 8 [r linsert xlist before a aa] assert_equal -1 [r linsert xlist before bad aaa] - assert_equal {aa a b zz c yy d dd} [r lrange xlist 0 10] + assert_equal "aa a $large zz c yy d dd" [r lrange xlist 0 10] # check inserting integer encoded value assert_equal 9 [r linsert xlist before aa 42] @@ -207,14 +208,14 @@ start_server { } test {LPUSHX, RPUSHX convert from ziplist to list} { - set large_value "aaaaaaaaaaaaaaaaa" + set large $largevalue(linkedlist) # convert when a large value is pushed create_ziplist xlist a - assert_equal 2 [r rpushx xlist $large_value] + assert_equal 2 [r rpushx xlist $large] assert_encoding linkedlist xlist create_ziplist xlist a - assert_equal 2 [r lpushx xlist $large_value] + assert_equal 2 [r lpushx xlist $large] assert_encoding linkedlist xlist # convert when the length threshold is exceeded @@ -227,14 +228,14 @@ start_server { } test {LINSERT convert from ziplist to list} { - set large_value "aaaaaaaaaaaaaaaaa" + set large $largevalue(linkedlist) # convert when a large value is inserted create_ziplist xlist a - assert_equal 2 [r linsert xlist before a $large_value] + assert_equal 2 [r linsert xlist before a $large] assert_encoding linkedlist xlist create_ziplist xlist a - assert_equal 2 [r linsert xlist after a $large_value] + assert_equal 2 [r linsert xlist after a $large] assert_encoding linkedlist xlist # convert when the length threshold is exceeded @@ -320,32 +321,38 @@ start_server { assert_error ERR* {r rpush mylist 0} } - foreach type {ziplist linkedlist} { + foreach {type large} [array get largevalue] { test "RPOPLPUSH base case - $type" { r del mylist1 mylist2 - create_$type mylist1 {a b c d} + create_$type mylist1 "a $large c d" assert_equal d [r rpoplpush mylist1 mylist2] assert_equal c [r rpoplpush mylist1 mylist2] - assert_equal {a b} [r lrange mylist1 0 -1] - assert_equal {c d} [r lrange mylist2 0 -1] + assert_equal "a $large" [r lrange mylist1 0 -1] + assert_equal "c d" [r lrange mylist2 0 -1] assert_encoding ziplist mylist2 } test "RPOPLPUSH with the same list as src and dst - $type" { - create_$type mylist {a b c} - assert_equal {a b c} [r lrange mylist 0 -1] + create_$type mylist "a $large c" + assert_equal "a $large c" [r lrange mylist 0 -1] assert_equal c [r rpoplpush mylist mylist] - assert_equal {c a b} [r lrange mylist 0 -1] + assert_equal "c a $large" [r lrange mylist 0 -1] } - foreach othertype {ziplist linkedlist} { + foreach {othertype otherlarge} [array get largevalue] { test "RPOPLPUSH with $type source and existing target $othertype" { - create_$type srclist {a b c d} - create_$othertype dstlist {x} - assert_equal d [r rpoplpush srclist dstlist] + create_$type srclist "a b c $large" + create_$othertype dstlist "$otherlarge" + assert_equal $large [r rpoplpush srclist dstlist] assert_equal c [r rpoplpush srclist dstlist] - assert_equal {a b} [r lrange srclist 0 -1] - assert_equal {c d x} [r lrange dstlist 0 -1] + assert_equal "a b" [r lrange srclist 0 -1] + assert_equal "c $large $otherlarge" [r lrange dstlist 0 -1] + + # When we rpoplpush'ed a large value, dstlist should be + # converted to the same encoding as srclist. + if {$type eq "linkedlist"} { + assert_encoding linkedlist dstlist + } } } } @@ -378,10 +385,10 @@ start_server { assert_equal {} [r rpoplpush srclist dstlist] } {} - foreach type {ziplist linkedlist} { + foreach {type large} [array get largevalue] { test "Basic LPOP/RPOP - $type" { - create_$type mylist {0 1 2} - assert_equal 0 [r lpop mylist] + create_$type mylist "$large 1 2" + assert_equal $large [r lpop mylist] assert_equal 2 [r rpop mylist] assert_equal 1 [r lpop mylist] assert_equal 0 [r llen mylist] @@ -416,28 +423,28 @@ start_server { } } - foreach type {ziplist linkedlist} { + foreach {type large} [array get largevalue] { test "LRANGE basics - $type" { - create_$type mylist {0 1 2 3 4 5 6 7 8 9} + create_$type mylist "$large 1 2 3 4 5 6 7 8 9" assert_equal {1 2 3 4 5 6 7 8} [r lrange mylist 1 -2] assert_equal {7 8 9} [r lrange mylist -3 -1] assert_equal {4} [r lrange mylist 4 4] } test "LRANGE inverted indexes - $type" { - create_$type mylist {0 1 2 3 4 5 6 7 8 9} + create_$type mylist "$large 1 2 3 4 5 6 7 8 9" assert_equal {} [r lrange mylist 6 2] } test "LRANGE out of range indexes including the full list - $type" { - create_$type mylist {1 2 3} - assert_equal {1 2 3} [r lrange mylist -1000 1000] + create_$type mylist "$large 1 2 3" + assert_equal "$large 1 2 3" [r lrange mylist -1000 1000] } test "LRANGE out of range negative end index - $type" { - create_$type mylist {1 2 3} - assert_equal {1} [r lrange mylist 0 -3] - assert_equal {} [r lrange mylist 0 -4] + create_$type mylist "$large 1 2 3" + assert_equal $large [r lrange mylist 0 -4] + assert_equal {} [r lrange mylist 0 -5] } } @@ -445,27 +452,28 @@ start_server { assert_equal {} [r lrange nosuchkey 0 1] } - foreach type {ziplist linkedlist} { + foreach {type large} [array get largevalue] { proc trim_list {type min max} { + upvar 1 large large r del mylist - create_$type mylist {1 2 3 4 5} + create_$type mylist "1 2 3 4 $large" r ltrim mylist $min $max r lrange mylist 0 -1 } test "LTRIM basics - $type" { - assert_equal {1} [trim_list $type 0 0] - assert_equal {1 2} [trim_list $type 0 1] - assert_equal {1 2 3} [trim_list $type 0 2] - assert_equal {2 3} [trim_list $type 1 2] - assert_equal {2 3 4 5} [trim_list $type 1 -1] - assert_equal {2 3 4} [trim_list $type 1 -2] - assert_equal {4 5} [trim_list $type -2 -1] - assert_equal {5} [trim_list $type -1 -1] - assert_equal {1 2 3 4 5} [trim_list $type -5 -1] - assert_equal {1 2 3 4 5} [trim_list $type -10 10] - assert_equal {1 2 3 4 5} [trim_list $type 0 5] - assert_equal {1 2 3 4 5} [trim_list $type 0 10] + assert_equal "1" [trim_list $type 0 0] + assert_equal "1 2" [trim_list $type 0 1] + assert_equal "1 2 3" [trim_list $type 0 2] + assert_equal "2 3" [trim_list $type 1 2] + assert_equal "2 3 4 $large" [trim_list $type 1 -1] + assert_equal "2 3 4" [trim_list $type 1 -2] + assert_equal "4 $large" [trim_list $type -2 -1] + assert_equal "$large" [trim_list $type -1 -1] + assert_equal "1 2 3 4 $large" [trim_list $type -5 -1] + assert_equal "1 2 3 4 $large" [trim_list $type -10 10] + assert_equal "1 2 3 4 $large" [trim_list $type 0 5] + assert_equal "1 2 3 4 $large" [trim_list $type 0 10] } test "LTRIM out of range negative end index - $type" { @@ -478,20 +486,19 @@ start_server { set mylist {} set startlen 32 r del mylist + + # Start with the large value to ensure the + # right encoding is used. + r rpush mylist $large + lappend mylist $large + for {set i 0} {$i < $startlen} {incr i} { set str [randomInt 9223372036854775807] r rpush mylist $str lappend mylist $str } - # do a push/pop of a large value to convert to a real list - if {$type eq "list"} { - r rpush mylist "aaaaaaaaaaaaaaaaa" - r rpop mylist - assert_encoding linkedlist mylist - } - - for {set i 0} {$i < 10000} {incr i} { + for {set i 0} {$i < 1000} {incr i} { set min [expr {int(rand()*$startlen)}] set max [expr {$min+int(rand()*$startlen)}] set mylist [lrange $mylist $min $max] @@ -508,12 +515,12 @@ start_server { } } - foreach type {ziplist linkedlist} { + foreach {type large} [array get largevalue] { test "LSET - $type" { - create_$type mylist {99 98 97 96 95} + create_$type mylist "99 98 $large 96 95" r lset mylist 1 foo r lset mylist -1 bar - assert_equal {99 foo 97 96 bar} [r lrange mylist 0 -1] + assert_equal "99 foo $large 96 bar" [r lrange mylist 0 -1] } test "LSET out of range index - $type" { @@ -530,38 +537,38 @@ start_server { assert_error ERR*value* {r lset nolist 0 foo} } - foreach type {ziplist linkedlist} { + foreach {type e} [array get largevalue] { test "LREM remove all the occurrences - $type" { - create_$type mylist {foo bar foobar foobared zap bar test foo} + create_$type mylist "$e foo bar foobar foobared zap bar test foo" assert_equal 2 [r lrem mylist 0 bar] - assert_equal {foo foobar foobared zap test foo} [r lrange mylist 0 -1] + assert_equal "$e foo foobar foobared zap test foo" [r lrange mylist 0 -1] } test "LREM remove the first occurrence - $type" { assert_equal 1 [r lrem mylist 1 foo] - assert_equal {foobar foobared zap test foo} [r lrange mylist 0 -1] + assert_equal "$e foobar foobared zap test foo" [r lrange mylist 0 -1] } test "LREM remove non existing element - $type" { assert_equal 0 [r lrem mylist 1 nosuchelement] - assert_equal {foobar foobared zap test foo} [r lrange mylist 0 -1] + assert_equal "$e foobar foobared zap test foo" [r lrange mylist 0 -1] } test "LREM starting from tail with negative count - $type" { - create_$type mylist {foo bar foobar foobared zap bar test foo foo} + create_$type mylist "$e foo bar foobar foobared zap bar test foo foo" assert_equal 1 [r lrem mylist -1 bar] - assert_equal {foo bar foobar foobared zap test foo foo} [r lrange mylist 0 -1] + assert_equal "$e foo bar foobar foobared zap test foo foo" [r lrange mylist 0 -1] } test "LREM starting from tail with negative count (2) - $type" { assert_equal 2 [r lrem mylist -2 foo] - assert_equal {foo bar foobar foobared zap test} [r lrange mylist 0 -1] + assert_equal "$e foo bar foobar foobared zap test" [r lrange mylist 0 -1] } test "LREM deleting objects that may be int encoded - $type" { - create_$type myotherlist {1 2 3} + create_$type myotherlist "$e 1 2 3" assert_equal 1 [r lrem myotherlist 1 2] - assert_equal 2 [r llen myotherlist] + assert_equal 3 [r llen myotherlist] } } } From d9e28bcf00a1e52c5e0c8cbc5f2c8c8cb7d7027f Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 29 Jul 2010 21:31:58 +0200 Subject: [PATCH 39/49] Fix ZUNIONSTORE/ZINTERSTORE to never store a NaN score. When +inf and -inf are added, the result is NaN. We don't want NaN scores in a sorted set, so agreed on the result of this operation being zero. --- src/t_zset.c | 4 ++++ tests/unit/type/zset.tcl | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/t_zset.c b/src/t_zset.c index 8efe3c2a..a85a9dc1 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -541,6 +541,10 @@ int qsortCompareZsetopsrcByCardinality(const void *s1, const void *s2) { inline static void zunionInterAggregate(double *target, double val, int aggregate) { if (aggregate == REDIS_AGGR_SUM) { *target = *target + val; + /* The result of adding two doubles is NaN when one variable + * is +inf and the other is -inf. When these numbers are added, + * we maintain the convention of the result being 0.0. */ + if (isnan(*target)) *target = 0.0; } else if (aggregate == REDIS_AGGR_MIN) { *target = val < *target ? val : *target; } else if (aggregate == REDIS_AGGR_MAX) { diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl index da264845..a289d883 100644 --- a/tests/unit/type/zset.tcl +++ b/tests/unit/type/zset.tcl @@ -433,6 +433,30 @@ start_server {tags {"zset"}} { list [r zinterstore zsetc 2 zseta zsetb aggregate max] [r zrange zsetc 0 -1 withscores] } {2 {b 2 c 3}} + foreach cmd {ZUNIONSTORE ZINTERSTORE} { + test "$cmd with +inf/-inf scores" { + r zadd zsetinf1 +inf key + r zadd zsetinf2 +inf key + r $cmd zsetinf3 2 zsetinf1 zsetinf2 + assert_equal inf [r zscore zsetinf3 key] + + r zadd zsetinf1 -inf key + r zadd zsetinf2 +inf key + r $cmd zsetinf3 2 zsetinf1 zsetinf2 + assert_equal 0 [r zscore zsetinf3 key] + + r zadd zsetinf1 +inf key + r zadd zsetinf2 -inf key + r $cmd zsetinf3 2 zsetinf1 zsetinf2 + assert_equal 0 [r zscore zsetinf3 key] + + r zadd zsetinf1 -inf key + r zadd zsetinf2 -inf key + r $cmd zsetinf3 2 zsetinf1 zsetinf2 + assert_equal -inf [r zscore zsetinf3 key] + } + } + tags {"slow"} { test {ZSETs skiplist implementation backlink consistency test} { set diff 0 From 673e1fb7e4c171f5ead560f6886e877f43730cf0 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 29 Jul 2010 22:13:31 +0200 Subject: [PATCH 40/49] Change getDoubleFromObject to fail on NaN. Return an error when the resulting value is not a number (NaN). Fix ZUNIONSTORE/ZINTERSTORE to clean up when a weight argument is not a double value. --- src/object.c | 3 ++- src/t_zset.c | 19 ++++++++----------- tests/unit/type/zset.tcl | 36 +++++++++++++++++++++--------------- 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/object.c b/src/object.c index 51582619..21268340 100644 --- a/src/object.c +++ b/src/object.c @@ -1,5 +1,6 @@ #include "redis.h" #include +#include robj *createObject(int type, void *ptr) { robj *o; @@ -319,7 +320,7 @@ int getDoubleFromObject(robj *o, double *target) { redisAssert(o->type == REDIS_STRING); if (o->encoding == REDIS_ENCODING_RAW) { value = strtod(o->ptr, &eptr); - if (eptr[0] != '\0') return REDIS_ERR; + if (eptr[0] != '\0' || isnan(value)) return REDIS_ERR; } else if (o->encoding == REDIS_ENCODING_INT) { value = (long)o->ptr; } else { diff --git a/src/t_zset.c b/src/t_zset.c index a85a9dc1..e93e5c40 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -327,11 +327,6 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i zset *zs; double *score; - if (isnan(scoreval)) { - addReplySds(c,sdsnew("-ERR provide score is Not A Number (nan)\r\n")); - return; - } - zsetobj = lookupKeyWrite(c->db,key); if (zsetobj == NULL) { zsetobj = createZsetObject(); @@ -361,7 +356,7 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i } if (isnan(*score)) { addReplySds(c, - sdsnew("-ERR resulting score is Not A Number (nan)\r\n")); + sdsnew("-ERR resulting score is not a number (NaN)\r\n")); zfree(score); /* Note that we don't need to check if the zset may be empty and * should be removed here, as we can only obtain Nan as score if @@ -417,15 +412,13 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i void zaddCommand(redisClient *c) { double scoreval; - - if (getDoubleFromObjectOrReply(c, c->argv[2], &scoreval, NULL) != REDIS_OK) return; + if (getDoubleFromObjectOrReply(c,c->argv[2],&scoreval,NULL) != REDIS_OK) return; zaddGenericCommand(c,c->argv[1],c->argv[3],scoreval,0); } void zincrbyCommand(redisClient *c) { double scoreval; - - if (getDoubleFromObjectOrReply(c, c->argv[2], &scoreval, NULL) != REDIS_OK) return; + if (getDoubleFromObjectOrReply(c,c->argv[2],&scoreval,NULL) != REDIS_OK) return; zaddGenericCommand(c,c->argv[1],c->argv[3],scoreval,1); } @@ -608,8 +601,12 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { if (remaining >= (setnum + 1) && !strcasecmp(c->argv[j]->ptr,"weights")) { j++; remaining--; for (i = 0; i < setnum; i++, j++, remaining--) { - if (getDoubleFromObjectOrReply(c, c->argv[j], &src[i].weight, NULL) != REDIS_OK) + if (getDoubleFromObjectOrReply(c,c->argv[j],&src[i].weight, + "weight value is not a double") != REDIS_OK) + { + zfree(src); return; + } } } else if (remaining >= 2 && !strcasecmp(c->argv[j]->ptr,"aggregate")) { j++; remaining--; diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl index a289d883..642922e9 100644 --- a/tests/unit/type/zset.tcl +++ b/tests/unit/type/zset.tcl @@ -435,6 +435,8 @@ start_server {tags {"zset"}} { foreach cmd {ZUNIONSTORE ZINTERSTORE} { test "$cmd with +inf/-inf scores" { + r del zsetinf1 zsetinf2 + r zadd zsetinf1 +inf key r zadd zsetinf2 +inf key r $cmd zsetinf3 2 zsetinf1 zsetinf2 @@ -455,6 +457,16 @@ start_server {tags {"zset"}} { r $cmd zsetinf3 2 zsetinf1 zsetinf2 assert_equal -inf [r zscore zsetinf3 key] } + + test "$cmd with NaN weights" { + r del zsetinf1 zsetinf2 + + r zadd zsetinf1 1.0 key + r zadd zsetinf2 1.0 key + assert_error "*weight value is not a double*" { + r $cmd zsetinf3 2 zsetinf1 zsetinf2 weights nan nan + } + } } tags {"slow"} { @@ -501,22 +513,16 @@ start_server {tags {"zset"}} { } {} } - test {ZSET element can't be set to nan with ZADD} { - set e {} - catch {r zadd myzset nan abc} e - set _ $e - } {*Not A Number*} + test {ZSET element can't be set to NaN with ZADD} { + assert_error "*not a double*" {r zadd myzset nan abc} + } - test {ZSET element can't be set to nan with ZINCRBY} { - set e {} - catch {r zincrby myzset nan abc} e - set _ $e - } {*Not A Number*} + test {ZSET element can't be set to NaN with ZINCRBY} { + assert_error "*not a double*" {r zadd myzset nan abc} + } - test {ZINCRBY calls leading to Nan are refused} { - set e {} + test {ZINCRBY calls leading to NaN result in error} { r zincrby myzset +inf abc - catch {r zincrby myzset -inf abc} e - set _ $e - } {*Not A Number*} + assert_error "*NaN*" {r zincrby myzset -inf abc} + } } From 68254919284ec958225e1bc5fb2951ef096c92d1 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Sun, 1 Aug 2010 11:20:26 +0200 Subject: [PATCH 41/49] Fix assertion function on value encoding --- tests/support/test.tcl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/support/test.tcl b/tests/support/test.tcl index 298e4c77..93f64928 100644 --- a/tests/support/test.tcl +++ b/tests/support/test.tcl @@ -36,8 +36,8 @@ proc assert_encoding {enc key} { # Swapped out values don't have an encoding, so make sure that # the value is swapped in before checking the encoding. set dbg [r debug object $key] - while {[string match "* swapped:*" $dbg]} { - [r debug swapin $key] + while {[string match "* swapped at:*" $dbg]} { + r debug swapin $key set dbg [r debug object $key] } assert_match "* encoding:$enc *" $dbg From bcf2995c987acea7f5485ec0e3717a29a7e98457 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 2 Aug 2010 18:13:39 +0200 Subject: [PATCH 42/49] support for write operations against expiring keys, by master-controlled expiring in replication and AOF synthesizing DEL operations --- src/db.c | 51 +++++++++++++++++++++++-------- src/redis.c | 81 ++++++++++++++++++++++++++++---------------------- src/redis.h | 2 +- src/t_string.c | 1 - 4 files changed, 86 insertions(+), 49 deletions(-) diff --git a/src/db.c b/src/db.c index 958a9f6b..d8a5d0b2 100644 --- a/src/db.c +++ b/src/db.c @@ -45,7 +45,7 @@ robj *lookupKeyRead(redisDb *db, robj *key) { } robj *lookupKeyWrite(redisDb *db, robj *key) { - deleteIfVolatile(db,key); + expireIfNeeded(db,key); return lookupKey(db,key); } @@ -321,7 +321,6 @@ void renameGenericCommand(redisClient *c, int nx) { return; incrRefCount(o); - deleteIfVolatile(c->db,c->argv[2]); if (dbAdd(c->db,c->argv[2],o) == REDIS_ERR) { if (nx) { decrRefCount(o); @@ -375,7 +374,6 @@ void moveCommand(redisClient *c) { } /* Try to add the element to the target DB */ - deleteIfVolatile(dst,c->argv[1]); if (dbAdd(dst,c->argv[1],o) == REDIS_ERR) { addReply(c,shared.czero); return; @@ -430,8 +428,45 @@ time_t getExpire(redisDb *db, robj *key) { return (time_t) dictGetEntryVal(de); } +/* Propagate expires into slaves and the AOF file. + * When a key expires in the master, a DEL operation for this key is sent + * to all the slaves and the AOF file if enabled. + * + * This way the key expiry is centralized in one place, and since both + * AOF and the master->slave link guarantee operation ordering, everything + * will be consistent even if we allow write operations against expiring + * keys. */ +void propagateExpire(redisDb *db, robj *key) { + struct redisCommand *cmd; + robj *argv[2]; + + cmd = lookupCommand("del"); + argv[0] = createStringObject("DEL",3); + argv[1] = key; + incrRefCount(key); + + if (server.appendonly) + feedAppendOnlyFile(cmd,db->id,argv,2); + if (listLength(server.slaves)) + replicationFeedSlaves(server.slaves,db->id,argv,2); + + decrRefCount(key); +} + int expireIfNeeded(redisDb *db, robj *key) { time_t when = getExpire(db,key); + + /* If we are running in the context of a slave, return ASAP: + * the slave key expiration is controlled by the master that will + * send us synthesized DEL operations for expired keys. + * + * Still we try to return the right information to the caller, + * that is, 0 if we think the key should be still valid, 1 if + * we think the key is expired at this time. */ + if (server.masterhost != NULL) { + return time(NULL) > when; + } + if (when < 0) return 0; /* Return when this key has not expired */ @@ -440,15 +475,7 @@ int expireIfNeeded(redisDb *db, robj *key) { /* Delete the key */ server.stat_expiredkeys++; server.dirty++; - return dbDelete(db,key); -} - -int deleteIfVolatile(redisDb *db, robj *key) { - if (getExpire(db,key) < 0) return 0; - - /* Delete the key */ - server.stat_expiredkeys++; - server.dirty++; + propagateExpire(db,key); return dbDelete(db,key); } diff --git a/src/redis.c b/src/redis.c index c8b1c781..27ade8b1 100644 --- a/src/redis.c +++ b/src/redis.c @@ -435,6 +435,48 @@ void updateDictResizePolicy(void) { /* ======================= Cron: called every 100 ms ======================== */ +/* Try to expire a few timed out keys. The algorithm used is adaptive and + * will use few CPU cycles if there are few expiring keys, otherwise + * it will get more aggressive to avoid that too much memory is used by + * keys that can be removed from the keyspace. */ +void activeExpireCycle(void) { + int j; + + for (j = 0; j < server.dbnum; j++) { + int expired; + redisDb *db = server.db+j; + + /* Continue to expire if at the end of the cycle more than 25% + * of the keys were expired. */ + do { + long num = dictSize(db->expires); + time_t now = time(NULL); + + expired = 0; + if (num > REDIS_EXPIRELOOKUPS_PER_CRON) + num = REDIS_EXPIRELOOKUPS_PER_CRON; + while (num--) { + dictEntry *de; + time_t t; + + if ((de = dictGetRandomKey(db->expires)) == NULL) break; + t = (time_t) dictGetEntryVal(de); + if (now > t) { + sds key = dictGetEntryKey(de); + robj *keyobj = createStringObject(key,sdslen(key)); + + propagateExpire(db,keyobj); + dbDelete(db,keyobj); + decrRefCount(keyobj); + expired++; + server.stat_expiredkeys++; + } + } + } while (expired > REDIS_EXPIRELOOKUPS_PER_CRON/4); + } +} + + int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientData) { int j, loops = server.cronloops++; REDIS_NOTUSED(eventLoop); @@ -533,41 +575,10 @@ int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientData) { } } - /* Try to expire a few timed out keys. The algorithm used is adaptive and - * will use few CPU cycles if there are few expiring keys, otherwise - * it will get more aggressive to avoid that too much memory is used by - * keys that can be removed from the keyspace. */ - for (j = 0; j < server.dbnum; j++) { - int expired; - redisDb *db = server.db+j; - - /* Continue to expire if at the end of the cycle more than 25% - * of the keys were expired. */ - do { - long num = dictSize(db->expires); - time_t now = time(NULL); - - expired = 0; - if (num > REDIS_EXPIRELOOKUPS_PER_CRON) - num = REDIS_EXPIRELOOKUPS_PER_CRON; - while (num--) { - dictEntry *de; - time_t t; - - if ((de = dictGetRandomKey(db->expires)) == NULL) break; - t = (time_t) dictGetEntryVal(de); - if (now > t) { - sds key = dictGetEntryKey(de); - robj *keyobj = createStringObject(key,sdslen(key)); - - dbDelete(db,keyobj); - decrRefCount(keyobj); - expired++; - server.stat_expiredkeys++; - } - } - } while (expired > REDIS_EXPIRELOOKUPS_PER_CRON/4); - } + /* Expire a few keys per cycle, only if this is a master. + * On slaves we wait for DEL operations synthesized by the master + * in order to guarantee a strict consistency. */ + if (server.masterhost == NULL) activeExpireCycle(); /* Swap a few keys on disk if we are over the memory limit and VM * is enbled. Try to free objects from the free list first. */ diff --git a/src/redis.h b/src/redis.h index fb051f8e..27520c19 100644 --- a/src/redis.h +++ b/src/redis.h @@ -752,8 +752,8 @@ void resetServerSaveParams(); /* db.c -- Keyspace access API */ int removeExpire(redisDb *db, robj *key); +void propagateExpire(redisDb *db, robj *key); int expireIfNeeded(redisDb *db, robj *key); -int deleteIfVolatile(redisDb *db, robj *key); time_t getExpire(redisDb *db, robj *key); int setExpire(redisDb *db, robj *key, time_t when); robj *lookupKey(redisDb *db, robj *key); diff --git a/src/t_string.c b/src/t_string.c index f55595c2..3b8a39bb 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -17,7 +17,6 @@ void setGenericCommand(redisClient *c, int nx, robj *key, robj *val, robj *expir } } - if (nx) deleteIfVolatile(c->db,key); retval = dbAdd(c->db,key,val); if (retval == REDIS_ERR) { if (!nx) { From c25a5d3b1062f3398a96a76ecd27c6f3a77a446e Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 2 Aug 2010 21:37:39 +0200 Subject: [PATCH 43/49] memory leak removed from expire propagation code --- src/db.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/db.c b/src/db.c index d8a5d0b2..6ac2b0d7 100644 --- a/src/db.c +++ b/src/db.c @@ -450,7 +450,8 @@ void propagateExpire(redisDb *db, robj *key) { if (listLength(server.slaves)) replicationFeedSlaves(server.slaves,db->id,argv,2); - decrRefCount(key); + decrRefCount(argv[0]); + decrRefCount(argv[1]); } int expireIfNeeded(redisDb *db, robj *key) { From 0cf5b7b57cde8b699198a866b04feca9f5394d03 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 3 Aug 2010 12:26:30 +0200 Subject: [PATCH 44/49] allow to set a new EXPIRE of an existing volatile key --- src/db.c | 22 ++++++++-------------- src/redis.h | 2 +- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/db.c b/src/db.c index 6ac2b0d7..5acda3d5 100644 --- a/src/db.c +++ b/src/db.c @@ -401,16 +401,13 @@ int removeExpire(redisDb *db, robj *key) { } } -int setExpire(redisDb *db, robj *key, time_t when) { +void setExpire(redisDb *db, robj *key, time_t when) { dictEntry *de; /* Reuse the sds from the main dict in the expire dict */ - redisAssert((de = dictFind(db->dict,key->ptr)) != NULL); - if (dictAdd(db->expires,dictGetEntryKey(de),(void*)when) == DICT_ERR) { - return 0; - } else { - return 1; - } + de = dictFind(db->dict,key->ptr); + redisAssert(de != NULL); + dictReplace(db->expires,dictGetEntryKey(de),(void*)when); } /* Return the expire time of the specified key, or -1 if no expire @@ -504,13 +501,10 @@ void expireGenericCommand(redisClient *c, robj *key, robj *param, long offset) { return; } else { time_t when = time(NULL)+seconds; - if (setExpire(c->db,key,when)) { - addReply(c,shared.cone); - touchWatchedKey(c->db,key); - server.dirty++; - } else { - addReply(c,shared.czero); - } + setExpire(c->db,key,when); + addReply(c,shared.cone); + touchWatchedKey(c->db,key); + server.dirty++; return; } } diff --git a/src/redis.h b/src/redis.h index 27520c19..c211cfb5 100644 --- a/src/redis.h +++ b/src/redis.h @@ -755,7 +755,7 @@ int removeExpire(redisDb *db, robj *key); void propagateExpire(redisDb *db, robj *key); int expireIfNeeded(redisDb *db, robj *key); time_t getExpire(redisDb *db, robj *key); -int setExpire(redisDb *db, robj *key, time_t when); +void setExpire(redisDb *db, robj *key, time_t when); robj *lookupKey(redisDb *db, robj *key); robj *lookupKeyRead(redisDb *db, robj *key); robj *lookupKeyWrite(redisDb *db, robj *key); From 2c572622fb99f32328de58f815953f17d4ad0e4d Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 3 Aug 2010 13:08:32 +0200 Subject: [PATCH 45/49] no longer passing tests due to the new write-on-volatile semantics modified/removed --- tests/unit/basic.tcl | 14 +++----------- tests/unit/expire.tcl | 9 +++++---- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/tests/unit/basic.tcl b/tests/unit/basic.tcl index f888cabc..a8f7feb0 100644 --- a/tests/unit/basic.tcl +++ b/tests/unit/basic.tcl @@ -148,12 +148,11 @@ start_server {tags {"basic"}} { r get novar2 } {foobared} - test {SETNX will overwrite EXPIREing key} { + test {SETNX against volatile key} { r set x 10 r expire x 10000 - r setnx x 20 - r get x - } {20} + list [r setnx x 20] [r get x] + } {0 10} test {EXISTS} { set res {} @@ -362,13 +361,6 @@ start_server {tags {"basic"}} { list [r msetnx x1 xxx y2 yyy] [r get x1] [r get y2] } {1 xxx yyy} - test {MSETNX should remove all the volatile keys even on failure} { - r mset x 1 y 2 z 3 - r expire y 10000 - r expire z 10000 - list [r msetnx x A y B z C] [r mget x y z] - } {0 {1 {} {}}} - test {STRLEN against non existing key} { r strlen notakey } {0} diff --git a/tests/unit/expire.tcl b/tests/unit/expire.tcl index b80975b6..5de907ab 100644 --- a/tests/unit/expire.tcl +++ b/tests/unit/expire.tcl @@ -1,12 +1,13 @@ start_server {tags {"expire"}} { - test {EXPIRE - don't set timeouts multiple times} { + test {EXPIRE - set timeouts multiple times} { r set x foobar set v1 [r expire x 5] set v2 [r ttl x] set v3 [r expire x 10] set v4 [r ttl x] + r expire x 4 list $v1 $v2 $v3 $v4 - } {1 5 0 5} + } {1 5 1 10} test {EXPIRE - It should be still possible to read 'x'} { r get x @@ -19,13 +20,13 @@ start_server {tags {"expire"}} { } {{} 0} } - test {EXPIRE - Delete on write policy} { + test {EXPIRE - write on expire should work} { r del x r lpush x foo r expire x 1000 r lpush x bar r lrange x 0 -1 - } {bar} + } {bar foo} test {EXPIREAT - Check for EXPIRE alike behavior} { r del x From 6146329f1f3381e8daef47463a6588b161f10596 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 3 Aug 2010 13:38:39 +0200 Subject: [PATCH 46/49] replication test with expires --- tests/integration/replication.tcl | 18 ++++++++++++++++++ tests/support/util.tcl | 9 ++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/integration/replication.tcl b/tests/integration/replication.tcl index 4b258825..6ca5a6dd 100644 --- a/tests/integration/replication.tcl +++ b/tests/integration/replication.tcl @@ -23,6 +23,24 @@ start_server {tags {"repl"}} { } assert_equal [r debug digest] [r -1 debug digest] } + + test {MASTER and SLAVE consistency with expire} { + createComplexDataset r 50000 useexpire + after 4000 ;# Make sure everything expired before taking the digest + if {[r debug digest] ne [r -1 debug digest]} { + set csv1 [csvdump r] + set csv2 [csvdump {r -1}] + set fd [open /tmp/repldump1.txt w] + puts -nonewline $fd $csv1 + close $fd + set fd [open /tmp/repldump2.txt w] + puts -nonewline $fd $csv2 + close $fd + puts "Master - Slave inconsistency" + puts "Run diff -u against /tmp/repldump*.txt for more info" + } + assert_equal [r debug digest] [r -1 debug digest] + } } } diff --git a/tests/support/util.tcl b/tests/support/util.tcl index b9c89aa8..95153111 100644 --- a/tests/support/util.tcl +++ b/tests/support/util.tcl @@ -140,12 +140,19 @@ proc findKeyWithType {r type} { return {} } -proc createComplexDataset {r ops} { +proc createComplexDataset {r ops {opt {}}} { for {set j 0} {$j < $ops} {incr j} { set k [randomKey] set k2 [randomKey] set f [randomValue] set v [randomValue] + + if {[lsearch -exact $opt useexpire] != -1} { + if {rand() < 0.1} { + {*}$r expire [randomKey] [randomInt 2] + } + } + randpath { set d [expr {rand()}] } { From a539d29ac559ffb80bfe6b3f045eddbd772fa1ba Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 3 Aug 2010 14:19:20 +0200 Subject: [PATCH 47/49] PERSIST command implemented --- src/db.c | 20 +++++++++++++++----- src/redis.c | 1 + src/redis.h | 1 + 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/db.c b/src/db.c index 5acda3d5..81e41430 100644 --- a/src/db.c +++ b/src/db.c @@ -394,11 +394,7 @@ int removeExpire(redisDb *db, robj *key) { /* An expire may only be removed if there is a corresponding entry in the * main dict. Otherwise, the key will never be freed. */ redisAssert(dictFind(db->dict,key->ptr) != NULL); - if (dictDelete(db->expires,key->ptr) == DICT_OK) { - return 1; - } else { - return 0; - } + return dictDelete(db->expires,key->ptr) == DICT_OK; } void setExpire(redisDb *db, robj *key, time_t when) { @@ -528,3 +524,17 @@ void ttlCommand(redisClient *c) { } addReplySds(c,sdscatprintf(sdsempty(),":%d\r\n",ttl)); } + +void persistCommand(redisClient *c) { + dictEntry *de; + + de = dictFind(c->db->dict,c->argv[1]->ptr); + if (de == NULL) { + addReply(c,shared.czero); + } else { + if (removeExpire(c->db,c->argv[1])) + addReply(c,shared.cone); + else + addReply(c,shared.czero); + } +} diff --git a/src/redis.c b/src/redis.c index 27ade8b1..1a581a92 100644 --- a/src/redis.c +++ b/src/redis.c @@ -170,6 +170,7 @@ struct redisCommand readonlyCommandTable[] = { {"info",infoCommand,1,REDIS_CMD_INLINE,NULL,0,0,0}, {"monitor",monitorCommand,1,REDIS_CMD_INLINE,NULL,0,0,0}, {"ttl",ttlCommand,2,REDIS_CMD_INLINE,NULL,1,1,1}, + {"persist",persistCommand,2,REDIS_CMD_INLINE,NULL,1,1,1}, {"slaveof",slaveofCommand,3,REDIS_CMD_INLINE,NULL,0,0,0}, {"debug",debugCommand,-2,REDIS_CMD_INLINE,NULL,0,0,0}, {"config",configCommand,-2,REDIS_CMD_BULK,NULL,0,0,0}, diff --git a/src/redis.h b/src/redis.h index c211cfb5..781fb209 100644 --- a/src/redis.h +++ b/src/redis.h @@ -838,6 +838,7 @@ void expireCommand(redisClient *c); void expireatCommand(redisClient *c); void getsetCommand(redisClient *c); void ttlCommand(redisClient *c); +void persistCommand(redisClient *c); void slaveofCommand(redisClient *c); void debugCommand(redisClient *c); void msetCommand(redisClient *c); From 1fb4e8def723ac836ba96e5369f22a0bf463578d Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 3 Aug 2010 14:25:22 +0200 Subject: [PATCH 48/49] PERSIST: a fix and some basic test --- src/db.c | 6 ++++-- tests/unit/expire.tcl | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/db.c b/src/db.c index 81e41430..0dec95b1 100644 --- a/src/db.c +++ b/src/db.c @@ -532,9 +532,11 @@ void persistCommand(redisClient *c) { if (de == NULL) { addReply(c,shared.czero); } else { - if (removeExpire(c->db,c->argv[1])) + if (removeExpire(c->db,c->argv[1])) { addReply(c,shared.cone); - else + server.dirty++; + } else { addReply(c,shared.czero); + } } } diff --git a/tests/unit/expire.tcl b/tests/unit/expire.tcl index 5de907ab..6f16ed58 100644 --- a/tests/unit/expire.tcl +++ b/tests/unit/expire.tcl @@ -60,4 +60,15 @@ start_server {tags {"expire"}} { catch {r setex z -10 foo} e set _ $e } {*invalid expire*} + + test {PERSIST can undo an EXPIRE} { + r set x foo + r expire x 50 + list [r ttl x] [r persist x] [r ttl x] [r get x] + } {50 1 -1 foo} + + test {PERSIST returns 0 against non existing or non volatile keys} { + r set x foo + list [r persist foo] [r persist nokeyatall] + } {0 0} } From cbce5171451eb53f1370aacc30decd74512347ac Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 5 Aug 2010 11:36:39 +0200 Subject: [PATCH 49/49] redis cli argument splitting is general and is now moved into the sds.c lib --- src/redis-cli.c | 70 ++------------------------------------------ src/sds.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ src/sds.h | 1 + 3 files changed, 81 insertions(+), 67 deletions(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index dac82862..b4a10890 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -366,79 +366,15 @@ static char **convertToSds(int count, char** args) { return sds; } -static char **splitArguments(char *line, int *argc) { - char *p = line; - char *current = NULL; - char **vector = NULL; - - *argc = 0; - while(1) { - /* skip blanks */ - while(*p && isspace(*p)) p++; - if (*p) { - /* get a token */ - int inq=0; /* set to 1 if we are in "quotes" */ - int done = 0; - - if (current == NULL) current = sdsempty(); - while(!done) { - if (inq) { - if (*p == '\\' && *(p+1)) { - char c; - - p++; - switch(*p) { - case 'n': c = '\n'; break; - case 'r': c = '\r'; break; - case 't': c = '\t'; break; - case 'b': c = '\b'; break; - case 'a': c = '\a'; break; - default: c = *p; break; - } - current = sdscatlen(current,&c,1); - } else if (*p == '"') { - done = 1; - } else { - current = sdscatlen(current,p,1); - } - } else { - switch(*p) { - case ' ': - case '\n': - case '\r': - case '\t': - case '\0': - done=1; - break; - case '"': - inq=1; - break; - default: - current = sdscatlen(current,p,1); - break; - } - } - if (*p) p++; - } - /* add the token to the vector */ - vector = zrealloc(vector,((*argc)+1)*sizeof(char*)); - vector[*argc] = current; - (*argc)++; - current = NULL; - } else { - return vector; - } - } -} - #define LINE_BUFLEN 4096 static void repl() { int argc, j; - char *line, **argv; + char *line; + sds *argv; while((line = linenoise("redis> ")) != NULL) { if (line[0] != '\0') { - argv = splitArguments(line,&argc); + argv = sdssplitargs(line,&argc); linenoiseHistoryAdd(line); if (config.historyfile) linenoiseHistorySave(config.historyfile); if (argc > 0) { diff --git a/src/sds.c b/src/sds.c index 5e67f044..4878f8a6 100644 --- a/src/sds.c +++ b/src/sds.c @@ -382,3 +382,80 @@ sds sdscatrepr(sds s, char *p, size_t len) { } return sdscatlen(s,"\"",1); } + +/* Split a line into arguments, where every argument can be in the + * following programming-language REPL-alike form: + * + * foo bar "newline are supported\n" and "\xff\x00otherstuff" + * + * The number of arguments is stored into *argc, and an array + * of sds is returned. The caller should sdsfree() all the returned + * strings and finally zfree() the array itself. + * + * Note that sdscatrepr() is able to convert back a string into + * a quoted string in the same format sdssplitargs() is able to parse. + */ +sds *sdssplitargs(char *line, int *argc) { + char *p = line; + char *current = NULL; + char **vector = NULL; + + *argc = 0; + while(1) { + /* skip blanks */ + while(*p && isspace(*p)) p++; + if (*p) { + /* get a token */ + int inq=0; /* set to 1 if we are in "quotes" */ + int done = 0; + + if (current == NULL) current = sdsempty(); + while(!done) { + if (inq) { + if (*p == '\\' && *(p+1)) { + char c; + + p++; + switch(*p) { + case 'n': c = '\n'; break; + case 'r': c = '\r'; break; + case 't': c = '\t'; break; + case 'b': c = '\b'; break; + case 'a': c = '\a'; break; + default: c = *p; break; + } + current = sdscatlen(current,&c,1); + } else if (*p == '"') { + done = 1; + } else { + current = sdscatlen(current,p,1); + } + } else { + switch(*p) { + case ' ': + case '\n': + case '\r': + case '\t': + case '\0': + done=1; + break; + case '"': + inq=1; + break; + default: + current = sdscatlen(current,p,1); + break; + } + } + if (*p) p++; + } + /* add the token to the vector */ + vector = zrealloc(vector,((*argc)+1)*sizeof(char*)); + vector[*argc] = current; + (*argc)++; + current = NULL; + } else { + return vector; + } + } +} diff --git a/src/sds.h b/src/sds.h index ef3a418f..a0e224f5 100644 --- a/src/sds.h +++ b/src/sds.h @@ -70,5 +70,6 @@ void sdstolower(sds s); void sdstoupper(sds s); sds sdsfromlonglong(long long value); sds sdscatrepr(sds s, char *p, size_t len); +sds *sdssplitargs(char *line, int *argc); #endif