Blocking wait in transaction execution

[rayao]

It's unexpected to me when all methods I call are async, is there any setting to tune to avoid this blocking wait? I also found similar stack in TakeLock/ReleaseLock calls, seems they're implemented upon transactions. We have a service exposes APIs that do Redis lock take/release, we're afraid that when request rate increases, blocking wait will put the service in danger.

[mgravell]

What is the scenario here? What exactly is your code doing? Is this a transaction with constraints, perhaps? If so: that may be a factor here - I'm a little surprised it didn't offload the entire thing to the write loop, though.

I'm right now in the middle of a big overhaul of the write loop, which may help with the latter, but there will always be a pinch point using transactions with constraints, do to (grungy details that aren't very interesting). However, in most cases Lua will be a much easier, simpler, and more direct mechanism for constrained batches - I wonder if refactoring to use a Lua script would be the best option.

Without example code, it is hard to say more.

[rayao]

Code corresponds to stack above

            var transaction = cache.CreateTransaction();
            if (comparand.HasValue)
            {
                transaction.AddCondition(Condition.HashEqual(key, VersionField, comparand.Version));
            }
            else
            {
                transaction.AddCondition(Condition.HashNotExists(key, VersionField));
            }

            var entries = new HashEntry[]
            {
                new HashEntry(ValueField, value),
                new HashEntry(VersionField, version.Version),
            };
            var redisKey = key;

            transaction.HashSetAsync(redisKey, entries);
            transaction.KeyExpireAsync(redisKey, expiry);
            return transaction.ExecuteAsync();

[rayao]

And even more simple with *Lock methods

        public Task<bool> ExtendLock(string formId)
        {
            return _redisCache.ExtendLock(
                $"{ExcelLockFormat}-{formId}",
                _contextProvider.User.UserSessionId,
                SdxEventLockDuration);
        }

[rayao]

Seems there's a lock to take to execute the transaction. Maybe the entire transaction should be queued to run in write loop? Is it possible to replace *Lock with Lua scripts?

[mgravell]

Yes, that is suitable for Lua. Not at PC right now, but will try to provide a translation when I am at desk.

[aniprasad]

Hi @mgravell Would you happen to have any updates on this?

Would something like this work? for taking the lock, extending the lock, and releasing the lock respectively

  const string TakeLock = @"
      local lockDuration = tonumber(ARGV[4])
      local setResult = redis.call('SET', KEYS[1], ARGV[1], ARGV[2], ARGV[3], lockDuration)
      return setResult";
      string[] keys = new string[] { "key" };
      string[] args = new string[] {
        "value",
        "NX",
        "EX",
        duration
  };

const string ExtendLock = @"
    local session = redis.call('GET', KEYS[1])
    if session == ARGV[1]
    then
        local lockDuration = tonumber(ARGV[2])
        return redis.call('SET', KEYS[1], ARGV[1], 'EX', lockDuration)
    end
    return session";
    string[] keys = new string[] { "key };
    string[] args = new string[]
    {
        "value"
        duration
    };

const string ReleaseLock = @"
    local session = redis.call('GET', KEYS[1])
    if session == ARGV[1]
    then
        return redis.call('DEL', KEYS[1])
    end";
    string[] keys = new string[] { "key" };
    string[] args = new string[] { "value" };

I plan on using the ScriptEvaluateAsync method for these scripts like ScriptEvaluateAsync(script, keys, args, CommandFlags.None)

[mgravell]

Sorry, slipped off my plate; that looks broadly OK to me and is a preferable solution than MULTI/EXEC ; some nits:

• in ReleaseLock you might want to use UNLINK instead of DEL, but if we assume modest value (ARGV[1]) it won't matter hugely
• in ExtendLock you might just want to use EXPIRE rather than SET ... EX, to avoid having to rewrite the payload
• TakeLock can probably be replaces with the StringSet with When.NotExists, but it is fine as written too