From 30978004b33575bb9c1ff45ff8df166792a90f8b Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 30 Sep 2015 16:24:17 +0200 Subject: [PATCH 01/20] redis-cli pipe mode: don't stay in the write loop forever. The code was broken and resulted in redis-cli --pipe to, most of the times, writing everything received in the standard input to the Redis connection socket without ever reading back the replies, until all the content to write was written. This means that Redis had to accumulate all the output in the output buffers of the client, consuming a lot of memory. Fixed thanks to the original report of anomalies in the behavior provided by Twitter user @fsaintjacques. --- src/redis-cli.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index 825162b810c..92e29412771 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -1397,6 +1397,7 @@ static void getRDB(void) { * Bulk import (pipe) mode *--------------------------------------------------------------------------- */ +#define PIPEMODE_WRITE_LOOP_MAX_BYTES (128*1024) static void pipeMode(void) { int fd = context->fd; long long errors = 0, replies = 0, obuf_len = 0, obuf_pos = 0; @@ -1473,6 +1474,8 @@ static void pipeMode(void) { /* Handle the writable state: we can send protocol to the server. */ if (mask & AE_WRITABLE) { + ssize_t loop_nwritten = 0; + while(1) { /* Transfer current buffer to server. */ if (obuf_len != 0) { @@ -1489,6 +1492,7 @@ static void pipeMode(void) { } obuf_len -= nwritten; obuf_pos += nwritten; + loop_nwritten += nwritten; if (obuf_len != 0) break; /* Can't accept more data. */ } /* If buffer is empty, load from stdin. */ @@ -1524,7 +1528,8 @@ static void pipeMode(void) { obuf_pos = 0; } } - if (obuf_len == 0 && eof) break; + if ((obuf_len == 0 && eof) || + loop_nwritten > PIPEMODE_WRITE_LOOP_MAX_BYTES) break; } } From dc03e4c51b6ec1cccaaa0a76f61832e499f5871b Mon Sep 17 00:00:00 2001 From: Kevin McGehee Date: Wed, 14 Oct 2015 12:03:47 -0700 Subject: [PATCH 02/20] Fix master timeout during handshake This change allows a slave to properly time out a dead master during the extended asynchronous synchronization state machine. Now, slaves will record their last interaction with the master and apply the replication timeout before a response to the PSYNC request is received. --- src/replication.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/replication.c b/src/replication.c index cef65dfd779..e6b64d7299f 100644 --- a/src/replication.c +++ b/src/replication.c @@ -41,6 +41,7 @@ void replicationDiscardCachedMaster(void); void replicationResurrectCachedMaster(int newfd); void replicationSendAck(void); void putSlaveOnline(redisClient *slave); +int serverInHandshakeState(int repl_state); /* --------------------------- Utility functions ---------------------------- */ @@ -1196,6 +1197,7 @@ char *sendSynchronousCommand(int flags, int fd, ...) { return sdscatprintf(sdsempty(),"-Reading from master: %s", strerror(errno)); } + server.repl_transfer_lastio = server.unixtime; return sdsnew(buf); } return NULL; @@ -1626,7 +1628,7 @@ void undoConnectWithMaster(void) { int fd = server.repl_transfer_s; redisAssert(server.repl_state == REDIS_REPL_CONNECTING || - server.repl_state == REDIS_REPL_RECEIVE_PONG); + serverInHandshakeState(server.repl_state)); aeDeleteFileEvent(server.el,fd,AE_READABLE|AE_WRITABLE); close(fd); server.repl_transfer_s = -1; @@ -1645,7 +1647,7 @@ 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) + serverInHandshakeState(server.repl_state)) { undoConnectWithMaster(); } else { @@ -1802,6 +1804,20 @@ void roleCommand(redisClient *c) { } } +/* Returns 1 if the given replication state is a handshake state, + * 0 otherwise. */ +int serverInHandshakeState(int repl_state) { + return repl_state == REDIS_REPL_RECEIVE_PONG || + repl_state == REDIS_REPL_SEND_AUTH || + repl_state == REDIS_REPL_RECEIVE_AUTH || + repl_state == REDIS_REPL_SEND_PORT || + repl_state == REDIS_REPL_RECEIVE_PORT || + repl_state == REDIS_REPL_SEND_CAPA || + repl_state == REDIS_REPL_RECEIVE_CAPA || + repl_state == REDIS_REPL_SEND_PSYNC || + repl_state == REDIS_REPL_RECEIVE_PSYNC; +} + /* Send a REPLCONF ACK command to the master to inform it about the current * processed offset. If we are not connected with a master, the command has * no effects. */ @@ -2186,7 +2202,7 @@ void replicationCron(void) { /* Non blocking connection timeout? */ if (server.masterhost && (server.repl_state == REDIS_REPL_CONNECTING || - server.repl_state == REDIS_REPL_RECEIVE_PONG) && + serverInHandshakeState(server.repl_state)) && (time(NULL)-server.repl_transfer_lastio) > server.repl_timeout) { redisLog(REDIS_WARNING,"Timeout connecting to the MASTER..."); From 6ef80f4ed21c14a879166a96dda91733a48d895c Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 15 Oct 2015 09:59:07 +0200 Subject: [PATCH 03/20] Minor changes to PR #2813. * Function to test for slave handshake renamed slaveIsInHandshakeState. * Function no longer accepts arguments since it always tests the same global state. * Test for state translated to a range test since defines are guaranteed to stay in order in the future. * Use the new function in the ROLE command implementation as well. --- src/replication.c | 57 ++++++++++++++++++----------------------------- 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/src/replication.c b/src/replication.c index e6b64d7299f..b3ff458a5d9 100644 --- a/src/replication.c +++ b/src/replication.c @@ -41,7 +41,6 @@ void replicationDiscardCachedMaster(void); void replicationResurrectCachedMaster(int newfd); void replicationSendAck(void); void putSlaveOnline(redisClient *slave); -int serverInHandshakeState(int repl_state); /* --------------------------- Utility functions ---------------------------- */ @@ -910,6 +909,13 @@ void updateSlavesWaitingBgsave(int bgsaveerr, int type) { /* ----------------------------------- SLAVE -------------------------------- */ +/* Returns 1 if the given replication state is a handshake state, + * 0 otherwise. */ +int slaveIsInHandshakeState(void) { + return server.repl_state >= REDIS_REPL_RECEIVE_PONG && + server.repl_state <= REDIS_REPL_RECEIVE_PSYNC; +} + /* Abort the async download of the bulk dataset while SYNC-ing with master */ void replicationAbortSyncTransfer(void) { redisAssert(server.repl_state == REDIS_REPL_TRANSFER); @@ -1628,7 +1634,7 @@ void undoConnectWithMaster(void) { int fd = server.repl_transfer_s; redisAssert(server.repl_state == REDIS_REPL_CONNECTING || - serverInHandshakeState(server.repl_state)); + slaveIsInHandshakeState()); aeDeleteFileEvent(server.el,fd,AE_READABLE|AE_WRITABLE); close(fd); server.repl_transfer_s = -1; @@ -1647,7 +1653,7 @@ int cancelReplicationHandshake(void) { if (server.repl_state == REDIS_REPL_TRANSFER) { replicationAbortSyncTransfer(); } else if (server.repl_state == REDIS_REPL_CONNECTING || - serverInHandshakeState(server.repl_state)) + slaveIsInHandshakeState()) { undoConnectWithMaster(); } else { @@ -1782,42 +1788,23 @@ void roleCommand(redisClient *c) { addReplyBulkCBuffer(c,"slave",5); addReplyBulkCString(c,server.masterhost); addReplyLongLong(c,server.masterport); - switch(server.repl_state) { - case REDIS_REPL_NONE: slavestate = "none"; break; - case REDIS_REPL_CONNECT: slavestate = "connect"; break; - case REDIS_REPL_CONNECTING: slavestate = "connecting"; break; - case REDIS_REPL_RECEIVE_PONG: - case REDIS_REPL_SEND_AUTH: - case REDIS_REPL_RECEIVE_AUTH: - case REDIS_REPL_SEND_PORT: - case REDIS_REPL_RECEIVE_PORT: - case REDIS_REPL_SEND_CAPA: - case REDIS_REPL_RECEIVE_CAPA: - case REDIS_REPL_SEND_PSYNC: - case REDIS_REPL_RECEIVE_PSYNC: slavestate = "handshake"; break; - case REDIS_REPL_TRANSFER: slavestate = "sync"; break; - case REDIS_REPL_CONNECTED: slavestate = "connected"; break; - default: slavestate = "unknown"; break; + if (slaveIsInHandshakeState()) { + slavestate = "handshake"; + } else { + switch(server.repl_state) { + case REDIS_REPL_NONE: slavestate = "none"; break; + case REDIS_REPL_CONNECT: slavestate = "connect"; break; + case REDIS_REPL_CONNECTING: slavestate = "connecting"; break; + case REDIS_REPL_TRANSFER: slavestate = "sync"; break; + case REDIS_REPL_CONNECTED: slavestate = "connected"; break; + default: slavestate = "unknown"; break; + } } addReplyBulkCString(c,slavestate); addReplyLongLong(c,server.master ? server.master->reploff : -1); } } -/* Returns 1 if the given replication state is a handshake state, - * 0 otherwise. */ -int serverInHandshakeState(int repl_state) { - return repl_state == REDIS_REPL_RECEIVE_PONG || - repl_state == REDIS_REPL_SEND_AUTH || - repl_state == REDIS_REPL_RECEIVE_AUTH || - repl_state == REDIS_REPL_SEND_PORT || - repl_state == REDIS_REPL_RECEIVE_PORT || - repl_state == REDIS_REPL_SEND_CAPA || - repl_state == REDIS_REPL_RECEIVE_CAPA || - repl_state == REDIS_REPL_SEND_PSYNC || - repl_state == REDIS_REPL_RECEIVE_PSYNC; -} - /* Send a REPLCONF ACK command to the master to inform it about the current * processed offset. If we are not connected with a master, the command has * no effects. */ @@ -2202,8 +2189,8 @@ void replicationCron(void) { /* Non blocking connection timeout? */ if (server.masterhost && (server.repl_state == REDIS_REPL_CONNECTING || - serverInHandshakeState(server.repl_state)) && - (time(NULL)-server.repl_transfer_lastio) > server.repl_timeout) + slaveIsInHandshakeState()) && + (time(NULL)-server.repl_transfer_lastio) > server.repl_timeout) { redisLog(REDIS_WARNING,"Timeout connecting to the MASTER..."); undoConnectWithMaster(); From 8242d069f12f430d1087fc834e69973fef5d05a9 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 15 Oct 2015 10:03:33 +0200 Subject: [PATCH 04/20] Make clear that slave handshake states must be ordered. Make sure that people from the future will not break this rule. Related to issue #2813. --- src/redis.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/redis.h b/src/redis.h index b7f4ce3735f..34692a9523f 100644 --- a/src/redis.h +++ b/src/redis.h @@ -272,6 +272,7 @@ typedef long long mstime_t; /* millisecond time type. */ #define REDIS_REPL_NONE 0 /* No active replication */ #define REDIS_REPL_CONNECT 1 /* Must connect to master */ #define REDIS_REPL_CONNECTING 2 /* Connecting to master */ +/* --- Handshake states, must be ordered --- */ #define REDIS_REPL_RECEIVE_PONG 3 /* Wait for PING reply */ #define REDIS_REPL_SEND_AUTH 4 /* Send AUTH to master */ #define REDIS_REPL_RECEIVE_AUTH 5 /* Wait for AUTH reply */ @@ -280,6 +281,7 @@ typedef long long mstime_t; /* millisecond time type. */ #define REDIS_REPL_SEND_CAPA 8 /* Send REPLCONF capa */ #define REDIS_REPL_RECEIVE_CAPA 9 /* Wait for REPLCONF reply */ #define REDIS_REPL_SEND_PSYNC 10 /* Send PSYNC */ +/* --- End of handshake states --- */ #define REDIS_REPL_RECEIVE_PSYNC 11 /* Wait for PSYNC reply */ #define REDIS_REPL_TRANSFER 12 /* Receiving .rdb from master */ #define REDIS_REPL_CONNECTED 13 /* Connected to master */ From 7cb8481053cae4b41b71c6ac6ea2b2b5af360ba2 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 15 Oct 2015 10:04:35 +0200 Subject: [PATCH 05/20] Move end-comment of handshake states. For an error I missed the last handshake state. Related to issue #2813. --- src/redis.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/redis.h b/src/redis.h index 34692a9523f..8e63f6c5b61 100644 --- a/src/redis.h +++ b/src/redis.h @@ -281,8 +281,8 @@ typedef long long mstime_t; /* millisecond time type. */ #define REDIS_REPL_SEND_CAPA 8 /* Send REPLCONF capa */ #define REDIS_REPL_RECEIVE_CAPA 9 /* Wait for REPLCONF reply */ #define REDIS_REPL_SEND_PSYNC 10 /* Send PSYNC */ -/* --- End of handshake states --- */ #define REDIS_REPL_RECEIVE_PSYNC 11 /* Wait for PSYNC reply */ +/* --- End of handshake states --- */ #define REDIS_REPL_TRANSFER 12 /* Receiving .rdb from master */ #define REDIS_REPL_CONNECTED 13 /* Connected to master */ From cbf6614c1af560ed3219e08f38cefc164afb6fbc Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 15 Oct 2015 11:23:13 +0200 Subject: [PATCH 06/20] Regression test for issue #2813. --- tests/integration/replication.tcl | 53 +++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/integration/replication.tcl b/tests/integration/replication.tcl index bb907eba8ec..e811cf0eea3 100644 --- a/tests/integration/replication.tcl +++ b/tests/integration/replication.tcl @@ -1,3 +1,56 @@ +proc log_file_matches {log pattern} { + set fp [open $log r] + set content [read $fp] + close $fp + string match $pattern $content +} + +start_server {tags {"repl"}} { + set slave [srv 0 client] + set slave_host [srv 0 host] + set slave_port [srv 0 port] + set slave_log [srv 0 stdout] + start_server {} { + set master [srv 0 client] + set master_host [srv 0 host] + set master_port [srv 0 port] + + # Configure the master in order to hang waiting for the BGSAVE + # operation, so that the slave remains in the handshake state. + $master config set repl-diskless-sync yes + $master config set repl-diskless-sync-delay 1000 + + # Use a short replication timeout on the slave, so that if there + # are no bugs the timeout is triggered in a reasonable amount + # of time. + $slave config set repl-timeout 5 + + # Start the replication process... + $slave slaveof $master_host $master_port + + test {Slave enters handshake} { + wait_for_condition 50 1000 { + [string match *handshake* [$slave role]] + } else { + fail "Slave does not enter handshake state" + } + } + + # But make the master unable to send + # the periodic newlines to refresh the connection. The slave + # should detect the timeout. + $master debug sleep 10 + + test {Slave is able to detect timeout during handshake} { + wait_for_condition 50 1000 { + [log_file_matches $slave_log "*Timeout connecting to the MASTER*"] + } else { + fail "Slave is not able to detect timeout" + } + } + } +} + start_server {tags {"repl"}} { set A [srv 0 client] set A_host [srv 0 host] From 892b1c3c58a409580fe9cd2fafe065d983e68382 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 15 Oct 2015 12:46:07 +0200 Subject: [PATCH 07/20] Redis.conf example: make clear user must pass its path as argument. --- redis.conf | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/redis.conf b/redis.conf index 2206f2daf1f..66126d3ea63 100644 --- a/redis.conf +++ b/redis.conf @@ -1,4 +1,9 @@ -# Redis configuration file example +# Redis configuration file example. +# +# Note that in order to read the configuration file, Redis must be +# started with the file path as first argument: +# +# ./redis-server /path/to/redis.conf # Note on units: when memory size is needed, it is possible to specify # it in the usual form of 1k 5GB 4M and so forth: From 568c83dda75e6111b211eedeecee0e24043866d0 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 9 Oct 2015 16:15:53 +0200 Subject: [PATCH 08/20] Cluster: redis-trib fix, coverage for migrating=1 case. Kinda related to #2770. --- src/redis-trib.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/redis-trib.rb b/src/redis-trib.rb index 6002e4caa5d..8e07aa22add 100755 --- a/src/redis-trib.rb +++ b/src/redis-trib.rb @@ -496,6 +496,10 @@ def fix_open_slot(slot) # importing state in 1 slot. That's trivial to address. if migrating.length == 1 && importing.length == 1 move_slot(migrating[0],importing[0],slot,:verbose=>true,:fix=>true) + # Case 2: There are multiple nodes that claim the slot as importing, + # they probably got keys about the slot after a restart so opened + # the slot. In this case we just move all the keys to the owner + # according to the configuration. elsif migrating.length == 0 && importing.length > 0 xputs ">>> Moving all the #{slot} slot keys to its owner #{owner}" importing.each {|node| @@ -504,8 +508,14 @@ def fix_open_slot(slot) xputs ">>> Setting #{slot} as STABLE in #{node}" node.r.cluster("setslot",slot,"stable") } + # Case 3: There are no slots claiming to be in importing state, but + # there is a migrating node that actually don't have any key. We + # can just close the slot, probably a reshard interrupted in the middle. + elsif importing.length == 0 && migrating.length == 1 && + migrating[0].r.cluster("getkeysinslot",slot,10).length == 0 + migrating[0].r.cluster("setslot",slot,"stable") else - xputs "[ERR] Sorry, Redis-trib can't fix this slot yet (work in progress)" + xputs "[ERR] Sorry, Redis-trib can't fix this slot yet (work in progress). Slot is set as migrating in #{migrating.join(",")}, as importing in #{importing.join(",")}, owner is #{owner}" end end @@ -812,7 +822,7 @@ def move_slot(source,target,slot,o={}) source.r.client.call(["migrate",target.info[:host],target.info[:port],key,0,15000]) rescue => e if o[:fix] && e.to_s =~ /BUSYKEY/ - xputs "*** Target key #{key} exists. Replace it for FIX." + xputs "*** Target key #{key} exists. Replacing it for FIX." source.r.client.call(["migrate",target.info[:host],target.info[:port],key,0,15000,:replace]) else puts "" From 1fa63a78bc175d5adfdad23470fa3a24343f80f3 Mon Sep 17 00:00:00 2001 From: David Thomson Date: Wed, 14 Oct 2015 06:56:14 +0100 Subject: [PATCH 09/20] Update import command to optionally use copy and replace parameters --- src/redis-trib.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/redis-trib.rb b/src/redis-trib.rb index 8e07aa22add..577cf150dab 100755 --- a/src/redis-trib.rb +++ b/src/redis-trib.rb @@ -1149,7 +1149,8 @@ def call_cluster_cmd(argv,opt) def import_cluster_cmd(argv,opt) source_addr = opt['from'] xputs ">>> Importing data from #{source_addr} to cluster #{argv[1]}" - + use_copy = opt['copy'] + use_replace = opt['replace'] # Check the existing cluster. load_cluster_info_from_node(argv[0]) check_cluster @@ -1184,7 +1185,10 @@ def import_cluster_cmd(argv,opt) print "Migrating #{k} to #{target}: " STDOUT.flush begin - source.client.call(["migrate",target.info[:host],target.info[:port],k,0,15000]) + cmd = ["migrate",target.info[:host],target.info[:port],k,0,15000] + cmd << :copy if use_copy + cmd << :replace if use_replace + source.client.call(cmd) rescue => e puts e else @@ -1344,7 +1348,7 @@ def key_to_slot(key) ALLOWED_OPTIONS={ "create" => {"replicas" => true}, "add-node" => {"slave" => false, "master-id" => true}, - "import" => {"from" => :required}, + "import" => {"from" => :required, "copy" => false, "replace" => false}, "reshard" => {"from" => true, "to" => true, "slots" => true, "yes" => false} } From ab1f8ea5086cae7eaa58ec9bfacd238c6b0df7dc Mon Sep 17 00:00:00 2001 From: David Thomson Date: Wed, 14 Oct 2015 06:58:36 +0100 Subject: [PATCH 10/20] Add back blank line --- src/redis-trib.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/src/redis-trib.rb b/src/redis-trib.rb index 577cf150dab..068e60d4e13 100755 --- a/src/redis-trib.rb +++ b/src/redis-trib.rb @@ -1151,6 +1151,7 @@ def import_cluster_cmd(argv,opt) xputs ">>> Importing data from #{source_addr} to cluster #{argv[1]}" use_copy = opt['copy'] use_replace = opt['replace'] + # Check the existing cluster. load_cluster_info_from_node(argv[0]) check_cluster From c7ec1a367af33c66db228084ba9fbb72f8d1fc33 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 15 Oct 2015 15:44:54 +0200 Subject: [PATCH 11/20] Redis 3.0.5 --- 00-RELEASENOTES | 24 ++++++++++++++++++++++++ src/version.h | 2 +- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/00-RELEASENOTES b/00-RELEASENOTES index 6f126194921..67582f92479 100644 --- a/00-RELEASENOTES +++ b/00-RELEASENOTES @@ -10,6 +10,30 @@ 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 3.0.5 ] Release date: 15 Oct 2015 + +Upgrade urgency: MODERATE, the most important thing is a fix in the replication + code that may make the slave hanging forever if the master + remains with an open socket even if it is no longer able to + reply. + +* [FIX] MOVE now moves the TTL as well. A bug lasting forever... finally + fixed thanks to Andy Grunwald that reported it. + (reported by Andy Grunwald, fixed by Salvatore Sanfilippo) +* [FIX] Fix a false positive in HSTRLEN test. +* [FIX] Fix a bug in redis-cli --pipe mode that was not able to read back + replies from the server incrementally. Now a mass import will use + a lot less memory, and you can use --pipe to do incremental streaming. + (reported by Twitter user @fsaintjacques, fixed by Salvatore + Sanfilippo) +* [FIX] Slave detection of master timeout. (fixed by Kevin McGehee, refactoring + and regression test by Salvatore Sanfilippo) + +* [NEW] Cluster: redis-trib fix can fix an additional case for opens lots. + (Salvatore Sanfilippo) +* [NEW] Cluster: redis-trib import support for --copy and --replace options + (David Thomson) + --[ Redis 3.0.4 ] Release date: 8 Sep 2015 Upgrade urgency: HIGH for Redis and Sentinel. However note that in order to diff --git a/src/version.h b/src/version.h index 86aa96d484e..58fd3312aca 100644 --- a/src/version.h +++ b/src/version.h @@ -1 +1 @@ -#define REDIS_VERSION "3.0.4" +#define REDIS_VERSION "3.0.5" From c5f9f199df65d8ef4d39048776dc0e8ad94289fb Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Oct 2015 12:06:26 +0100 Subject: [PATCH 12/20] CONTRIBUTING updated. --- CONTRIBUTING | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING b/CONTRIBUTING index f7b6836f701..b33aacb3ea0 100644 --- a/CONTRIBUTING +++ b/CONTRIBUTING @@ -12,15 +12,17 @@ each source file that you contribute. PLEASE DO NOT POST GENERAL QUESTIONS that are not about bugs or suspected bugs in the Github issues system. We'll be very happy to help you and provide - all the support in the Redis Google Group. + all the support Reddit sub: - Redis Google Group address: - - https://groups.google.com/forum/?fromgroups#!forum/redis-db + http://reddit.com/r/redis + + There is also an active community of Redis users at Stack Overflow: + + http://stackoverflow.com/questions/tagged/redis # How to provide a patch for a new feature -1. Drop a message to the Redis Google Group with a proposal of semantics/API. +1. If it is a major feature or a semantical change, please post it as a new submission in r/redis on Reddit at http://reddit.com/r/redis. Try to be passionate about why the feature is needed, make users upvote your proposal to gain traction and so forth. Read feedbacks about the community. But in this first step **please don't write code yet**. 2. If in step 1 you get an acknowledge from the project leaders, use the following procedure to submit a patch: @@ -31,4 +33,6 @@ each source file that you contribute. d. Initiate a pull request on github ( http://help.github.com/send-pull-requests/ ) e. Done :) +For minor fixes just open a pull request on Github. + Thanks! From 28fb193ccdc59a6baf6738afc6525a75f07bdce7 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 9 Nov 2015 11:10:53 +0100 Subject: [PATCH 13/20] Fix error reply in subscribed Pub/Sub mode. PING is now a valid command to issue in this context. --- src/redis.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/redis.c b/src/redis.c index 0dbf7f4c291..62694c14b82 100644 --- a/src/redis.c +++ b/src/redis.c @@ -2259,7 +2259,7 @@ int processCommand(redisClient *c) { c->cmd->proc != unsubscribeCommand && c->cmd->proc != psubscribeCommand && c->cmd->proc != punsubscribeCommand) { - addReplyError(c,"only (P)SUBSCRIBE / (P)UNSUBSCRIBE / QUIT allowed in this context"); + addReplyError(c,"only (P)SUBSCRIBE / (P)UNSUBSCRIBE / PING / QUIT allowed in this context"); return REDIS_OK; } From d4f55990f863928464cebb2052f698e4d4056acb Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 17 Nov 2015 15:33:09 +0100 Subject: [PATCH 14/20] Fix MIGRATE entry in command table. Thanks to Oran Agra (@oranagra) for reporting. Key extraction would not work otherwise and it does not make sense to take wrong data in the command table. --- src/redis.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/redis.c b/src/redis.c index 62694c14b82..89f51773d64 100644 --- a/src/redis.c +++ b/src/redis.c @@ -261,7 +261,7 @@ struct redisCommand redisCommandTable[] = { {"cluster",clusterCommand,-2,"ar",0,NULL,0,0,0,0,0}, {"restore",restoreCommand,-4,"wm",0,NULL,1,1,1,0,0}, {"restore-asking",restoreCommand,-4,"wmk",0,NULL,1,1,1,0,0}, - {"migrate",migrateCommand,-6,"w",0,NULL,0,0,0,0,0}, + {"migrate",migrateCommand,-6,"ws",0,NULL,3,3,1,0,0}, {"asking",askingCommand,1,"r",0,NULL,0,0,0,0,0}, {"readonly",readonlyCommand,1,"rF",0,NULL,0,0,0,0,0}, {"readwrite",readwriteCommand,1,"rF",0,NULL,0,0,0,0,0}, From 3da69a9f226396d24fc0919fc4111645449a7cb1 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 17 Nov 2015 15:38:34 +0100 Subject: [PATCH 15/20] Update redis-cli help and the script to generate it. --- src/help.h | 188 +++++++++++++++++++++++++++++++-- utils/generate-command-help.rb | 4 +- 2 files changed, 183 insertions(+), 9 deletions(-) diff --git a/src/help.h b/src/help.h index 8395c525bb7..1ac5a882d18 100644 --- a/src/help.h +++ b/src/help.h @@ -1,4 +1,4 @@ -/* Automatically generated by utils/generate-command-help.rb, do not edit. */ +/* Automatically generated by generate-command-help.rb, do not edit. */ #ifndef __REDIS_HELP_H #define __REDIS_HELP_H @@ -15,7 +15,9 @@ static char *commandGroups[] = { "connection", "server", "scripting", - "hyperloglog" + "hyperloglog", + "cluster", + "geo" }; struct commandHelp { @@ -46,7 +48,7 @@ struct commandHelp { 9, "1.0.0" }, { "BITCOUNT", - "key [start] [end]", + "key [start end]", "Count set bits in a string", 1, "2.6.0" }, @@ -81,7 +83,7 @@ struct commandHelp { 9, "2.6.9" }, { "CLIENT KILL", - "ip:port", + "[ip:port] [ID client-id] [TYPE normal|slave|pubsub] [ADDR ip:port] [SKIPME yes/no]", "Kill the connection of a client", 9, "2.4.0" }, @@ -100,6 +102,116 @@ struct commandHelp { "Set the current connection name", 9, "2.6.9" }, + { "CLUSTER ADDSLOTS", + "slot [slot ...]", + "Assign new hash slots to receiving node", + 12, + "3.0.0" }, + { "CLUSTER COUNT-FAILURE-REPORTS", + "node-id", + "Return the number of failure reports active for a given node", + 12, + "3.0.0" }, + { "CLUSTER COUNTKEYSINSLOT", + "slot", + "Return the number of local keys in the specified hash slot", + 12, + "3.0.0" }, + { "CLUSTER DELSLOTS", + "slot [slot ...]", + "Set hash slots as unbound in receiving node", + 12, + "3.0.0" }, + { "CLUSTER FAILOVER", + "[FORCE|TAKEOVER]", + "Forces a slave to perform a manual failover of its master.", + 12, + "3.0.0" }, + { "CLUSTER FORGET", + "node-id", + "Remove a node from the nodes table", + 12, + "3.0.0" }, + { "CLUSTER GETKEYSINSLOT", + "slot count", + "Return local key names in the specified hash slot", + 12, + "3.0.0" }, + { "CLUSTER INFO", + "-", + "Provides info about Redis Cluster node state", + 12, + "3.0.0" }, + { "CLUSTER KEYSLOT", + "key", + "Returns the hash slot of the specified key", + 12, + "3.0.0" }, + { "CLUSTER MEET", + "ip port", + "Force a node cluster to handshake with another node", + 12, + "3.0.0" }, + { "CLUSTER NODES", + "-", + "Get Cluster config for the node", + 12, + "3.0.0" }, + { "CLUSTER REPLICATE", + "node-id", + "Reconfigure a node as a slave of the specified master node", + 12, + "3.0.0" }, + { "CLUSTER RESET", + "[HARD|SOFT]", + "Reset a Redis Cluster node", + 12, + "3.0.0" }, + { "CLUSTER SAVECONFIG", + "-", + "Forces the node to save cluster state on disk", + 12, + "3.0.0" }, + { "CLUSTER SET-CONFIG-EPOCH", + "config-epoch", + "Set the configuration epoch in a new node", + 12, + "3.0.0" }, + { "CLUSTER SETSLOT", + "slot IMPORTING|MIGRATING|STABLE|NODE [node-id]", + "Bind an hash slot to a specific node", + 12, + "3.0.0" }, + { "CLUSTER SLAVES", + "node-id", + "List slave nodes of the specified master node", + 12, + "3.0.0" }, + { "CLUSTER SLOTS", + "-", + "Get array of Cluster slot to node mappings", + 12, + "3.0.0" }, + { "COMMAND", + "-", + "Get array of Redis command details", + 9, + "2.8.13" }, + { "COMMAND COUNT", + "-", + "Get total number of Redis commands", + 9, + "2.8.13" }, + { "COMMAND GETKEYS", + "-", + "Extract keys given a full Redis command", + 9, + "2.8.13" }, + { "COMMAND INFO", + "command-name [command-name ...]", + "Get array of specific Redis command details", + 9, + "2.8.13" }, { "CONFIG GET", "parameter", "Get the value of a configuration parameter", @@ -181,7 +293,7 @@ struct commandHelp { 7, "1.2.0" }, { "EXISTS", - "key", + "key [key ...]", "Determine if a key exists", 0, "1.0.0" }, @@ -205,6 +317,36 @@ struct commandHelp { "Remove all keys from the current database", 9, "1.0.0" }, + { "GEOADD", + "key longitude latitude member [longitude latitude member ...]", + "Add one or more geospatial items in the geospatial index represented using a sorted set", + 13, + "" }, + { "GEODIST", + "key member1 member2 [unit]", + "Returns the distance between two members of a geospatial index", + 13, + "" }, + { "GEOHASH", + "key member [member ...]", + "Returns members of a geospatial index as standard geohash strings", + 13, + "" }, + { "GEOPOS", + "key member [member ...]", + "Returns longitude and latitude of members of a geospatial index", + 13, + "" }, + { "GEORADIUS", + "key longitude latitude radius m|km|ft|mi [WITHCOORD] [WITHDIST] [WITHHASH] [COUNT count]", + "Query a sorted set representing a geospatial index to fetch members matching a given maximum distance from a point", + 13, + "" }, + { "GEORADIUSBYMEMBER", + "key member radius m|km|ft|mi [WITHCOORD] [WITHDIST] [WITHHASH] [COUNT count]", + "Query a sorted set representing a geospatial index to fetch members matching a given maximum distance from a member", + 13, + "" }, { "GET", "key", "Get the value of a key", @@ -290,6 +432,11 @@ struct commandHelp { "Set the value of a hash field, only if the field does not exist", 5, "2.0.0" }, + { "HSTRLEN", + "key field", + "Get the length of the value of a hash field", + 5, + "3.2.0" }, { "HVALS", "key", "Get all the values in a hash", @@ -490,6 +637,16 @@ struct commandHelp { "Return a random key from the keyspace", 0, "1.0.0" }, + { "READONLY", + "-", + "Enables read queries for a connection to a cluster slave node", + 12, + "3.0.0" }, + { "READWRITE", + "-", + "Disables read queries for a connection to a cluster slave node", + 12, + "3.0.0" }, { "RENAME", "key newkey", "Rename a key", @@ -501,10 +658,15 @@ struct commandHelp { 0, "1.0.0" }, { "RESTORE", - "key ttl serialized-value", + "key ttl serialized-value [REPLACE]", "Create a key using the provided serialized value, previously obtained using DUMP.", 0, "2.6.0" }, + { "ROLE", + "-", + "Return the role of the instance in the context of replication", + 9, + "2.8.12" }, { "RPOP", "key", "Remove and get the last element in a list", @@ -512,7 +674,7 @@ struct commandHelp { "1.0.0" }, { "RPOPLPUSH", "source destination", - "Remove the last element in a list, append it to another list and return it", + "Remove the last element in a list, prepend it to another list and return it", 2, "1.2.0" }, { "RPUSH", @@ -720,13 +882,18 @@ struct commandHelp { "Forget about all watched keys", 7, "2.2.0" }, + { "WAIT", + "numslaves timeout", + "Wait for the synchronous replication of all the write commands sent in the context of the current connection", + 0, + "3.0.0" }, { "WATCH", "key [key ...]", "Watch the given keys to determine execution of the MULTI/EXEC block", 7, "2.2.0" }, { "ZADD", - "key score member [score member ...]", + "key [NX|XX] [CH] [INCR] score member [score member ...]", "Add one or more members to a sorted set, or update its score if it already exists", 4, "1.2.0" }, @@ -800,6 +967,11 @@ struct commandHelp { "Return a range of members in a sorted set, by index, with scores ordered from high to low", 4, "1.2.0" }, + { "ZREVRANGEBYLEX", + "key max min [LIMIT offset count]", + "Return a range of members in a sorted set, by lexicographical range, ordered from higher to lower strings.", + 4, + "2.8.9" }, { "ZREVRANGEBYSCORE", "key max min [WITHSCORES] [LIMIT offset count]", "Return a range of members in a sorted set, by score, with scores ordered from high to low", diff --git a/utils/generate-command-help.rb b/utils/generate-command-help.rb index 068953198d2..f3dfb31b324 100755 --- a/utils/generate-command-help.rb +++ b/utils/generate-command-help.rb @@ -12,7 +12,9 @@ "connection", "server", "scripting", - "hyperloglog" + "hyperloglog", + "cluster", + "geo" ].freeze GROUPS_BY_NAME = Hash[* From 8e491b1708f5d9ee5c44d940585a5b684d55be2b Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 17 Nov 2015 15:40:47 +0100 Subject: [PATCH 16/20] Remove "s" flag for MIGRATE in command table. Maybe there are legitimate use cases for MIGRATE inside Lua scripts, at least for now. When the command will be executed in an asynchronous fashion (planned) it is possible we'll no longer be able to permit it from within Lua scripts. --- src/redis.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/redis.c b/src/redis.c index 89f51773d64..05fbc66740e 100644 --- a/src/redis.c +++ b/src/redis.c @@ -261,7 +261,7 @@ struct redisCommand redisCommandTable[] = { {"cluster",clusterCommand,-2,"ar",0,NULL,0,0,0,0,0}, {"restore",restoreCommand,-4,"wm",0,NULL,1,1,1,0,0}, {"restore-asking",restoreCommand,-4,"wmk",0,NULL,1,1,1,0,0}, - {"migrate",migrateCommand,-6,"ws",0,NULL,3,3,1,0,0}, + {"migrate",migrateCommand,-6,"w",0,NULL,3,3,1,0,0}, {"asking",askingCommand,1,"r",0,NULL,0,0,0,0,0}, {"readonly",readonlyCommand,1,"rF",0,NULL,0,0,0,0,0}, {"readwrite",readwriteCommand,1,"rF",0,NULL,0,0,0,0,0}, From fe71dffbf29d88ef3e61d2a4e23f1ad6492b2077 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 27 Nov 2015 08:59:17 +0100 Subject: [PATCH 17/20] Redis Cluster: hint about validity factor when slave can't failover. --- src/cluster.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index 6280677ae26..c8e1b9f76f5 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -2624,8 +2624,10 @@ void clusterLogCantFailover(int reason) { (mstime() - myself->slaveof->fail_time) < nolog_fail_time) return; switch(reason) { - case REDIS_CLUSTER_CANT_FAILOVER_DATA_AGE: - msg = "Disconnected from master for longer than allowed."; + case CLUSTER_CANT_FAILOVER_DATA_AGE: + msg = "Disconnected from master for longer than allowed. " + "Please check the 'cluster-slave-validity-factor' configuration " + "option."; break; case REDIS_CLUSTER_CANT_FAILOVER_WAITING_DELAY: msg = "Waiting the delay before I can start a new failover."; From 3626699f1fd39b97f68858f9ca2d0bf0999aae53 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 27 Nov 2015 16:05:52 +0100 Subject: [PATCH 18/20] Handle wait3() errors. My guess was that wait3() with WNOHANG could never return -1 and an error. However issue #2897 may possibly indicate that this could happen under non clear conditions. While we try to understand this better, better to handle a return value of -1 explicitly, otherwise in the case a BGREWRITE is in progress but wait3() returns -1, the effect is to match the first branch of the if/else block since server.rdb_child_pid is -1, and call backgroundSaveDoneHandler() without a good reason, that will, in turn, crash the Redis server with an assertion. --- src/redis.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/redis.c b/src/redis.c index 05fbc66740e..3afe1601d5b 100644 --- a/src/redis.c +++ b/src/redis.c @@ -1172,7 +1172,13 @@ int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientData) { if (WIFSIGNALED(statloc)) bysignal = WTERMSIG(statloc); - if (pid == server.rdb_child_pid) { + if (pid == -1) { + redisLog(LOG_WARNING,"wait3() returned an error: %s. " + "rdb_child_pid = %d, aof_child_pid = %d", + strerror(errno), + (int) server.rdb_child_pid, + (int) server.aof_child_pid); + } else if (pid == server.rdb_child_pid) { backgroundSaveDoneHandler(exitcode,bysignal); } else if (pid == server.aof_child_pid) { backgroundRewriteDoneHandler(exitcode,bysignal); From 4f7d1e46cf267a1900d2abd8cf5df8f95f7f63d6 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 27 Nov 2015 16:10:34 +0100 Subject: [PATCH 19/20] Fix renamed define after merge. --- src/cluster.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cluster.c b/src/cluster.c index c8e1b9f76f5..edc030249b3 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -2624,7 +2624,7 @@ void clusterLogCantFailover(int reason) { (mstime() - myself->slaveof->fail_time) < nolog_fail_time) return; switch(reason) { - case CLUSTER_CANT_FAILOVER_DATA_AGE: + case REDIS_CLUSTER_CANT_FAILOVER_DATA_AGE: msg = "Disconnected from master for longer than allowed. " "Please check the 'cluster-slave-validity-factor' configuration " "option."; From 1b0f9bd09ee26fa7e97ac3c13d4cedf405079b05 Mon Sep 17 00:00:00 2001 From: Neuron Teckid Date: Tue, 15 Dec 2015 13:03:47 +0800 Subject: [PATCH 20/20] Keep different clusters from mixing --- src/cluster.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/cluster.c b/src/cluster.c index edc030249b3..e5c8767369c 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -1545,6 +1545,13 @@ int clusterProcessPacket(clusterLink *link) { redisLog(REDIS_DEBUG,"--- Processing packet of type %d, %lu bytes", type, (unsigned long) totlen); + /* if both nodes in "ok" clusters, they shouldn't have to meet each other */ + if (hdr->state == REDIS_CLUSTER_OK + && server.cluster->state == REDIS_CLUSTER_OK + && type == CLUSTERMSG_TYPE_MEET) { + return 1; + } + /* Perform sanity checks */ if (totlen < 16) return 1; /* At least signature, version, totlen, count. */ if (ntohs(hdr->ver) != CLUSTER_PROTO_VER)