StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.87k stars 1.51k forks source link

EVALSHA isn't working within transaction. #2531

Open AppsoleutSatbir opened 1 year ago

AppsoleutSatbir commented 1 year ago

I have create a LUA script to execute within a Transaction with other native command functions.

When I run the LUA script without transaction, it works as expected and EVALSHA get called always. But when I wrap the same LUA script with same key & values and execute in a transaction (with or without other command functions), the framework sends script for every execution. EVAL gets called on script always.

Is this an expected behaviour or am I doing from wrong? @NickCraver @mgravell

Any suggessions to use EVALSHA in Transaction without sending script everytime?

References:------- Below are MONITOR logs with NO Transaction:

1693207783.585817 [0 122.160.68.144:57986] "SELECT" "0"
1693207783.585817 [0 122.160.68.144:57986] "SCRIPT" "LOAD" "MY_LUA_SCRIPT"
1693207784.861812 [0 122.160.68.144:57986] "EVALSHA" "5092f92c0e62b72c3bf6539ab62a79fefc836395" "1" "{u:425}:pd" "{u:425}:pd" "$.logdt" "\"28-08-2023 12:59:45\""
1693207784.977812 [0 122.160.68.144:56816] "ping"
1693207786.137807 [0 122.160.68.144:57986] "EVALSHA" "5092f92c0e62b72c3bf6539ab62a79fefc836395" "1" "{u:425}:pd" "{u:425}:pd" "$.logdt" "\"28-08-2023 12:59:46\""
1693207788.525797 [0 122.160.68.144:57989] "PING"
1693207788.537797 [0 122.160.68.144:57988] "PING"

Below are MONITOR logs with Transaction:

1693207779.461834 [0 122.160.68.144:58003] "PING"
1693207779.757833 [0 122.160.68.144:57986] "MULTI"
1693207779.757833 [0 122.160.68.144:57986] "EVAL" "MY_LUA_SCRIPT"
1693207779.757833 [0 122.160.68.144:57986] "SELECT" "0"
1693207779.757833 [0 122.160.68.144:57986] "EXEC"
1693207780.473830 [0 122.160.68.144:58004] "PING"
1693207781.029828 [0 122.160.68.144:57986] "SELECT" "0"
1693207781.029828 [0 122.160.68.144:57986] "MULTI"
1693207781.029828 [0 122.160.68.144:57986] "EVAL" "MY_LUA_SCRIPT"
1693207781.029828 [0 122.160.68.144:57986] "SELECT" "0"
1693207781.029828 [0 122.160.68.144:57986] "EXEC"
1693207782.309823 [0 122.160.68.144:57986] "SELECT" "0"
1693207782.309823 [0 122.160.68.144:57986] "MULTI"
1693207782.309823 [0 122.160.68.144:57986] "EVAL" "MY_LUA_SCRIPT"
1693207782.309823 [0 122.160.68.144:57986] "SELECT" "0"
1693207782.309823 [0 122.160.68.144:57986] "EXEC"
1693207783.293819 [0 122.160.68.144:57970] "PING"

Extension function _JsonSCSetAsync:

public static async Task<bool> JsonSC_SetAsync(this ITransaction _db, RedisKey a_key, string a_path, object a_value, CommandFlags a_flags = CommandFlags.None)
{
    List<RedisValue> list = new List<RedisValue> { a_path, (RedisValue)JsonConvert.SerializeObject(a_value) };
    LuaScript l_sc = LuaScript.Prepare(SCRIPT__SET);
    RedisResult l_rr = (await _db.ScriptEvaluateAsync(l_sc, new { key = a_key, path = a_path, value = JsonConvert.SerializeObject(a_value) }));
    return l_rr.OKtoBoolean();
}

My test function for WITH Transaction:

async void TestMeWithTransaction()
{
    await Task.Delay(10000);
    Logger.Error("WithTransaction--START");
    RedisConnection l_conn = RedisHandlerCommands.Default.GetConnection();
    ITransaction l_trans = l_conn.Database.CreateTransaction();

    l_trans.JsonSC_SetAsync("{u:425}:pd", "$.logdt", DateTime.Now.ToString());
    bool l_isSuccess = await l_trans.ExecuteAsync();
    Logger.Error("IsSuccess: {l_isSuccess}", l_isSuccess);

    await Task.Delay(5000);
    l_trans.JsonSC_SetAsync("{u:425}:pd", "$.logdt", DateTime.Now.ToString());
    l_isSuccess = await l_trans.ExecuteAsync();
    Logger.Error("IsSuccess: {l_isSuccess}", l_isSuccess);

    await Task.Delay(5000);
    l_trans.JsonSC_SetAsync("{u:425}:pd", "$.logdt", DateTime.Now.ToString());
    l_isSuccess = await l_trans.ExecuteAsync();
    Logger.Error("IsSuccess: {l_isSuccess}", l_isSuccess);
    Logger.Error("WithTransaction--END");
    TestMeNoTransaction();
}

My test function for WITH NO Transaction:

async void TestMeNoTransaction()
{
    await Task.Delay(1000);
    Logger.Error("NoTransaction--START");
    RedisConnection l_conn = RedisHandlerCommands.Default.GetConnection();

    bool l_isSuccess = await l_conn.Database.JsonSC_SetAsync("{u:425}:pd", "$.logdt", DateTime.Now.ToString());         
    Logger.Error("IsSuccess: {l_isSuccess}", l_isSuccess);

    await Task.Delay(1000);
    l_isSuccess = await l_conn.Database.JsonSC_SetAsync("{u:425}:pd", "$.logdt", DateTime.Now.ToString());          
    Logger.Error("IsSuccess: {l_isSuccess}", l_isSuccess);

    await Task.Delay(1000);
    l_isSuccess = await l_conn.Database.JsonSC_SetAsync("{u:425}:pd", "$.logdt", DateTime.Now.ToString());          
    Logger.Error("IsSuccess: {l_isSuccess}", l_isSuccess);
    Logger.Error("NoTransaction--STARTEND");
}
mgravell commented 1 year ago

Right; so, the way the SHA caching works here is that we only update the cache state once we have seen the response from SCRIPT LOAD that confirms that it was indeed: loaded. And the nature of MULTI is that no responses are sent until the EXEC - hence we can't have updated the cache state until after the tran. Maybe load the script once outside the transaction, and it should work fine?

AppsoleutSatbir commented 1 year ago

Hi @mgravell Thanks for the suggession. I have managed to use EVALSHA in transaction using below example. Using EVALSHA in Transaction is only working when sending hash byte[] directly(Received from SCRIPT LOAD). I had tried LuaScript and LoadedLuaScript, but didnt work.

TEMP_SCRIPT:--- "return redis.call(\"JSON.GET\", KEY[1], ARGV[1])";

Getting MasterServer:---

public static IServer GetMasterServer(IConnectionMultiplexer a_mulex)
{
    IServer l_selectedServer = null;
    foreach (IServer l_server in a_mulex.GetServers())
    {
        if (l_server.IsReplica || !l_server.IsConnected) continue;
        l_selectedServer = l_server;
        break;
    }
    return l_selectedServer;
}
async void TestMeWithTransaction()
{
    RedisConnection l_conn = RedisHandlerCommands.Default.GetConnection();

    IServer l_server = JsonCommandScripts.GetMasterServer(l_conn.Mulux);
        bytes[] l_bytesHash = await l_server.ScriptLoadAsync(JsonCommandScripts.TEMP_SCRIPT);

       ITransaction l_trans = l_conn.Database.CreateTransaction();
       l_trans.ScriptEvaluateAsync(l_bytesHash , new RedisKey[] { "{Z:CPDS.1}:rt:Global:R:309" }, new RedisValue[] { "$" });
       l_isSuccess = await l_trans.ExecuteAsync(); **//TRUE. Using EvalSHA**
}

@mgravell Another question is, now that I am loading script directly into the IServer (Master Server). In case of:

  1. Server gets changed?
  2. Script gets flush dues to low memory? I am receiving error: "Message: NOSCRIPT No matching script. Please use EVAL...." from Redis.

Do I have to check for "SCRIPT EXIST" everytime before calling the script?

AppsoleutSatbir commented 1 year ago

@NickCraver @mgravell Any suggession on my last message?

mgravell commented 1 year ago

hmm; actually, it looks like there is an issue here; the library fails to unroll nested IMultiMessage; checking what we can do there

mgravell commented 1 year ago

re the unload: basically, there is no perfect solution if your scripts might get unloaded, but I recommend not using the manual SHA hash approach - that has the highest chance of failure. So; assuming we aren't using that, if you never want to see failure, you can use the CommandFlags.NoScriptCache hint, which will tell the library to always use EVAL (never EVALSHA); this will take more bandwidth, but will never hit this problem.

If you tell SE.Redis the full script, but don't add the NoScriptCache hint, then it will at least recover automatically if it sees failure - meaning: it will update the script cache and automatically reload the script as needed. Any failure will at least only be transient (usually only a single failure). There is currently no broadcast of "I unloaded {X}", so we can't be more active than that.

In reality, I almost never recommend using the LuaScript type; I literally always just use the string version; honestly, I'm tempted to just deprecate that type - thoughts @NickCraver ?

but as I say: I'll see if I can unbreak the underlying issue here, and give you a version that does work somewhat reliably

mgravell commented 1 year ago

OK, I've investigated, and the short version is "yes, it definitely does what you say"; it would be really, really hard to change this; let's keep this open, but: it isn't a quick fix

mgravell commented 1 year ago

On reflection: I don't think we should try to resolve this. Reasons: Lua script is meant to replace MULTI/EXEC. Anything you're currently doing in MULTI/EXEC you can do via Lua, but: better.

I don't think it makes sense to nest a Lua call inside MULTI/EXEC; instead, the entire MULTI/EXEC should be replaced with a single Lua call.

Can I ask for more context on your reasons for wanting Lua inside MULTI/EXEC? I suspect (although I'm open to learning) that it'll turn out to be that I'm not wrong on this - maybe I can convince you :)

AppsoleutSatbir commented 1 year ago

@mgravell. Thanks for your quick assessment & response.

Actually, my intention was to use RedisJSON's ITransaction.JsonSetAsync(KEY, PATH, VALUES....). But I found an issue with JsonSetAsync(....) is that it needs the path to be existed when setting a member.

For example, to set a member "D" at PATH "A.B.C.D"=10, JsonSetAsync(....) need "B" and "C" to exist in hierarchy first. To overcome this issue, I have written our own extension method ITransaction.JsonSetAsync_P(...) (Postfix '_P' means PATH functions) & SCRIPT which:

  1. tries to set the member first using JsonSetAsync(....)
  2. and upon fail, it creates the hierarchy objects first and call JsonSetAsync(...) again.

Because JsonSetAsync(....) works in Transaction, I wanted the our JsonSetAsync_P(...) to get execute in Transaction as well.

What would you suggest next?

Thanks for your efforts!!

AppsoleutSatbir commented 12 months ago

@mgravell Any update on my last message?

mgravell commented 12 months ago

Sorry, lots of plates spinning. What exact package and method are you using here? Can you link me? I couldn't find any such methods that depended on ITransaction; NReJson depends only on IDatabase[Async].