Unexpected reply on high volume scenario using ServiceStack.Redis

Question

My problem is very similar to this one: Protocol errors, "no more data" errors, "Zero length response" errors while using servicestack.redis in a high volume scenario

I'm using ServiceStack v3.9.54.0 in a C# web application working on IIS. I could see the errors in both Redis versions 2.8.17 and 3.0.501.

The errors I've been receiving are the following:

ServiceStack.Redis.RedisResponseException: Unexpected reply: +PONG, sPort: 65197, LastCommand: GET EX:KEY:230
at ServiceStack.Redis.RedisNativeClient.CreateResponseError(String error)
at ServiceStack.Redis.RedisNativeClient.ParseSingleLine(String r)
at ServiceStack.Redis.RedisNativeClient.ReadData()
at ServiceStack.Redis.RedisNativeClient.SendExpectData(Byte[][] cmdWithBinaryArgs)
at ServiceStack.Redis.RedisNativeClient.GetBytes(String key)
at ServiceStack.Redis.RedisNativeClient.Get(String key)

And:

ServiceStack.Redis.RedisResponseException: Unknown reply on integer response: 43PONG, sPort: 59017, LastCommand: EXISTS EX:AnKey:Cmp6
at ServiceStack.Redis.RedisNativeClient.CreateResponseError(String error)
at ServiceStack.Redis.RedisNativeClient.ReadLong()
at ServiceStack.Redis.RedisNativeClient.SendExpectLong(Byte[][] cmdWithBinaryArgs)
at ServiceStack.Redis.RedisNativeClient.Exists(String key)
at Redis.Documentos.RedisBaseType.Exists(String key)

The first thing that I thought was that I was sharing the Redis Connection across multiple threads, but I can't see the problem on my singleton implementation of the PooledRedisClientManager (Configs is a static class that stores the connection information):

public class RedisProvider
{
    public PooledRedisClientManager Pool { get; set; }
    private RedisProvider()
    {

        var srv = new List<string> { $"{Configs.Server}:{Configs.Port}" };

        Pool = new PooledRedisClientManager(srv, srv, null, 
            Configs.Database, Configs.PoolSize, Configs.PoolTimeout);
    }

    public IRedisClient GetClient()
    {
        try
        {
            var connection = (RedisClient)Pool.GetClient();
            return connection;
        }
        catch (TimeoutException)
        {
            return null;
        }
    }

    private static RedisProvider _instance;
    public static object _providerLock = new object();
    public static RedisProvider Provider
    {
        get
        {
            lock (_providerLock)
            {
                if (_instance == null)
                {
                    var instance = new RedisProvider();
                    _instance = instance;
                    return _instance;
                }
                else
                {

                    return _instance;
                }
            }
        }
    }

}

All the clients are obtained through the pool, as follows:

var redis = (RedisClient)RedisProvider.Provider.GetClient();

I'm sure that the redis var is not shared across multiple threads and, as far as I can see, this code shows a proper thread-safe implementation...

Any help would be much appreciated.

---

Edit: As per some technologies that I use, I have no access to the App Startup code nor can use using blocks. So, I wrap all clients like that:

RedisClient redis;
try {
    redis = (RedisClient)RedisProvider.Provider.GetClient();
    // Do stuff
} finally {
    redis.Dispose();
}

Answer 1

This error message is an indication the same redis client instance is being shared across multiple threads, the source code provided doesn't provide any verification that it's not.

The above RedisProvider is just a more verbose version of access wrapped around a singleton, e.g:

public static class RedisProvider
{
    public static IRedisClientManager Pool { get; set; }

    public static RedisClient GetClient()
    {
        return (RedisClient)Pool.GetClient();
    }
}

The RedisManager only needs to be initialized once on App Startup:

var srv = new List<string> { $"{Configs.Server}:{Configs.Port}" };
RedisProvider.Pool = new PooledRedisClientManager(srv, srv, null, 
    Configs.Database, Configs.PoolSize, Configs.PoolTimeout);

From then on, the verbose locking just adds overhead and doesn't provide any thread-safety benefits over accessing the Singleton RedisManager directly.

Whilst resolving the client is ThreadSafe:

var redis = RedisProvider.GetClient();

The redis client instance returned is not Thread-Safe (as per .NET conventions). As a result you need to make sure you're not sharing the same instance across multiple threads, you also need to ensure the client is disposed after use.

To ensure that it is accessed and disposed in the same thread, you should wrap the client usage in a using statement:

using (var redis = RedisProvider.GetClient())
{
    //...
} 

If you do this whenever you need to use the RedisClient and don't share the same client instance in a different background thread, async task, parallelized code, etc you should no longer have any multi-threading issues. When you need a new client instance in a different thread you should use the same access pattern and retrieve (and dispose) a separate client instance from the pool.