From 0de01d8e514e68bb9eb4511ace1cbd0027913717 Mon Sep 17 00:00:00 2001 From: artix Date: Tue, 18 Feb 2020 10:46:10 +0100 Subject: [PATCH 1/4] Support Redis Cluster Proxy PROXY INFO command --- 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 7ad80c0a1..7e440d67c 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -1292,7 +1292,11 @@ static int cliSendCommand(int argc, char **argv, long repeat) { (argc == 3 && !strcasecmp(command,"latency") && !strcasecmp(argv[1],"graph")) || (argc == 2 && !strcasecmp(command,"latency") && - !strcasecmp(argv[1],"doctor"))) + !strcasecmp(argv[1],"doctor")) || + /* Format PROXY INFO command for Redis Cluster Proxy: + * https://github.com/artix75/redis-cluster-proxy */ + (argc >= 2 && !strcasecmp(command,"proxy") && + !strcasecmp(argv[1],"info"))) { output_raw = 1; } From 70705e758dc6ecf887dd472de98ff72d8a60fc48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=95=EC=8A=B9=ED=98=84?= Date: Wed, 18 Mar 2020 14:40:50 +0900 Subject: [PATCH 2/4] Update redis.conf --- redis.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis.conf b/redis.conf index c9d256bef..7c55a3ab0 100644 --- a/redis.conf +++ b/redis.conf @@ -1628,7 +1628,7 @@ hz 10 # offers, and enables by default, the ability to use an adaptive HZ value # which will temporary raise when there are many connected clients. # -# When dynamic HZ is enabled, the actual configured HZ will be used as +# When dynamic HZ is enabled, the actual configured HZ will be used # as a baseline, but multiples of the configured HZ value will be actually # used as needed once more clients are connected. In this way an idle # instance will use very little CPU time while a busy instance will be From 0c9916d008247df5cef12180495d9221eee6f60d Mon Sep 17 00:00:00 2001 From: WuYunlong Date: Wed, 18 Mar 2020 16:17:46 +0800 Subject: [PATCH 3/4] Add 14-consistency-check.tcl to prove there is a data consistency issue. --- tests/cluster/tests/14-consistency-check.tcl | 87 ++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 tests/cluster/tests/14-consistency-check.tcl diff --git a/tests/cluster/tests/14-consistency-check.tcl b/tests/cluster/tests/14-consistency-check.tcl new file mode 100644 index 000000000..a43725ebc --- /dev/null +++ b/tests/cluster/tests/14-consistency-check.tcl @@ -0,0 +1,87 @@ +source "../tests/includes/init-tests.tcl" + +test "Create a 5 nodes cluster" { + create_cluster 5 5 +} + +test "Cluster should start ok" { + assert_cluster_state ok +} + +test "Cluster is writable" { + cluster_write_test 0 +} + +proc find_non_empty_master {} { + set master_id_no {} + foreach_redis_id id { + if {[RI $id role] eq {master} && [R $id dbsize] > 0} { + set master_id_no $id + } + } + return $master_id_no +} + +proc get_one_of_my_replica {id} { + set replica_port [lindex [lindex [lindex [R $id role] 2] 0] 1] + set replica_id_num [get_instance_id_by_port redis $replica_port] + return $replica_id_num +} + +proc cluster_write_keys_with_expire {id ttl} { + set prefix [randstring 20 20 alpha] + set port [get_instance_attrib redis $id port] + set cluster [redis_cluster 127.0.0.1:$port] + for {set j 100} {$j < 200} {incr j} { + $cluster setex key_expire.$j $ttl $prefix.$j + } + $cluster close +} + +proc test_slave_load_expired_keys {aof} { + test "Slave expired keys is loaded when restarted: appendonly=$aof" { + set master_id [find_non_empty_master] + set replica_id [get_one_of_my_replica $master_id] + + set master_dbsize [R $master_id dbsize] + set slave_dbsize [R $replica_id dbsize] + assert_equal $master_dbsize $slave_dbsize + + set data_ttl 5 + cluster_write_keys_with_expire $master_id $data_ttl + after 100 + set replica_dbsize_1 [R $replica_id dbsize] + assert {$replica_dbsize_1 > $slave_dbsize} + + R $replica_id config set appendonly $aof + R $replica_id config rewrite + + set start_time [clock seconds] + set end_time [expr $start_time+$data_ttl+2] + R $replica_id save + set replica_dbsize_2 [R $replica_id dbsize] + assert {$replica_dbsize_2 > $slave_dbsize} + kill_instance redis $replica_id + + set master_port [get_instance_attrib redis $master_id port] + exec ../../../src/redis-cli -h 127.0.0.1 -p $master_port debug sleep [expr $data_ttl+3] > /dev/null & + + while {[clock seconds] <= $end_time} { + #wait for $data_ttl seconds + } + restart_instance redis $replica_id + + wait_for_condition 200 50 { + [R $replica_id ping] eq {PONG} + } else { + fail "replica #$replica_id not started" + } + + set replica_dbsize_3 [R $replica_id dbsize] + assert {$replica_dbsize_3 > $slave_dbsize} + } +} + +test_slave_load_expired_keys no +after 5000 +test_slave_load_expired_keys yes From f4a6b931cceb33ce730cc1ba4bb3c09070d275d3 Mon Sep 17 00:00:00 2001 From: WuYunlong Date: Wed, 18 Mar 2020 16:20:10 +0800 Subject: [PATCH 4/4] Fix master replica inconsistency for upgrading scenario. Before this commit, when upgrading a replica, expired keys will not be loaded, thus causing replica having less keys in db. To this point, master and replica's keys is logically consistent. However, before the keys in master and replica are physically consistent, that is, they have the same dbsize, if master got a problem and the replica got promoted and becomes new master of that partition, and master updates a key which does not exist on master, but physically exists on the old master(new replica), the old master would refuse to update the key, thus causing master and replica data inconsistent. How could this happen? That's all because of the wrong judgement of roles while starting up the server. We can not use server.masterhost to judge if the server is master or replica, since it fails in cluster mode. When we start the server, we load rdb and do want to load expired keys, and do not want to have the ability to active expire keys, if it is a replica. --- src/rdb.c | 2 +- src/server.c | 7 ++++++- src/server.h | 2 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index cbcea96c6..5d34f5a32 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -2231,7 +2231,7 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) { * received from the master. In the latter case, the master is * responsible for key expiry. If we would expire keys here, the * snapshot taken by the master may not be reflected on the slave. */ - if (server.masterhost == NULL && !(rdbflags&RDBFLAGS_AOF_PREAMBLE) && expiretime != -1 && expiretime < now) { + if (iAmMaster() && !(rdbflags&RDBFLAGS_AOF_PREAMBLE) && expiretime != -1 && expiretime < now) { decrRefCount(key); decrRefCount(val); } else { diff --git a/src/server.c b/src/server.c index f702da94a..467d09b67 100644 --- a/src/server.c +++ b/src/server.c @@ -1691,7 +1691,7 @@ void databasesCron(void) { /* Expire keys by random sampling. Not required for slaves * as master will synthesize DELs for us. */ if (server.active_expire_enabled) { - if (server.masterhost == NULL) { + if (iAmMaster()) { activeExpireCycle(ACTIVE_EXPIRE_CYCLE_SLOW); } else { expireSlaveKeys(); @@ -4863,6 +4863,11 @@ int redisIsSupervised(int mode) { return 0; } +int iAmMaster(void) { + return ((!server.cluster_enabled && server.masterhost == NULL) || + (server.cluster_enabled && nodeIsMaster(server.cluster->myself))); +} + int main(int argc, char **argv) { struct timeval tv; diff --git a/src/server.h b/src/server.h index fa6770dfa..fdfe5b8ea 100644 --- a/src/server.h +++ b/src/server.h @@ -2393,4 +2393,6 @@ int tlsConfigure(redisTLSContextConfig *ctx_config); #define redisDebugMark() \ printf("-- MARK %s:%d --\n", __FILE__, __LINE__) +int iAmMaster(void); + #endif