Redis 2.5.8 (2.6.0 RC2).

Use safe dictionary iterator from KEYS (fixes #487)

00-RELEASENOTES

@@ -26,12 +26,21 @@ 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.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 +62,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/version.h

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

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}
 }