Ayende @ Rahien

My name is Oren Eini
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:


+972 52-548-6969

, @ Q c

Posts: 10 | Comments: 36

filter by tags archive

Connection Pooling: Implemention

time to read 7 min | 1376 words

Given the following contact:

/// <summary>
/// Thread Safety - This is NOT a thread safe connection
/// Exception Safety - After an exception is thrown, it should be disposed and not used afterward
/// Connection Pooling - It is expected that this will be part of a connection pool
/// </summary>
public class DistributedHashTableStorageClient

I decided that I needed to really didn’t want to pass the responsibility for that to the client, and that I wanted to handle that inside my library. Here is what I came up with:

public class DefaultConnectionPool
    private static readonly ILog log = LogManager.GetLogger(typeof (DefaultConnectionPool));
    readonly object locker = new object();

    private readonly Dictionary<NodeEndpoint, LinkedList<PooledDistributedHashTableStorageClientConnection>> pooledConnections =
        new Dictionary<NodeEndpoint, LinkedList<PooledDistributedHashTableStorageClientConnection>>();

    public IDistributedHashTableStorage Create(NodeEndpoint endpoint)
        PooledDistributedHashTableStorageClientConnection storage = null;
        lock (locker)
            LinkedList<PooledDistributedHashTableStorageClientConnection> value;
            if (pooledConnections.TryGetValue(endpoint, out value) && value.Count > 0)
                storage = value.First.Value;
        if (storage != null)
            if (storage.Connected == false)
                log.DebugFormat("Found unconnected connection in the pool for {0}", endpoint.Sync);
                catch (Exception e)
                    log.Debug("Error when disposing unconnected connection in the pool", e);
                return storage;
        log.DebugFormat("Creating new connection in the pool to {0}", endpoint.Sync);
        return new PooledDistributedHashTableStorageClientConnection(this, endpoint);

    private void PutMeBack(PooledDistributedHashTableStorageClientConnection connection)
        lock (locker)
            LinkedList<PooledDistributedHashTableStorageClientConnection> value;
            if (pooledConnections.TryGetValue(connection.Endpoint, out value) == false)
                pooledConnections[connection.Endpoint] = value = new LinkedList<PooledDistributedHashTableStorageClientConnection>();
        log.DebugFormat("Put connection for {0} back in the pool", connection.Endpoint.Sync);

    class PooledDistributedHashTableStorageClientConnection : DistributedHashTableStorageClient
        private readonly DefaultConnectionPool pool;

        public PooledDistributedHashTableStorageClientConnection(
            DefaultConnectionPool pool,
            NodeEndpoint endpoint) : base(endpoint)
            this.pool = pool;

        public bool Connected
            get { return client.Connected; }

        public override void Dispose()
            if(Marshal.GetExceptionCode() != 0)//we are here because of some sort of error
                log.Debug("There was an error during the usage of pooled client connection, will not return it to the pool (may be poisioned)");
            else if(Connected == false)
                log.Debug("The connection was disconnected, will not return connection to the pool");

I think that should pretty much cover everything I need.



Tommy Carlier

PooledDistributedHashTableStorageClientConnection? Try discussing that class on Twitter. :-P


Oh, the joys of naming guidelines gone berserk.

Marc Brooks

I'm uncomfortable with the incestuous probing in Dispose for the Marshal.GetExceptionCode. Seems very fragile to have that implementation logic dependency. Would it be better to have a delegate wrapper that allows you to catch the actual exception that would have resulted and flag and handle it safely without that assumption?

My other, more vague, concern is the placing of the "partially setup" LinkedList in PutMeBack. It seems that you shouldn't be shoving an empty LinkedList into the pooled connection hashtable until you have added the connection to the list.

One option would be to set a boolean flag based on the TryGetValue failure, new up the LinkedList, and then fall through to the AddLast. Then, after the list is now fully valid, check the flag and shove the new list in the pooled connections if needed.

Better yet, construct if the TryGetValue failed, new the LinkedList, AddLast the connection and then shove the new list in the pooled connections. Then the else-clause of the TryGetValue is merely responsible for an AddLast call against an already-existing list.

Last concern, I'm troubled by the "hungarianish" naming of your connection hashtable. Seems like an awful lot of implementation details being called out in the name of the class... Unless you have tons of other kinds running around, I would suggest PooledClientConnection or similar intention-revealing (instead of implementation-declaring) naming.


By 'an exception being thrown'.. this would be similar to WCF where a client instance is dumped once its state is error'd? in which case it seems sensible that the connection should indicate if it is in an error'd state.. rather than odd tricks trying to find out if it is in error'd state or not.

Ayende Rahien


I agree that the Marshal trick isn't nice, but it is the only alternative to the delegate trick, and I am getting tired of recursive delegates tricks.

Partially setup? I am not sure that I understand. It is perfectly legit to have an empty list in the pool.

What is the implementation about the class name?It is implementing DistributedHashTableStorageClient, and the rest is just what it is.

Client has no meaning in the app

Ayende Rahien


No, the idea is that if I am getting an error when using this, I am not sure what the state of the server is, so I am dumping the connection.

There is no easy way for me to discover that, so an error will trigger the dumping.

And yes, I could have done this differently, but that would mean that I would have to:

a) modify the base class

b) do more work

This is actually simpler than all the alternatives, and it produces a "don't need to think about it" semantics for the user


Repository (as in database) pooling logic (such as MySQL .Net Connector, and MS SQL's connector) keep a pool of connections too.

Prior to handing a connection to the client (or when its put back, I forget which), the connection is 'reset', that clears all session state etc information to make sure the pooled connection being returned is 'fresh'.

If an SQL error occurs when someone uses a connection, it can still be disposed of and put back in the pool - because its reset when given to another client.

That could be an alternative to deciding whether to re-pool the connection based upon whether the previous disposal was because of an error.

Ayende Rahien


That would require me to implement a reset logic, something which would probably be quite complicated.

Comment preview

Comments have been closed on this topic.


  1. Production postmortem: The case of the memory eater and high load - about one day from now
  2. Production postmortem: The case of the lying configuration file - 3 days from now
  3. Production postmortem: The industry at large - 4 days from now
  4. The insidious cost of allocations - 5 days from now
  5. Find the bug: The concurrent memory buster - 6 days from now

And 4 more posts are pending...

There are posts all the way to Sep 10, 2015


  1. Find the bug (5):
    20 Apr 2011 - Why do I get a Null Reference Exception?
  2. Production postmortem (10):
    14 Aug 2015 - The case of the man in the middle
  3. What is new in RavenDB 3.5 (7):
    12 Aug 2015 - Monitoring support
  4. Career planning (6):
    24 Jul 2015 - The immortal choices aren't
View all series


Main feed Feed Stats
Comments feed   Comments Feed Stats