CODEWITHSUNDEEP
azureredisservicestack.redisazure-redis-cache
Peter Kellner
Peter Kellner

Reputation: 15478

Redis Connections May Not be Closing with c#

I'm connecting to Azure Redis and they show me the number of open connections to my redis server. I've got the following c# code that encloses all my Redis sets and gets. Should this be leaking connections?

       using (var connectionMultiplexer = ConnectionMultiplexer.Connect(connectionString))
        {

            lock (Locker)
            {
                redis = connectionMultiplexer.GetDatabase();
            }

            var o = CacheSerializer.Deserialize<T>(redis.StringGet(cacheKeyName));
            if (o != null)
            {
                return o;
            }
            lock (Locker)
            {
                // get lock but release if it takes more than 60 seconds to complete to avoid deadlock if this app crashes before release
                //using (redis.AcquireLock(cacheKeyName + "-lock", TimeSpan.FromSeconds(60)))

                var lockKey = cacheKeyName + "-lock";
                if (redis.LockTake(lockKey, Environment.MachineName, TimeSpan.FromSeconds(10)))
                {
                    try
                    {
                        o = CacheSerializer.Deserialize<T>(redis.StringGet(cacheKeyName));
                        if (o == null)
                        {
                            o = func();
                            redis.StringSet(cacheKeyName, CacheSerializer.Serialize(o),
                                TimeSpan.FromSeconds(cacheTimeOutSeconds));
                        }
                        redis.LockRelease(lockKey, Environment.MachineName);
                        return o;
                    }
                    finally
                    {
                        redis.LockRelease(lockKey, Environment.MachineName);
                    }
                }
                return o;
            }

        }
    }

Upvotes: 6

Views: 7665

Answers (2)

David Burg
David Burg

Reputation: 1163

I was suggesting try using Close (or CloseAsync) method explicitly. In a test setting you may be using different connections for different test cases and not want to share a single multiplexer. A search for public code using Redis client shows a pattern of Close followed by Dispose calls.

Noting in the XML method documentation of Redis client that close method is described as doing more:

    //
    // Summary:
    //     Close all connections and release all resources associated with this object
    //
    // Parameters:
    //   allowCommandsToComplete:
    //     Whether to allow all in-queue commands to complete first.
    public void Close(bool allowCommandsToComplete = true);
    //
    // Summary:
    //     Close all connections and release all resources associated with this object
    //
    // Parameters:
    //   allowCommandsToComplete:
    //     Whether to allow all in-queue commands to complete first.
    [AsyncStateMachine(typeof(<CloseAsync>d__183))]
    public Task CloseAsync(bool allowCommandsToComplete = true);

...

//
// Summary:
//     Release all resources associated with this object
public void Dispose();

And then I looked up the code for the client, found it here:

https://github.com/StackExchange/StackExchange.Redis/blob/master/src/StackExchange.Redis/ConnectionMultiplexer.cs

And we can see Dispose method calling Close (not the usual override-able protected Dispose(bool)), further more with the wait for connections to close set to true. It appears to be an atypical dispose pattern implementation in that by trying all the closure and waiting on them it is chancing to run into exception while Dispose method contract is supposed to never throw one.

Upvotes: 0

Vladimir Dorokhov
Vladimir Dorokhov

Reputation: 3839

You can keep connectionMultiplexer in a static variable and not create it for every get/set. That will keep one connection to Redis always opening and proceed your operations faster.

Update: Please, have a look at StackExchange.Redis basic usage: https://github.com/StackExchange/StackExchange.Redis/blob/master/Docs/Basics.md

"Note that ConnectionMultiplexer implements IDisposable and can be disposed when no longer required, but I am deliberately not showing using statement usage, because it is exceptionally rare that you would want to use a ConnectionMultiplexer briefly, as the idea is to re-use this object."

It works nice for me, keeping single connection to Azure Redis (sometimes, create 2 connections, but this by design). Hope it will help you.

Upvotes: 5

Related Questions