StackExchange.Redis icon indicating copy to clipboard operation
StackExchange.Redis copied to clipboard

Support read-only commands (EVALSHA_RO, ...)

Open michael-eckhart-woellkart opened this issue 3 years ago • 9 comments

Hi team,

I want to use lua scripts on replica. Currently a EVALSHA command is send, which is not allowed by replicas. Redis supports also a read only version of this command EVALSHA_RO.

Is it possible to implement this read-only commands? https://redis.io/docs/manual/programmability/#read-only-scripts

I think we can probably do this quite simply, perhaps using the existing routing command-flags, i.e. if it is anything other than DemqndPrimary,and it is a v7+ server: use the RO option. Sound reasonable?

I also wonder whether you simply adding #!lua flags=no-writes at the top of the script would work (and work today).

mgravell avatar Aug 02 '22 06:08 mgravell

@mgravell, I think you first solution sounds good, because for "DemandPrimary" there is only read-only possible :).

I also tried with "#!lua flags=no-writes" at the top, but it does still not work.

I am not sure if the "DemandPrimary" is working well. My local cache has one replica, but when I run this test, it will throw an exception. image

None of the switch matches, so null will be returned. image

I think I was misunderstanding the configuration. I needed to add the replica as own endpoint.

var connectionMultiplexer = ConnectionMultiplexer.Connect("localhost:6379,abortConnect=False", (opt) =>
{
    opt.EndPoints.Add("localhost", 6380);
});

The good thing that it works now locally, but azure still uses verseion 4 and 6, which does not support "#!lua flags=no-writes" or EVALSHA_RO.

If this is non-cluster, then yes: replicas need to be in the configuration - but you can do it more simply via the config string localhost:6379,localhost:6380,abortConnect=False - the library will talk to both endpoints and determine the topology. For cluster, there are commands to discover much more about the topology automatically, which the library will use.

However! If you're using redis server below 7: none of these read-only script options will work, so... :shrug:

(/cc @philon-msft for server feature / version thoughts)

mgravell avatar Aug 02 '22 08:08 mgravell

Thanks for the hint with the connectionstring. Yes, I guess we need to host our own redis cache.

I had a quick look at implementing, and it isn't quite as simple as I would have hoped; since the default of CommandFlags is PreferMaster, the above implementation would mean that by default all scripts would be interpreted as read-only, which is very definitely not what we want. So; we could either:

  1. add new CommandFlags.ReadOnly which would have this side-effect; for now only used by EVAL/EVALSHA, but reusable in principle
  2. add new methods (ScriptEvaluateReadOnly or similar)

personally I kinda prefer 1, but I'm open to suggestions

mgravell avatar Aug 02 '22 10:08 mgravell

I personally prefer 2, because otherwise you have a new options only for to commands (as you mentioned above). But I am not a collaborator of this library. Also option 1 would be ok for me :).

spike of 1 for illustration only; note this could easily be changed to option 2; https://github.com/StackExchange/StackExchange.Redis/pull/2207

mgravell avatar Aug 02 '22 10:08 mgravell