Automatically detect and reupload flushed LUA script

[ORuban]

Though executed scripts are guaranteed to be in the script cache of a given execution of a Redis instance forever, things like Redis instance restart do flush the script cache. This causing RedisServerException with "NOSCRIPT No matching script. Please use EVAL." errors on client side. Right now the recovery step is to explicitly reapload the script.

It would be awesome if LUA script flushing could be handled by the library internally. It would also be a good fit to what EVAL command documentation describes:

Practically speaking, for the client it is much better to simply assume that in the context of a given connection, cached scripts are guaranteed to be there unless an administrator explicitly called the SCRIPT FLUSH command.

[mgravell]

For the ScriptEvaluate use-case, this should already be the case. Do you have an example of the API you're using when this is failing?

[ORuban]

we are using IDatabaseAsync.ScriptEvaluateAsync, in particular this overload

Task<RedisResult> ScriptEvaluateAsync(byte[] hash, RedisKey[] keys = null, RedisValue[] values = null, CommandFlags flags = CommandFlags.None);

Just in case, we deal with IDatabase instance that is received via IConnectionMultiplexer.GetDatabase()

IConnectionMultiplexer multiplexer = ...
var database = multiplexer.GetDatabase();
database.ScriptEvaluateAsync(hash, keys, values, CommandFlags.None);

[NickCraver]

@ORuban which version of the library are you on? I'm wondering if you have a version from before this was handled.

[ORuban]

@NickCraver we are having a fork that last time was merged with StackExchange.Redis master at Nov, 13. Please, as you say some work has been done, may you point to related threads for reference so we could learn what was changed? Just asking as I had tried to search accross any recent commits, issues using "Lua" /"script" as keyworld, but failed to find anything related.

[mgravell]

it looks like you're only supplying the hash; in that scenario, it can't possibly reconstruct the script - all it has is the hash; it isn't going to try and brute-force a script that has the same hash :)

if you use the overload that takes the string version, it still uses EVALSHA - it performs the hashing internally (IIRC, the first time it sees a new script, it performs an EVAL and SCRIPT LOAD - only switching to EVALSHA when it has actually seen a success response from the SCRIPT LOAD)

[ORuban]

thank you, I have checked the corresponding code and it should be exactly what we need. Though, it seems to be a bit redundant to have hash calculation for every operation, especially when LoadedLuaScript is used. It already has both Hash and original script inside. So in theory the logic could be: try to find LoadedLuaScript.Hash in cache, on miss -> upload script using LoadedLuaScript.ExecutableScript.

[mgravell]

which is why I asked which API you were using, and you said ScriptEvaluateAsync. If what you're saying is that LoadedLuaScript.ExecutableScript never reloads scripts: then yes, that sounds fixable

[ORuban]

it is more like we use ScriptEvaluateAsync directly just because LoadedLuaScript.EvaluateAsync currently does nothing special, as it just propagates call:

public Task<RedisResult> EvaluateAsync(IDatabaseAsync db, object ps = null, RedisKey? withKeyPrefix = null, CommandFlags flags = CommandFlags.None)
        {
            Original.ExtractParameters(ps, withKeyPrefix, out RedisKey[] keys, out RedisValue[] args);
            return db.ScriptEvaluateAsync(Hash, keys, args, flags);
        }
        public Task<RedisResult> EvaluateAsync(IDatabaseAsync db, object ps = null, RedisKey? withKeyPrefix = null, CommandFlags flags = CommandFlags.None)
        {
            Original.ExtractParameters(ps, withKeyPrefix, out RedisKey[] keys, out RedisValue[] args);
            return db.ScriptEvaluateAsync(Hash, keys, args, flags);
        }

So yes, based on above and to make things clear, let me please rethrase original issue to

LoadedLuaScript.ExecutableScript misses scripts reload.

[NickCraver]

Okay so what happens today is we do flush the cache locally in the result processor (here: https://github.com/StackExchange/StackExchange.Redis/blob/528d732f4daf2742e46d6fdad006e0e735c49abf/src/StackExchange.Redis/ResultProcessor.cs#L1537), but then we don't care if you already have an instance of a LoadedLuaScript. Agreed looks like we can improve that probably, and it's testable just need to grab some time, have some other items high-pri for 2.5 but can take a poke at this. Thanks for the details @ORuban! Scenario makes sense now.

[martintmk]

@NickCraver , @mgravell any chance of taking a quick look at the proposed PR? So that I can continue work on it. Thanks :)

[NickCraver]

Tidying up here - improvements were merged back in #2170 and in the 2.6.66 release - thanks for the ideas and help here!