From c95a582a749a83b117bdd51f4a7a29a616a0bf3c Mon Sep 17 00:00:00 2001
From: Madelyn Olson <matolson@amazon.com>
Date: Thu, 30 May 2019 11:50:31 -0700
Subject: [PATCH 1/5] Add configuration option for allowing reads on cluster
down
---
redis.conf | 16 ++++++++++++++++
src/cluster.c | 13 ++++++++++---
src/config.c | 2 ++
src/server.h | 2 ++
4 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/redis.conf b/redis.conf
index d37760176..870849a79 100644
--- a/redis.conf
+++ b/redis.conf
@@ -1194,6 +1194,22 @@ lua-time-limit 5000
#
# cluster-replica-no-failover no
+# This option, when set to yes, allows nodes to serve read traffic while the
+# the cluster is in a down state, as long as it believes it owns the slots.
+#
+# This is useful for two cases. The first case is for when an application
+# doesn't require consistency of data during node failures or network partitions.
+# One example of this is a cache, where as long as the node has the data it
+# should be able to serve it.
+#
+# The second use case is for configurations that don't meet the recommended
+# three shards but want to enable cluster mode and scale later. A
+# master outage in a 1 or 2 shard configuration causes a read/write outage to the
+# entire cluster without this option set, with it set there is only a write outage.
+# Without a quorum of masters, slot ownership will not change automatically.
+#
+# cluster-allow-reads-when-down no
+
# In order to setup your cluster make sure to read the documentation
# available at http://redis.io web site.
diff --git a/src/cluster.c b/src/cluster.c
index 9e6ddb2c4..4024035e9 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -5595,8 +5595,12 @@ clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, in
* without redirections or errors in all the cases. */
if (n == NULL) return myself;
- /* Cluster is globally down but we got keys? We can't serve the request. */
- if (server.cluster->state != CLUSTER_OK) {
+ /* Cluster is globally down but we got keys? We only serve the request
+ * if it is a read command and when allow_reads_when_down is enabled. */
+ if ((server.cluster->state != CLUSTER_OK) &&
+ !(server.cluster_allow_reads_when_down && ((cmd->flags & CMD_READONLY)
+ || (cmd->proc == evalCommand) || (cmd->proc == evalShaCommand))))
+ {
if (error_code) *error_code = CLUSTER_REDIR_DOWN_STATE;
return NULL;
}
@@ -5701,7 +5705,10 @@ int clusterRedirectBlockedClientIfNeeded(client *c) {
dictEntry *de;
dictIterator *di;
- /* If the cluster is down, unblock the client with the right error. */
+ /* If the cluster is down, unblock the client with the right error.
+ * If the cluster is configured to allow reads on cluster down, we
+ * still want to emit this error since a write will be required
+ * to unblock them which may never come. */
if (server.cluster->state == CLUSTER_FAIL) {
clusterRedirectClient(c,NULL,0,CLUSTER_REDIR_DOWN_STATE);
return 1;
diff --git a/src/config.c b/src/config.c
index 5bfb0de5c..5ff333fec 100644
--- a/src/config.c
+++ b/src/config.c
@@ -2166,6 +2166,8 @@ standardConfig configs[] = {
createBoolConfig("syslog-enabled", NULL, IMMUTABLE_CONFIG, server.syslog_enabled, 0, NULL, NULL),
createBoolConfig("cluster-enabled", NULL, IMMUTABLE_CONFIG, server.cluster_enabled, 0, NULL, NULL),
createBoolConfig("appendonly", NULL, MODIFIABLE_CONFIG, server.aof_enabled, 0, NULL, updateAppendonly),
+ createBoolConfig("cluster-allow-reads-when-down", NULL, MODIFIABLE_CONFIG, server.cluster_allow_reads_when_down, 0, NULL, NULL},
+
/* String Configs */
createStringConfig("aclfile", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.acl_filename, "", NULL, NULL),
diff --git a/src/server.h b/src/server.h
index f2c93241c..b5e51002b 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1334,6 +1334,8 @@ struct redisServer {
to set in order to suppress certain
native Redis Cluster features. Check the
REDISMODULE_CLUSTER_FLAG_*. */
+ int cluster_allow_reads_when_down; /* Are reads allowed when the cluster
+ is down? */
/* Scripting */
lua_State *lua; /* The Lua interpreter. We use just one for all clients */
client *lua_client; /* The "fake client" to query Redis from Lua */
From 44aa22c63553d6be2879c51161feb50c1b31eab8 Mon Sep 17 00:00:00 2001
From: Madelyn Olson <matolson@amazon.com>
Date: Mon, 28 Oct 2019 12:58:03 -0700
Subject: [PATCH 2/5] Added better exception handling around scripting and
module
---
src/module.c | 11 +++++++++--
src/scripting.c | 16 ++++++++++++----
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/src/module.c b/src/module.c
index 5d9a39387..31d337b14 100644
--- a/src/module.c
+++ b/src/module.c
@@ -3174,6 +3174,8 @@ fmterr:
* EINVAL: wrong command arity.
* ENOENT: command does not exist.
* EPERM: operation in Cluster instance with key in non local slot.
+ * EROFS: operation in Cluster instance when a write command is sent
+ * in a readonly state.
*
* This API is documented here: https://redis.io/topics/modules-intro
*/
@@ -3231,13 +3233,18 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
* trying to access non-local keys, with the exception of commands
* received from our master. */
if (server.cluster_enabled && !(ctx->client->flags & CLIENT_MASTER)) {
+ int error_code;
/* Duplicate relevant flags in the module client. */
c->flags &= ~(CLIENT_READONLY|CLIENT_ASKING);
c->flags |= ctx->client->flags & (CLIENT_READONLY|CLIENT_ASKING);
- if (getNodeByQuery(c,c->cmd,c->argv,c->argc,NULL,NULL) !=
+ if (getNodeByQuery(c,c->cmd,c->argv,c->argc,NULL,&error_code) !=
server.cluster->myself)
{
- errno = EPERM;
+ if (error_code == CLUSTER_REDIR_DOWN_STATE) {
+ errno = EROFS;
+ } else {
+ errno = EPERM;
+ }
goto cleanup;
}
}
diff --git a/src/scripting.c b/src/scripting.c
index 7cf21f408..c627207d5 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -679,15 +679,23 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
if (server.cluster_enabled && !server.loading &&
!(server.lua_caller->flags & CLIENT_MASTER))
{
+ int error_code;
/* Duplicate relevant flags in the lua client. */
c->flags &= ~(CLIENT_READONLY|CLIENT_ASKING);
c->flags |= server.lua_caller->flags & (CLIENT_READONLY|CLIENT_ASKING);
- if (getNodeByQuery(c,c->cmd,c->argv,c->argc,NULL,NULL) !=
+ if (getNodeByQuery(c,c->cmd,c->argv,c->argc,NULL,&error_code) !=
server.cluster->myself)
{
- luaPushError(lua,
- "Lua script attempted to access a non local key in a "
- "cluster node");
+ if (error_code == CLUSTER_REDIR_DOWN_STATE) {
+ luaPushError(lua,
+ "Lua script attempted execute a write command while "
+ "cluster is down");
+ } else {
+ luaPushError(lua,
+ "Lua script attempted to access a non local key in a "
+ "cluster node");
+ }
+
goto cleanup;
}
}
From 576a08908b0fcf85a01803507238c4cc14bd85d1 Mon Sep 17 00:00:00 2001
From: Madelyn Olson <matolson@amazon.com>
Date: Wed, 30 Oct 2019 00:11:17 -0700
Subject: [PATCH 3/5] Split error message so dependandent callers give a useful
result
---
src/cluster.c | 23 +++++++++++++++--------
src/cluster.h | 1 +
src/module.c | 5 ++++-
src/scripting.c | 10 +++++++---
4 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/src/cluster.c b/src/cluster.c
index 4024035e9..a18543e0b 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -5476,8 +5476,8 @@ void readwriteCommand(client *c) {
* already "down" but it is fragile to rely on the update of the global state,
* so we also handle it here.
*
- * CLUSTER_REDIR_DOWN_STATE if the cluster is down but the user attempts to
- * execute a command that addresses one or more keys. */
+ * CLUSTER_REDIR_DOWN_STATE and CLUSTER_REDIR_DOWN_RO_STATE if the cluster is
+ * down but the user attempts to execute a command that addresses one or more keys. */
clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, int argc, int *hashslot, int *error_code) {
clusterNode *n = NULL;
robj *firstkey = NULL;
@@ -5597,12 +5597,17 @@ clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, in
/* Cluster is globally down but we got keys? We only serve the request
* if it is a read command and when allow_reads_when_down is enabled. */
- if ((server.cluster->state != CLUSTER_OK) &&
- !(server.cluster_allow_reads_when_down && ((cmd->flags & CMD_READONLY)
- || (cmd->proc == evalCommand) || (cmd->proc == evalShaCommand))))
- {
- if (error_code) *error_code = CLUSTER_REDIR_DOWN_STATE;
- return NULL;
+ if (server.cluster->state != CLUSTER_OK) {
+ if (!server.cluster_allow_reads_when_down) {
+ if (error_code) *error_code = CLUSTER_REDIR_DOWN_STATE;
+ return NULL;
+ }
+
+ if (!(cmd->flags & CMD_READONLY) && !(cmd->proc == evalCommand)
+ && !(cmd->proc == evalShaCommand)) {
+ if (error_code) *error_code = CLUSTER_REDIR_DOWN_RO_STATE;
+ return NULL;
+ }
}
/* Return the hashslot by reference. */
@@ -5671,6 +5676,8 @@ void clusterRedirectClient(client *c, clusterNode *n, int hashslot, int error_co
addReplySds(c,sdsnew("-TRYAGAIN Multiple keys request during rehashing of slot\r\n"));
} else if (error_code == CLUSTER_REDIR_DOWN_STATE) {
addReplySds(c,sdsnew("-CLUSTERDOWN The cluster is down\r\n"));
+ } else if (error_code == CLUSTER_REDIR_DOWN_RO_STATE) {
+ addReplySds(c,sdsnew("-CLUSTERDOWN The cluster is down and only accepts read commands\r\n"));
} else if (error_code == CLUSTER_REDIR_DOWN_UNBOUND) {
addReplySds(c,sdsnew("-CLUSTERDOWN Hash slot not served\r\n"));
} else if (error_code == CLUSTER_REDIR_MOVED ||
diff --git a/src/cluster.h b/src/cluster.h
index 3ba60df6e..35fc0cbfa 100644
--- a/src/cluster.h
+++ b/src/cluster.h
@@ -29,6 +29,7 @@
#define CLUSTER_REDIR_MOVED 4 /* -MOVED redirection required. */
#define CLUSTER_REDIR_DOWN_STATE 5 /* -CLUSTERDOWN, global state. */
#define CLUSTER_REDIR_DOWN_UNBOUND 6 /* -CLUSTERDOWN, unbound slot. */
+#define CLUSTER_REDIR_DOWN_RO_STATE 7 /* -CLUSTERDOWN, allow reads. */
struct clusterNode;
diff --git a/src/module.c b/src/module.c
index 31d337b14..a4e7cbe2c 100644
--- a/src/module.c
+++ b/src/module.c
@@ -3176,6 +3176,7 @@ fmterr:
* EPERM: operation in Cluster instance with key in non local slot.
* EROFS: operation in Cluster instance when a write command is sent
* in a readonly state.
+ * ENETDOWN: operation in Cluster instance when cluster is down.
*
* This API is documented here: https://redis.io/topics/modules-intro
*/
@@ -3240,8 +3241,10 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
if (getNodeByQuery(c,c->cmd,c->argv,c->argc,NULL,&error_code) !=
server.cluster->myself)
{
- if (error_code == CLUSTER_REDIR_DOWN_STATE) {
+ if (error_code == CLUSTER_REDIR_DOWN_RO_STATE) {
errno = EROFS;
+ } else if (error_code == CLUSTER_REDIR_DOWN_STATE) {
+ errno = ENETDOWN;
} else {
errno = EPERM;
}
diff --git a/src/scripting.c b/src/scripting.c
index c627207d5..96eb9681d 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -686,11 +686,15 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
if (getNodeByQuery(c,c->cmd,c->argv,c->argc,NULL,&error_code) !=
server.cluster->myself)
{
- if (error_code == CLUSTER_REDIR_DOWN_STATE) {
+ if (error_code == CLUSTER_REDIR_DOWN_RO_STATE) {
luaPushError(lua,
- "Lua script attempted execute a write command while "
+ "Lua script attempted to execute a write command while the"
+ "cluster is down and readonly");
+ } else if (error_code == CLUSTER_REDIR_DOWN_STATE) {
+ luaPushError(lua,
+ "Lua script attempted to execute a command while the"
"cluster is down");
- } else {
+ } else {}
luaPushError(lua,
"Lua script attempted to access a non local key in a "
"cluster node");
From 12caffee6159c6fc2bdf226a224a6c5ed67a42cb Mon Sep 17 00:00:00 2001
From: Madelyn Olson <matolson@amazon.com>
Date: Mon, 16 Dec 2019 23:38:31 -0800
Subject: [PATCH 4/5] Added a missed space in lua errors
---
src/scripting.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/scripting.c b/src/scripting.c
index 96eb9681d..98d4b2b7e 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -688,13 +688,13 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
{
if (error_code == CLUSTER_REDIR_DOWN_RO_STATE) {
luaPushError(lua,
- "Lua script attempted to execute a write command while the"
+ "Lua script attempted to execute a write command while the "
"cluster is down and readonly");
} else if (error_code == CLUSTER_REDIR_DOWN_STATE) {
luaPushError(lua,
- "Lua script attempted to execute a command while the"
+ "Lua script attempted to execute a command while the "
"cluster is down");
- } else {}
+ } else {
luaPushError(lua,
"Lua script attempted to access a non local key in a "
"cluster node");
From 7b3e3d6a13db7bfe0b93056d9a004185e18d6ae3 Mon Sep 17 00:00:00 2001
From: Madelyn Olson <matolson@amazon.com>
Date: Mon, 16 Dec 2019 23:40:19 -0800
Subject: [PATCH 5/5] Resolved merge miss
---
src/config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/config.c b/src/config.c
index 5ff333fec..3d53e656b 100644
--- a/src/config.c
+++ b/src/config.c
@@ -2166,7 +2166,7 @@ standardConfig configs[] = {
createBoolConfig("syslog-enabled", NULL, IMMUTABLE_CONFIG, server.syslog_enabled, 0, NULL, NULL),
createBoolConfig("cluster-enabled", NULL, IMMUTABLE_CONFIG, server.cluster_enabled, 0, NULL, NULL),
createBoolConfig("appendonly", NULL, MODIFIABLE_CONFIG, server.aof_enabled, 0, NULL, updateAppendonly),
- createBoolConfig("cluster-allow-reads-when-down", NULL, MODIFIABLE_CONFIG, server.cluster_allow_reads_when_down, 0, NULL, NULL},
+ createBoolConfig("cluster-allow-reads-when-down", NULL, MODIFIABLE_CONFIG, server.cluster_allow_reads_when_down, 0, NULL, NULL),
/* String Configs */