More complete release notes for 2.5.9

Compare integers in ziplist regardless of encoding

00-RELEASENOTES

@@ -21,17 +21,45 @@ but it is better to keep them in mind:
 * INFO output is a bit different now, and contains empty lines and comments
   starting with '#'. All the major clients should be already fixed to work
   with the new INFO format.
+* Slaves are only read-only by default (but you can change this easily
+  setting the "slave-read-only" configuration option to "no" editing your
+  redis.conf or using CONFIG SET.
 
 Also the following redis.conf and CONFIG GET / SET parameters changed name:
 
     * hash-max-zipmap-entries, now replaced by hash-max-ziplist-entries
     * hash-max-zipmap-value, now replaced by hash-max-ziplist-value
-    * glueoutputbuf was no completely removed as it does not make sense
+    * glueoutputbuf option was now completely removed (was deprecated)
 
 ---------
 CHANGELOG
 ---------
 
+What's new in Redis 2.5.9 (aka 2.6 Release Candidate 3)
+=======================================================
+
+UPGRADE URGENCY: critical, upgrade ASAP.
+
+* [BUGFIX] Fix for issue #500 (https://github.com/antirez/redis/pull/500).
+           Redis 2.6-RC1 and RC2 may corrupt ziplist-encoded sorted sets
+           produced by Redis 2.4.x.
+* [BUGFIX] Fixed several bugs in init.d script.
+* [BUGFIX] syncio.c functions modified for speed and correctness. On osx
+           (and possibly other BSD-based systems) the slave would block on
+           replication to send the SYNC command when the master was not
+           available. This is fixed now, but was not affecting Linux installs.
+* Now when slave-serve-stake-data is set to yes and the master is down, instead
+  of reporting a generic error Redis replies with -MASTERDOWN.
+
+What's new in Redis 2.5.8 (aka 2.6 Release Candidate 2)
+=======================================================
+
+UPGRADE URGENCY: high for all the users of the KEYS command, otherwise low.
+
+* [BUGFIX] Fix for KEYS command: if the DB contains keys with expires the KEYS
+           command may return the wrong output, having duplicated or missing
+           keys. See issue #487 and #488 on github for details.
+
 What's new in Redis 2.5.7 (aka 2.6 Release Candidate 1)
 =======================================================
 
@@ -53,6 +81,7 @@ An overview of new features and changes in Redis 2.6.x
   precision (PEXPIRE, PTTL, ...).
 * Better memory usage for "small" lists, ziplists and hashes when fields or
   values contain small integers.
+* Read only slaves.
 * Clients max output buffer soft and hard limits. You can specifiy different
   limits for different classes of clients (normal,pubsub,slave).
 * AOF is now able to rewrite aggregate data types using variadic commands,

src/db.c

@@ -255,7 +255,7 @@ void keysCommand(redisClient *c) {
     unsigned long numkeys = 0;
     void *replylen = addDeferredMultiBulkLength(c);
 
-    di = dictGetIterator(c->db->dict);
+    di = dictGetSafeIterator(c->db->dict);
     allkeys = (pattern[0] == '*' && pattern[1] == '\0');
     while((de = dictNext(di)) != NULL) {
         sds key = dictGetKey(de);

src/redis.c

@@ -970,6 +970,8 @@ void createSharedObjects(void) {
         "-LOADING Redis is loading the dataset in memory\r\n"));
     shared.slowscripterr = createObject(REDIS_STRING,sdsnew(
         "-BUSY Redis is busy running a script. You can only call SCRIPT KILL or SHUTDOWN NOSAVE.\r\n"));
+    shared.masterdownerr = createObject(REDIS_STRING,sdsnew(
+        "-MASTERDOWN Link with MASTER is down and slave-serve-stale-data is set to 'no'.\r\n"));
     shared.bgsaveerr = createObject(REDIS_STRING,sdsnew(
         "-MISCONF Redis is configured to save RDB snapshots, but is currently not able to persist on disk. Commands that may modify the data set are disabled. Please check Redis logs for details about the error.\r\n"));
     shared.roslaveerr = createObject(REDIS_STRING,sdsnew(
@@ -1541,8 +1543,7 @@ int processCommand(redisClient *c) {
         server.repl_serve_stale_data == 0 &&
         c->cmd->proc != infoCommand && c->cmd->proc != slaveofCommand)
     {
-        addReplyError(c,
-            "link with MASTER is down and slave-serve-stale-data is set to no");
+        addReply(c, shared.masterdownerr);
         return REDIS_OK;
     }
 

src/redis.h

@@ -366,7 +366,8 @@ struct sharedObjectsStruct {
     *colon, *nullbulk, *nullmultibulk, *queued,
     *emptymultibulk, *wrongtypeerr, *nokeyerr, *syntaxerr, *sameobjecterr,
     *outofrangeerr, *noscripterr, *loadingerr, *slowscripterr, *bgsaveerr,
-    *roslaveerr, *oomerr, *plus, *messagebulk, *pmessagebulk, *subscribebulk,
+    *masterdownerr, *roslaveerr,
+    *oomerr, *plus, *messagebulk, *pmessagebulk, *subscribebulk,
     *unsubscribebulk, *psubscribebulk, *punsubscribebulk, *del, *rpop, *lpop,
     *select[REDIS_SHARED_SELECT_CMDS],
     *integers[REDIS_SHARED_INTEGERS],

src/replication.c

@@ -423,7 +423,7 @@ void syncWithMaster(aeEventLoop *el, int fd, void *privdata, int mask) {
     }
 
     /* Issue the SYNC command */
-    if (syncWrite(fd,"SYNC \r\n",7,server.repl_syncio_timeout*1000) == -1) {
+    if (syncWrite(fd,"SYNC\r\n",6,server.repl_syncio_timeout*1000) == -1) {
         redisLog(REDIS_WARNING,"I/O error writing to MASTER: %s",
             strerror(errno));
         goto error;

src/syncio.c

@@ -42,10 +42,10 @@
 
 #define REDIS_SYNCIO_RESOLUTION 10 /* Resolution in milliseconds */
 
-/* Write the specified payload to 'fd'. If writing the whole payload will be done
- * within 'timeout' milliseconds the operation succeeds and 'size' is returned.
- * Otherwise the operation fails, -1 is returned, and an unspecified partial write
- * could be performed against the file descriptor. */
+/* Write the specified payload to 'fd'. If writing the whole payload will be
+ * done within 'timeout' milliseconds the operation succeeds and 'size' is
+ * returned. Otherwise the operation fails, -1 is returned, and an unspecified
+ * partial write could be performed against the file descriptor. */
 ssize_t syncWrite(int fd, char *ptr, ssize_t size, long long timeout) {
     ssize_t nwritten, ret = size;
     long long start = mstime();
@@ -56,13 +56,19 @@ ssize_t syncWrite(int fd, char *ptr, ssize_t size, long long timeout) {
                           remaining : REDIS_SYNCIO_RESOLUTION;
         long long elapsed;
 
-        if (aeWait(fd,AE_WRITABLE,wait) & AE_WRITABLE) {
-            nwritten = write(fd,ptr,size);
-            if (nwritten == -1) return -1;
+        /* Optimistically try to write before checking if the file descriptor
+         * is actually writable. At worst we get EAGAIN. */
+        nwritten = write(fd,ptr,size);
+        if (nwritten == -1) {
+            if (errno != EAGAIN) return -1;
+        } else {
             ptr += nwritten;
             size -= nwritten;
-            if (size == 0) return ret;
         }
+        if (size == 0) return ret;
+
+        /* Wait */
+        aeWait(fd,AE_WRITABLE,wait);
         elapsed = mstime() - start;
         if (elapsed >= timeout) {
             errno = ETIMEDOUT;
@@ -72,8 +78,8 @@ ssize_t syncWrite(int fd, char *ptr, ssize_t size, long long timeout) {
     }
 }
 
-/* Read the specified amount of bytes from 'fd'. If all the bytes are read within
- * 'timeout' milliseconds the operation succeed and 'size' is returned.
+/* Read the specified amount of bytes from 'fd'. If all the bytes are read
+ * within 'timeout' milliseconds the operation succeed and 'size' is returned.
  * Otherwise the operation fails, -1 is returned, and an unspecified amount of
  * data could be read from the file descriptor. */
 ssize_t syncRead(int fd, char *ptr, ssize_t size, long long timeout) {
@@ -81,19 +87,27 @@ ssize_t syncRead(int fd, char *ptr, ssize_t size, long long timeout) {
     long long start = mstime();
     long long remaining = timeout;
 
+    if (size == 0) return 0;
     while(1) {
         long long wait = (remaining > REDIS_SYNCIO_RESOLUTION) ?
                           remaining : REDIS_SYNCIO_RESOLUTION;
         long long elapsed;
 
-        if (aeWait(fd,AE_READABLE,wait) & AE_READABLE) {
-            nread = read(fd,ptr,size);
-            if (nread <= 0) return -1;
+        /* Optimistically try to read before checking if the file descriptor
+         * is actually readable. At worst we get EAGAIN. */
+        nread = read(fd,ptr,size);
+        if (nread == 0) return -1; /* short read. */
+        if (nread == -1) {
+            if (errno != EAGAIN) return -1;
+        } else {
             ptr += nread;
             size -= nread;
             totread += nread;
-            if (size == 0) return totread;
         }
+        if (size == 0) return totread;
+
+        /* Wait */
+        aeWait(fd,AE_READABLE,wait);
         elapsed = mstime() - start;
         if (elapsed >= timeout) {
             errno = ETIMEDOUT;

src/version.h

@@ -1 +1 @@
-#define REDIS_VERSION "2.5.7"
+#define REDIS_VERSION "2.5.9"

src/ziplist.c

@@ -773,12 +773,11 @@ unsigned int ziplistCompare(unsigned char *p, unsigned char *sstr, unsigned int
             return 0;
         }
     } else {
-        /* Try to compare encoded values */
+        /* Try to compare encoded values. Don't compare encoding because
+         * different implementations may encoded integers differently. */
         if (zipTryEncoding(sstr,slen,&sval,&sencoding)) {
-            if (entry.encoding == sencoding) {
-                zval = zipLoadInteger(p+entry.headersize,entry.encoding);
-                return zval == sval;
-            }
+          zval = zipLoadInteger(p+entry.headersize,entry.encoding);
+          return zval == sval;
         }
     }
     return 0;

tests/integration/replication.tcl

@@ -2,9 +2,13 @@ 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}
+            wait_for_condition 50 100 {
+                [s -1 role] eq {slave} &&
+                [string match {*master_link_status:up*} [r -1 info replication]]
+            } else {
+                fail "Can't turn the instance into a slave"
+            }
+        }
 
         test {BRPOPLPUSH replication, when blocking against empty list} {
             set rd [redis_deferring_client]
@@ -82,12 +86,11 @@ start_server {tags {"repl"}} {
     set master_host [srv 0 host]
     set master_port [srv 0 port]
     set slaves {}
-    set load_handle0 [start_write_load $master_host $master_port 20]
-    set load_handle1 [start_write_load $master_host $master_port 20]
+    set load_handle0 [start_write_load $master_host $master_port 3]
+    set load_handle1 [start_write_load $master_host $master_port 5]
     set load_handle2 [start_write_load $master_host $master_port 20]
-    set load_handle3 [start_write_load $master_host $master_port 20]
-    set load_handle4 [start_write_load $master_host $master_port 20]
-    after 2000
+    set load_handle3 [start_write_load $master_host $master_port 8]
+    set load_handle4 [start_write_load $master_host $master_port 4]
     start_server {} {
         lappend slaves [srv 0 client]
         start_server {} {
@@ -95,6 +98,7 @@ start_server {tags {"repl"}} {
             start_server {} {
                 lappend slaves [srv 0 client]
                 test "Connect multiple slaves at the same time (issue #141)" {
+                    # Send SALVEOF commands to slaves
                     [lindex $slaves 0] slaveof $master_host $master_port
                     [lindex $slaves 1] slaveof $master_host $master_port
                     [lindex $slaves 2] slaveof $master_host $master_port
@@ -113,16 +117,33 @@ start_server {tags {"repl"}} {
                     if {$retry == 0} {
                         error "assertion:Slaves not correctly synchronized"
                     }
+
+                    # Stop the write load
                     stop_write_load $load_handle0
                     stop_write_load $load_handle1
                     stop_write_load $load_handle2
                     stop_write_load $load_handle3
                     stop_write_load $load_handle4
-                    set retry 10
-                    while {$retry && ([$master debug digest] ne [[lindex $slaves 0] debug digest])} {
-                        after 1000
-                        incr retry -1
+
+                    # Wait that slaves exit the "loading" state
+                    wait_for_condition 500 100 {
+                        ![string match {*loading:1*} [[lindex $slaves 0] info]] &&
+                        ![string match {*loading:1*} [[lindex $slaves 1] info]] &&
+                        ![string match {*loading:1*} [[lindex $slaves 2] info]]
+                    } else {
+                        fail "Slaves still loading data after too much time"
                     }
+
+                    # Make sure that slaves and master have same number of keys
+                    wait_for_condition 500 100 {
+                        [$master dbsize] == [[lindex $slaves 0] dbsize] &&
+                        [$master dbsize] == [[lindex $slaves 1] dbsize] &&
+                        [$master dbsize] == [[lindex $slaves 2] dbsize]
+                    } else {
+                        fail "Different number of keys between masted and slave after too long time."
+                    }
+
+                    # Check digests
                     set digest [$master debug digest]
                     set digest0 [[lindex $slaves 0] debug digest]
                     set digest1 [[lindex $slaves 1] debug digest]
@@ -131,10 +152,6 @@ start_server {tags {"repl"}} {
                     assert {$digest eq $digest0}
                     assert {$digest eq $digest1}
                     assert {$digest eq $digest2}
-                    #puts [$master dbsize]
-                    #puts [[lindex $slaves 0] dbsize]
-                    #puts [[lindex $slaves 1] dbsize]
-                    #puts [[lindex $slaves 2] dbsize]
                 }
            }
         }

tests/unit/aofrw.tcl

@@ -7,8 +7,12 @@ start_server {tags {"aofrw"}} {
         r bgrewriteaof
         r config set appendonly no
         r exec
-        set result [exec tail -n1 < [srv 0 stdout] ]
-    } {*Killing*AOF*child*}
+        wait_for_condition 50 100 {
+            [string match {*Killing*AOF*child*} [exec tail -n5 < [srv 0 stdout]]]
+        } else {
+            fail "Can't find 'Killing AOF child' into recent logs"
+        }
+    }
 
     foreach d {string int} {
         foreach e {ziplist linkedlist} {

tests/unit/expire.tcl

@@ -141,4 +141,15 @@ start_server {tags {"expire"}} {
         set size2 [r dbsize]
         list $size1 $size2
     } {3 0}
+
+    test {5 keys in, 5 keys out} {
+        r flushdb
+        r set a c
+        r expire a 5
+        r set t c
+        r set e c
+        r set s c
+        r set foo b
+        lsort [r keys *]
+    } {a e foo s t}
 }

utils/redis_init_script.tpl

@@ -1,7 +1,7 @@
 
 case "$1" in
     start)
-        if [ -f $$PIDFILE ]
+        if [ -f $PIDFILE ]
         then
                 echo "$PIDFILE exists, process is already running or crashed"
         else