Redis 2.6.9

Sometimes it is much simpler to debug complex Redis installations if it
is possible to assign clients a name that is displayed in the CLIENT
LIST output.

This is the case, for example, for "leaked" connections. The ability to
provide a name to the client makes it quite trivial to understand what
is the part of the code implementing the client not releasing the
resources appropriately.

Behavior:

    CLIENT SETNAME: set a name for the client, or remove the current
                    name if an empty name is set.
    CLIENT GETNAME: get the current name, or a nil.
    CLIENT LIST: now displays the client name if any.

Thanks to Mark Gravell for pushing this idea forward.

00-RELEASENOTES

@@ -14,6 +14,20 @@ HIGH:     There is a critical bug that may affect a subset of users. Upgrade!
 CRITICAL: There is a critical bug affecting MOST USERS. Upgrade ASAP.
 --------------------------------------------------------------------------------
 
+--[ Redis 2.6.9 ]
+
+UPGRADE URGENCY: MODERATE if you use replication.
+
+* [BUGFIX]   Changing master at runtime (SLAVEOF command) in presence of
+             network problems, or in very rapid succession, could result
+             in non-critical problems (GitHub Issue #828).
+* [IMPROVED] CLINGET GETNAME and SETNAME to set and query connection names
+             reported by CLIENT LIST. Very useful for debugging of
+             problems.
+* [IMPROVED] redis-cli is now able to transfer an RDB file from a remote
+             server to a local file using the --rdb <filename> command
+             line option.
+
 --[ Redis 2.6.8 ]
 
 UPGRADE URGENCY: MODERATE if you use Lua scripting. Otherwise LOW.

src/Makefile.dep

@@ -1,6 +1,7 @@
 adlist.o: adlist.c adlist.h zmalloc.h
 ae.o: ae.c ae.h zmalloc.h config.h ae_kqueue.c
 ae_epoll.o: ae_epoll.c
+ae_evport.o: ae_evport.c
 ae_kqueue.o: ae_kqueue.c
 ae_select.o: ae_select.c
 anet.o: anet.c fmacros.h anet.h
@@ -10,6 +11,9 @@ aof.o: aof.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
 bio.o: bio.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
   ../deps/lua/src/luaconf.h ae.h sds.h dict.h adlist.h zmalloc.h anet.h \
   ziplist.h intset.h version.h util.h rdb.h rio.h bio.h
+bitops.o: bitops.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
+  ../deps/lua/src/luaconf.h ae.h sds.h dict.h adlist.h zmalloc.h anet.h \
+  ziplist.h intset.h version.h util.h rdb.h rio.h
 config.o: config.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
   ../deps/lua/src/luaconf.h ae.h sds.h dict.h adlist.h zmalloc.h anet.h \
   ziplist.h intset.h version.h util.h rdb.h rio.h
@@ -22,7 +26,7 @@ debug.o: debug.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
   ziplist.h intset.h version.h util.h rdb.h rio.h sha1.h
 dict.o: dict.c fmacros.h dict.h zmalloc.h
 endianconv.o: endianconv.c
-intset.o: intset.c intset.h zmalloc.h endianconv.h
+intset.o: intset.c intset.h zmalloc.h endianconv.h config.h
 lzf_c.o: lzf_c.c lzfP.h
 lzf_d.o: lzf_d.c lzfP.h
 memtest.o: memtest.c
@@ -53,7 +57,7 @@ redis-benchmark.o: redis-benchmark.c fmacros.h ae.h \
 redis-check-aof.o: redis-check-aof.c fmacros.h config.h
 redis-check-dump.o: redis-check-dump.c lzf.h
 redis-cli.o: redis-cli.c fmacros.h version.h ../deps/hiredis/hiredis.h \
-  sds.h zmalloc.h ../deps/linenoise/linenoise.h help.h
+  sds.h zmalloc.h ../deps/linenoise/linenoise.h help.h anet.h ae.h
 redis.o: redis.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
   ../deps/lua/src/luaconf.h ae.h sds.h dict.h adlist.h zmalloc.h anet.h \
   ziplist.h intset.h version.h util.h rdb.h rio.h slowlog.h bio.h \
@@ -70,6 +74,11 @@ scripting.o: scripting.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
   ../deps/lua/src/lauxlib.h ../deps/lua/src/lua.h \
   ../deps/lua/src/lualib.h
 sds.o: sds.c sds.h zmalloc.h
+sentinel.o: sentinel.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
+  ../deps/lua/src/luaconf.h ae.h sds.h dict.h adlist.h zmalloc.h anet.h \
+  ziplist.h intset.h version.h util.h rdb.h rio.h \
+  ../deps/hiredis/hiredis.h ../deps/hiredis/async.h \
+  ../deps/hiredis/hiredis.h
 sha1.o: sha1.c sha1.h config.h
 slowlog.o: slowlog.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
   ../deps/lua/src/luaconf.h ae.h sds.h dict.h adlist.h zmalloc.h anet.h \
@@ -96,6 +105,6 @@ t_zset.o: t_zset.c redis.h fmacros.h config.h ../deps/lua/src/lua.h \
   ../deps/lua/src/luaconf.h ae.h sds.h dict.h adlist.h zmalloc.h anet.h \
   ziplist.h intset.h version.h util.h rdb.h rio.h
 util.o: util.c fmacros.h util.h
-ziplist.o: ziplist.c zmalloc.h util.h ziplist.h endianconv.h
-zipmap.o: zipmap.c zmalloc.h endianconv.h
+ziplist.o: ziplist.c zmalloc.h util.h ziplist.h endianconv.h config.h
+zipmap.o: zipmap.c zmalloc.h endianconv.h config.h
 zmalloc.o: zmalloc.c config.h zmalloc.h

src/aof.c

@@ -442,6 +442,7 @@ struct redisClient *createFakeClient(void) {
 
     selectDb(c,0);
     c->fd = -1;
+    c->name = NULL;
     c->querybuf = sdsempty();
     c->querybuf_peak = 0;
     c->argc = 0;

src/networking.c

@@ -70,6 +70,7 @@ redisClient *createClient(int fd) {
 
     selectDb(c,0);
     c->fd = fd;
+    c->name = NULL;
     c->bufpos = 0;
     c->querybuf = sdsempty();
     c->querybuf_peak = 0;
@@ -668,6 +669,7 @@ void freeClient(redisClient *c) {
     }
 
     /* Release memory */
+    if (c->name) decrRefCount(c->name);
     zfree(c->argv);
     freeClientMultiState(c);
     zfree(c);
@@ -1123,9 +1125,11 @@ sds getClientInfoString(redisClient *client) {
     if (emask & AE_WRITABLE) *p++ = 'w';
     *p = '\0';
     return sdscatprintf(sdsempty(),
-        "addr=%s:%d fd=%d age=%ld idle=%ld flags=%s db=%d sub=%d psub=%d multi=%d qbuf=%lu qbuf-free=%lu obl=%lu oll=%lu omem=%lu events=%s cmd=%s",
+        "addr=%s:%d fd=%d name=%s age=%ld idle=%ld flags=%s db=%d sub=%d psub=%d multi=%d qbuf=%lu qbuf-free=%lu obl=%lu oll=%lu omem=%lu events=%s cmd=%s",
         (client->flags & REDIS_UNIX_SOCKET) ? server.unixsocket : ip,
-        port,client->fd,
+        port,
+        client->fd,
+        client->name ? (char*)client->name->ptr : "",
         (long)(server.unixtime - client->ctime),
         (long)(server.unixtime - client->lastinteraction),
         flags,
@@ -1190,6 +1194,39 @@ void clientCommand(redisClient *c) {
             }
         }
         addReplyError(c,"No such client");
+    } else if (!strcasecmp(c->argv[1]->ptr,"setname") && c->argc == 3) {
+        int j, len = sdslen(c->argv[2]->ptr);
+        char *p = c->argv[2]->ptr;
+
+        /* Setting the client name to an empty string actually removes
+         * the current name. */
+        if (len == 0) {
+            if (c->name) decrRefCount(c->name);
+            c->name = NULL;
+            addReply(c,shared.ok);
+            return;
+        }
+
+        /* Otherwise check if the charset is ok. We need to do this otherwise
+         * CLIENT LIST format will break. You should always be able to
+         * split by space to get the different fields. */
+        for (j = 0; j < len; j++) {
+            if (p[j] < '!' || p[j] > '~') { /* ASCII is assumed. */
+                addReplyError(c,
+                    "Client names cannot contain spaces, "
+                    "newlines or special characters.");
+                return;
+            }
+        }
+        if (c->name) decrRefCount(c->name);
+        c->name = c->argv[2];
+        incrRefCount(c->name);
+        addReply(c,shared.ok);
+    } else if (!strcasecmp(c->argv[1]->ptr,"getname") && c->argc == 2) {
+        if (c->name)
+            addReplyBulk(c,c->name);
+        else
+            addReply(c,shared.nullbulk);
     } else {
         addReplyError(c, "Syntax error, try CLIENT (LIST | KILL ip:port)");
     }

src/redis-cli.c

@@ -41,6 +41,7 @@
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <assert.h>
+#include <fcntl.h>
 
 #include "hiredis.h"
 #include "sds.h"
@@ -73,6 +74,8 @@ static struct config {
     int cluster_reissue_command;
     int slave_mode;
     int pipe_mode;
+    int getrdb_mode;
+    char *rdb_filename;
     int bigkeys;
     int stdinarg; /* get last arg from stdin. (-x option) */
     char *auth;
@@ -660,6 +663,9 @@ static int parseOptions(int argc, char **argv) {
             config.latency_mode = 1;
         } else if (!strcmp(argv[i],"--slave")) {
             config.slave_mode = 1;
+        } else if (!strcmp(argv[i],"--rdb") && !lastarg) {
+            config.getrdb_mode = 1;
+            config.rdb_filename = argv[++i];
         } else if (!strcmp(argv[i],"--pipe")) {
             config.pipe_mode = 1;
         } else if (!strcmp(argv[i],"--bigkeys")) {
@@ -720,6 +726,7 @@ static void usage() {
 "  --raw            Use raw formatting for replies (default when STDOUT is not a tty)\n"
 "  --latency        Enter a special mode continuously sampling latency\n"
 "  --slave          Simulate a slave showing commands received from the master\n"
+"  --rdb <filename> Transfer an RDB dump from remote server to local file.\n"
 "  --pipe           Transfer raw Redis protocol from stdin to server\n"
 "  --bigkeys        Sample Redis keys looking for big keys\n"
 "  --eval <file>    Send an EVAL command using the Lua script at <file>\n"
@@ -927,15 +934,15 @@ static void latencyMode(void) {
     }
 }
 
-static void slaveMode(void) {
+/* Sends SYNC and reads the number of bytes in the payload. Used both by
+ * slaveMode() and getRDB(). */
+unsigned long long sendSync(int fd) {
     /* To start we need to send the SYNC command and return the payload.
      * The hiredis client lib does not understand this part of the protocol
      * and we don't want to mess with its buffers, so everything is performed
      * using direct low-level I/O. */
-    int fd = context->fd;
-    char buf[1024], *p;
+    char buf[4096], *p;
     ssize_t nread;
-    unsigned long long payload;
 
     /* Send the SYNC command. */
     if (write(fd,"SYNC\r\n",6) != 6) {
@@ -955,12 +962,25 @@ static void slaveMode(void) {
         p++;
     }
     *p = '\0';
-    payload = strtoull(buf+1,NULL,10);
-    fprintf(stderr,"SYNC with master, discarding %lld bytes of bulk tranfer...\n",
-            payload);
+    if (buf[0] == '-') {
+        printf("SYNC with master failed: %s\n", buf);
+        exit(1);
+    }
+    return strtoull(buf+1,NULL,10);
+}
+
+static void slaveMode(void) {
+    int fd = context->fd;
+    unsigned long long payload = sendSync(fd);
+    char buf[1024];
+
+    fprintf(stderr,"SYNC with master, discarding %llu "
+                   "bytes of bulk tranfer...\n", payload);
 
     /* Discard the payload. */
     while(payload) {
+        ssize_t nread;
+
         nread = read(fd,buf,(payload > sizeof(buf)) ? sizeof(buf) : payload);
         if (nread <= 0) {
             fprintf(stderr,"Error reading RDB payload while SYNCing\n");
@@ -970,11 +990,56 @@ static void slaveMode(void) {
     }
     fprintf(stderr,"SYNC done. Logging commands from master.\n");
 
-    /* Now we can use the hiredis to read the incoming protocol. */
+    /* Now we can use hiredis to read the incoming protocol. */
     config.output = OUTPUT_CSV;
     while (cliReadReply(0) == REDIS_OK);
 }
 
+/* This function implements --rdb, so it uses the replication protocol in order
+ * to fetch the RDB file from a remote server. */
+static void getRDB(void) {
+    int s = context->fd;
+    int fd;
+    unsigned long long payload = sendSync(s);
+    char buf[4096];
+
+    fprintf(stderr,"SYNC sent to master, writing %llu bytes to '%s'\n",
+        payload, config.rdb_filename);
+
+    /* Write to file. */
+    if (!strcmp(config.rdb_filename,"-")) {
+        fd = STDOUT_FILENO;
+    } else {
+        fd = open(config.rdb_filename, O_CREAT|O_WRONLY, 0644);
+        if (fd == -1) {
+            fprintf(stderr, "Error opening '%s': %s\n", config.rdb_filename,
+                strerror(errno));
+            exit(1);
+        }
+    }
+
+    while(payload) {
+        ssize_t nread, nwritten;
+        
+        nread = read(s,buf,(payload > sizeof(buf)) ? sizeof(buf) : payload);
+        if (nread <= 0) {
+            fprintf(stderr,"I/O Error reading RDB payload from socket\n");
+            exit(1);
+        }
+        nwritten = write(fd, buf, nread);
+        if (nwritten != nread) {
+            fprintf(stderr,"Error writing data to file: %s\n",
+                strerror(errno));
+            exit(1);
+        }
+        payload -= nread;
+    }
+    close(s); /* Close the file descriptor ASAP as fsync() may take time. */
+    fsync(fd);
+    fprintf(stderr,"Transfer finished with success.\n");
+    exit(0);
+}
+
 static void pipeMode(void) {
     int fd = context->fd;
     long long errors = 0, replies = 0, obuf_len = 0, obuf_pos = 0;
@@ -1203,6 +1268,8 @@ int main(int argc, char **argv) {
     config.latency_mode = 0;
     config.cluster_mode = 0;
     config.slave_mode = 0;
+    config.getrdb_mode = 0;
+    config.rdb_filename = NULL;
     config.pipe_mode = 0;
     config.bigkeys = 0;
     config.stdinarg = 0;
@@ -1231,6 +1298,12 @@ int main(int argc, char **argv) {
         slaveMode();
     }
 
+    /* Get RDB mode. */
+    if (config.getrdb_mode) {
+        cliConnect(0);
+        getRDB();
+    }
+
     /* Pipe mode */
     if (config.pipe_mode) {
         if (cliConnect(0) == REDIS_ERR) exit(1);

src/redis.h

@@ -380,6 +380,7 @@ typedef struct redisClient {
     int fd;
     redisDb *db;
     int dictid;
+    robj *name;             /* As set by CLIENT SETNAME */
     sds querybuf;
     size_t querybuf_peak;   /* Recent (100ms or more) peak of querybuf size */
     int argc;

src/replication.c

@@ -695,6 +695,27 @@ void undoConnectWithMaster(void) {
     server.repl_state = REDIS_REPL_CONNECT;
 }
 
+/* This function aborts a non blocking replication attempt if there is one
+ * in progress, by canceling the non-blocking connect attempt or
+ * the initial bulk transfer.
+ *
+ * If there was a replication handshake in progress 1 is returned and
+ * the replication state (server.repl_state) set to REDIS_REPL_CONNECT.
+ *
+ * Otherwise zero is returned and no operation is perforemd at all. */
+int cancelReplicationHandshake(void) {
+    if (server.repl_state == REDIS_REPL_TRANSFER) {
+        replicationAbortSyncTransfer();
+    } else if (server.repl_state == REDIS_REPL_CONNECTING ||
+             server.repl_state == REDIS_REPL_RECEIVE_PONG)
+    {
+        undoConnectWithMaster();
+    } else {
+        return 0;
+    }
+    return 1;
+}
+
 void slaveofCommand(redisClient *c) {
     if (!strcasecmp(c->argv[1]->ptr,"no") &&
         !strcasecmp(c->argv[2]->ptr,"one")) {
@@ -702,11 +723,7 @@ void slaveofCommand(redisClient *c) {
             sdsfree(server.masterhost);
             server.masterhost = NULL;
             if (server.master) freeClient(server.master);
-            if (server.repl_state == REDIS_REPL_TRANSFER)
-                replicationAbortSyncTransfer();
-            else if (server.repl_state == REDIS_REPL_CONNECTING ||
-                     server.repl_state == REDIS_REPL_RECEIVE_PONG)
-                undoConnectWithMaster();
+            cancelReplicationHandshake();
             server.repl_state = REDIS_REPL_NONE;
             redisLog(REDIS_NOTICE,"MASTER MODE enabled (user request)");
         }
@@ -730,8 +747,7 @@ void slaveofCommand(redisClient *c) {
         server.masterport = port;
         if (server.master) freeClient(server.master);
         disconnectSlaves(); /* Force our slaves to resync with us as well. */
-        if (server.repl_state == REDIS_REPL_TRANSFER)
-            replicationAbortSyncTransfer();
+        cancelReplicationHandshake();
         server.repl_state = REDIS_REPL_CONNECT;
         redisLog(REDIS_NOTICE,"SLAVE OF %s:%d enabled (user request)",
             server.masterhost, server.masterport);

src/version.h

@@ -1 +1 @@
-#define REDIS_VERSION "2.6.8"
+#define REDIS_VERSION "2.6.9"

tests/unit/introspection.tcl

@@ -19,4 +19,37 @@ start_server {tags {"introspection"}} {
         assert_match {*eval*} [$rd read]
         assert_match {*lua*"set"*"foo"*"bar"*} [$rd read]
     }
+
+    test {CLIENT GETNAME should return NIL if name is not assigned} {
+        r client getname
+    } {}
+
+    test {CLIENT LIST shows empty fields for unassigned names} {
+        r client list
+    } {*name= *}
+    
+    test {CLIENT SETNAME does not accept spaces} {
+        catch {r client setname "foo bar"} e
+        set e
+    } {ERR*}
+
+    test {CLIENT SETNAME can assign a name to this connection} {
+        assert_equal [r client setname myname] {OK}
+        r client list
+    } {*name=myname*}
+
+    test {CLIENT SETNAME can change the name of an existing connection} {
+        assert_equal [r client setname someothername] {OK}
+        r client list
+    } {*name=someothername*}
+
+    test {After CLIENT SETNAME, connection can still be closed} {
+        set rd [redis_deferring_client]
+        $rd client setname foobar
+        assert_equal [$rd read] "OK"
+        assert_match {*foobar*} [r client list]
+        $rd close
+        # Now the client should no longer be listed
+        string match {*foobar*} [r client list]
+    } {0}
 }