summaryrefslogtreecommitdiff
path: root/src/db.c
diff options
context:
space:
mode:
authorViktor Söderqvist <viktor.soderqvist@est.tech>2022-02-08 12:19:27 +0100
committerGitHub <noreply@github.com>2022-02-08 13:19:27 +0200
commitb571c9609da99a62e79de1ca268f3c4431e5f38d (patch)
tree61941bfdebbcb6b620ddb5b9522235e8c7825abc /src/db.c
parent2e1bc942aa00a76ed3b0e5e2678da4ac90071d19 (diff)
downloadredis-b571c9609da99a62e79de1ca268f3c4431e5f38d.tar.gz
Remove assert and refuse delete expired on ro replicas (#10248)
There's an assertion added recently to make sure that non-write commands don't use lookupKeyWrite, It was initially meant to be used only on read-only replicas, but we thought it'll not have enough coverage, so used it on the masters too. We now realize that in some cases this can cause issues for modules, so we remove the assert. Other than that, we also make sure not to force expireIfNeeded on read-only replicas. even if they somehow run a write command. See https://github.com/redis/redis/pull/9572#discussion_r800179373
Diffstat (limited to 'src/db.c')
-rw-r--r--src/db.c20
1 files changed, 10 insertions, 10 deletions
diff --git a/src/db.c b/src/db.c
index 11665fa1d..b8a6ebae3 100644
--- a/src/db.c
+++ b/src/db.c
@@ -83,16 +83,16 @@ robj *lookupKey(redisDb *db, robj *key, int flags) {
robj *val = NULL;
if (de) {
val = dictGetVal(de);
- int force_delete_expired = flags & LOOKUP_WRITE;
- if (force_delete_expired) {
- /* Forcing deletion of expired keys on a replica makes the replica
- * inconsistent with the master. The reason it's allowed for write
- * commands is to make writable replicas behave consistently. It
- * shall not be used in readonly commands. Modules are accepted so
- * that we don't break old modules. */
- client *c = server.in_script ? scriptGetClient() : server.current_client;
- serverAssert(!c || !c->cmd || (c->cmd->flags & (CMD_WRITE|CMD_MODULE)));
- }
+ /* Forcing deletion of expired keys on a replica makes the replica
+ * inconsistent with the master. We forbid it on readonly replicas, but
+ * we have to allow it on writable replicas to make write commands
+ * behave consistently.
+ *
+ * It's possible that the WRITE flag is set even during a readonly
+ * command, since the command may trigger events that cause modules to
+ * perform additional writes. */
+ int is_ro_replica = server.masterhost && server.repl_slave_ro;
+ int force_delete_expired = flags & LOOKUP_WRITE && !is_ro_replica;
if (expireIfNeeded(db, key, force_delete_expired)) {
/* The key is no longer valid. */
val = NULL;