http://ayende.comAyende @ RahienCopyright (C) Ayende Rahien 2004 - 2021 (c) 202660Andrew, That would require me to implement a reset logic, something which would probably be quite complicated. http://ayende.com/4049/connection-pooling-implemention#comment8http://ayende.com/4049/connection-pooling-implemention#comment8Sun, 07 Jun 2009 04:58:00 GMTRepository (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. http://ayende.com/4049/connection-pooling-implemention#comment7http://ayende.com/4049/connection-pooling-implemention#comment7Sun, 07 Jun 2009 04:46:32 GMTStephen, 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 http://ayende.com/4049/connection-pooling-implemention#comment6http://ayende.com/4049/connection-pooling-implemention#comment6Sat, 06 Jun 2009 22:08:26 GMTMarc, 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 http://ayende.com/4049/connection-pooling-implemention#comment5http://ayende.com/4049/connection-pooling-implemention#comment5Sat, 06 Jun 2009 22:00:39 GMTBy '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. http://ayende.com/4049/connection-pooling-implemention#comment4http://ayende.com/4049/connection-pooling-implemention#comment4Sat, 06 Jun 2009 21:14:09 GMTI'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. http://ayende.com/4049/connection-pooling-implemention#comment3http://ayende.com/4049/connection-pooling-implemention#comment3Sat, 06 Jun 2009 19:14:25 GMTOh, the joys of naming guidelines gone berserk. http://ayende.com/4049/connection-pooling-implemention#comment2http://ayende.com/4049/connection-pooling-implemention#comment2Sat, 06 Jun 2009 17:56:47 GMTPooledDistributedHashTableStorageClientConnection? Try discussing that class on Twitter. :-P http://ayende.com/4049/connection-pooling-implemention#comment1http://ayende.com/4049/connection-pooling-implemention#comment1Sat, 06 Jun 2009 17:36:52 GMT