The problem/use-case that the feature addresses

Lua operations that are readonly can run on replicas. However command info for evalsha and eval tell that they can be write commands. Redis clients use this information to route to replica or master. Evalsha is routed to masters since it's not a read only command. This primarily is for redis in cluster mode.

Description of the feature

Similar to GEORADIUS_RO, introduce EVAL_RO and EVALSHA_RO that are readonly versions of EVAL and EVALSHA.

Alternatives you've considered

Modifying client library to explicitly route calls to replica nodes.

Additional information

Have made a contribution on go-redis to explicitly read from replicas https://github.com/go-redis/redis/pull/1581

Comment From: madolson

@prathik You should be able to send an EVAL/EVALSHA command to a replica in cluster mode. We do an explicit check here: https://github.com/redis/redis/blob/unstable/src/cluster.c#L5887

This seems more like just a client feature. The client could expose a virtual "EVALSHARO" command that really executes EVALSHA just against a read replica.

Comment From: madolson

Actually, thinking about this more, there is probably some value in a RO version of EVAL/EVALSHA. I can see two clear use cases: 1. During failovers, we can allow this type of eval command. 2. A lua script that does one write becomes unkillable, so this is a safety mechanism to limit the blast radius.

Comment From: prathik

@madolson yes, clients can expose a readonly version of evalsha, it's a similar idea that I contributed to go-redis/redis.

It's more idiomatic to have explicit readonly commands to run on replicas. For eg. SET is not readonly thus blocked on replicas and GET is readonly thus allowed on replica.

There is an exception made to Exec Lua commands. Though evalsha / eval should still be allowed to run on replicas for backward compatibility adding evalsha_ro and eval_ro would make it more explicit that commands can be run on replicas.

They give the advantage that you are talking about above. Any decision that has to be made on read only command would apply to evalsha_ro / eval_ro and avoid any specific branching needed to be done for eval / evalsha on replicas. Would like to know if you have any vision or direction for this and I can take it forward and help contributing to redis!

Comment From: itamarhaber

Sounds like a good idea to me /cc @MeirShpilraien

Comment From: madolson

@redis/core-team Does anyone dislike this idea? This ties into a related conversation we were having about scripts with different modes. A possible alternative to this is to have an eval/evalsha with flags.

Comment From: oranagra

At first when i saw GEORADIUS_RO being mentioned i thought i won't like it (because i really dislike GEORADIUS_RO), but actually that's because GEORADIUS is wrong (an argument changing the behavior of the command from read-only to write).

So in that light i actually like the idea of EVAL_RO, which would set the flag of the command, and also prevent any non read-only commands (and maybe also may-replicate ones) from being executed by the script.

Trying to extrapolate that idea to our other problem with use-memory, maybe we can also create and EVAL_DEL command (one that allows write commands, but disallows use-memory commands. I must say that unlike EVAL_RO, this one feels wrong. another idea is for EVAL_FLAGS to take a list of flags like read write use-memory, i don't particularly like this idea either.

I wanna remind us that we had some idea of adding a redis.somethign() API which will be called by the script before it executes any redis command. the main differences between that idea and the one in this issue, is that being a command flag, the client library can be aware of it, and on the other hand, it's a little less couple with the script. so i suppose that for RO, a command flag is better, and for the other flags, an API called by the script is better.

Comment From: madolson

The problem with flags is that it also doesn't help with the ACL limitation of reducing the blast radius of a script. So I suppose we have converged around EVAL_RO and EVALSHA_RO. Which seems like an easy thing to implement.

Comment From: nmvk

Taking a look

Comment From: yossigo

I agree with the analysis and conclusion, but not very happy with underscores in the command name. I propose to consider something like EVAL:RO which could be implemented as a new command but also suggest it's a new semantic of a "command modifier".

Comment From: oranagra

@yossigo , this would screw off the command stats.

cmdstat_eval:ro:calls=1....

a client breaking info lines at the : may fail in several ways.