Signalr: Redis Backplane failure

[Barbu666]

Hi,

We're running 3 Azure web role backed by a Redis backplane in production. It's been running very smoothly in the past month but yesterday the backplane went "down" and this error was all over the place in the 3 servers log. Rebooting the servers fixed the problem.

ERR Error running script (call to f_c883e7b82acc1c2a0bb1f2ee805f928afa1c1abb): @user_script:2: user_script:2: attempt to compare boolean with string

at Microsoft.AspNet.SignalR.Messaging.ScaleoutStream.Send(Func`2 send, Object state)
at Microsoft.AspNet.SignalR.Infrastructure.Connection.Send(ConnectionMessage message)
at Microsoft.AspNet.SignalR.Hubs.HubDispatcher.Outgoing(IHubOutgoingInvokerContext context)
at Microsoft.AspNet.SignalR.Hubs.HubPipelineModule.<>c__DisplayClass1b.b__19(IHubOutgoingInvokerContext context)
at Microsoft.AspNet.SignalR.Hubs.HubPipeline.Send(IHubOutgoingInvokerContext context)
at Microsoft.AspNet.SignalR.Hubs.SignalProxy.TryInvokeMember(InvokeMemberBinder binder, Object[] args, Object& result)

Any idea what could have caused this?

Thanks

[Jehele]

I use Redis 2.8.12, SignalR with Redis 2.2.0
I found that simple restart of Redis windows service leads to the same and it seems that problem is in RedisConnection.RestoreLatestValueForKey. After a change in the lua script, it seems to work (I investigate it using a simple web project).

Does it make sense? Is the script correct?

new version

var redisResult = await _connection.GetDatabase(database).ScriptEvaluateAsync(
                   @"
                    local newvalue=-1
                    if redis.call('EXISTS', KEYS[1]) == 1 then 
                        newvalue = tonumber(redis.call('GET', KEYS[1]))
                    end
                    if newvalue < tonumber(ARGV[1]) then
                        return redis.call('SET', KEYS[1], ARGV[1])
                    else
                        return nil
                    end",
         new RedisKey[] { key },
         new RedisValue[] { _latestMessageId });

Original version:

var redisResult = await _connection.GetDatabase(database).ScriptEvaluateAsync(
                   @"local newvalue = redis.call('GET', KEYS[1])
                    if newvalue < ARGV[1] then
                        return redis.call('SET', KEYS[1], ARGV[1])
                    else
                        return nil
                    end",
                   new RedisKey[] { key },
                   new RedisValue[] { _latestMessageId });

[josh-endries]

Any idea when this change will get released? We just ran into this in prod as well.

[AndyMDoyle]

Any ideas when this fix will be released?

[josh-endries]

We actually abandoned using Redis because of this issue.

[moozzyk]

We would like to take it for 2.2.1 but I have a question with regards to the PR. Is there any specific reason why there is an explicit check if the key exists instead of just checking the value returned from GET?

[moozzyk]

Fixed in ece16d8f56f0d586f0aad18f525b8b42045d107d and fba976ea9a617a32bb3c4450ea7ca1f33c21d7fd

[scruffyllama]

@moozzyk , I think your commit perpetuates the part of the exception listed above... " attempt to compare boolean with string". You removed the "tonumber" casting, so now it can still fail in comparing a string to a number, can't it? We are seeing this exception all the time where I work, and we use version 2.2.1 (which is supposed to contain the fix for this issue).

[moozzyk]

@davidocooper - when/how does this happen? I just tried and cannot repro this error.

[moozzyk]

@davidocooper - while I don't see the script blow up it still has an issue caused by missing tonumber(). I think it was not observed because this script is only run when reconnecting to Redis.

[scruffyllama]

@moozzyk - So this project is at my work. It is a large project, and difficult to test - it will take me a while to get to a point of testing/debugging the project to repro (if I even can in a local environment). But the bug I am working is worded "Whenever you log in or do an action that requires SingnalR to communicate" the exception mentioned above is thrown, verbatim. So with the little I know about redis/signalr, I do know that the "tonumber()" function would make the comparison work if the KEY ends up being a string and not a number value. So that's where I am. As soon as I get more info, I will add it here. Thanks for your responsiveness!

[moozzyk]

@davidocooper - If this is stated with "whenever..." then I think this is not the script this bug is about. The script in this bug is only executed when the connection to Redis is dropped for whatever reason and restored (i.e. reconnect). It shouldn't happen frequently. There is one more script: https://github.com/SignalR/SignalR/blob/799d9bc32524066344cb3656e5f28f2fd03ba9b3/src/Microsoft.AspNet.SignalR.Redis/RedisMessageBus.cs#L79-L84 . This one gets executed all the time and I think you might have an issue with this script. You could connect to your Redis db using a Redis client (e.g. redis-cli) and poke around. I would start from checking if the key is really a number. You could also just try executing this script from the cli and see if you can repro the error. My suspicion is that something else is using the same RedisKey as SignalR and is storing non-number values and when SigalR tries to execute the script the error happens (which is still better than silently taking the other value and incrementing it).