Unverified Commit b571c960 authored by Viktor Söderqvist's avatar Viktor Söderqvist Committed by GitHub
Browse files

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
parent 2e1bc942
...@@ -83,16 +83,16 @@ robj *lookupKey(redisDb *db, robj *key, int flags) { ...@@ -83,16 +83,16 @@ robj *lookupKey(redisDb *db, robj *key, int flags) {
robj *val = NULL; robj *val = NULL;
if (de) { if (de) {
val = dictGetVal(de); val = dictGetVal(de);
int force_delete_expired = flags & LOOKUP_WRITE; /* Forcing deletion of expired keys on a replica makes the replica
if (force_delete_expired) { * inconsistent with the master. We forbid it on readonly replicas, but
/* Forcing deletion of expired keys on a replica makes the replica * we have to allow it on writable replicas to make write commands
* inconsistent with the master. The reason it's allowed for write * behave consistently.
* commands is to make writable replicas behave consistently. It *
* shall not be used in readonly commands. Modules are accepted so * It's possible that the WRITE flag is set even during a readonly
* that we don't break old modules. */ * command, since the command may trigger events that cause modules to
client *c = server.in_script ? scriptGetClient() : server.current_client; * perform additional writes. */
serverAssert(!c || !c->cmd || (c->cmd->flags & (CMD_WRITE|CMD_MODULE))); 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)) { if (expireIfNeeded(db, key, force_delete_expired)) {
/* The key is no longer valid. */ /* The key is no longer valid. */
val = NULL; val = NULL;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment