On Tue, Aug 28, 2012 at 2:45 PM, Gary Tully <[email protected]> wrote: > the removeFirst/add will ensure that we iterate over the pooled > connections once we reach the pool limit but in the case of > maxConnections == 1 it is unnecessary. > A pool of one is not really a pool though! >
Whether you have 1 or N connections in the pool shouldn't matter, its the fact that the pool can re-use connections to avoid the cost of re-establishing connections each time the client needs to send a message. As its only the session that are stored in the Apache Commons Pool logic, then I don't see that the connection pooling is using any kind of grow/shrink pattern, where connections can be discarded if they become idle etc. Also I assume re-connection / keep-alive and whatnot of these connection is using the built-in failover transport of AMQ client. Anyway I see the point. So if you specify maxConnections=50, then the activemq-pool will *always* create 50 connections, which cannot shrink or grow on demand (not considering the first 50 messages will create a new connection if the pool is not reached max yet). Isn't this a problem as you will then always take up 50 physical connections, even if you have a very low messaging usage, (lets say you send 1 message per minute). Then you will round robin among those 50 physical connections. And what if there is a problem creating a connection, in establishing a physical connection to the broker? How would the pool be able to "handle" this, and use on of the existing connections that already works? > On 28 August 2012 12:31, Claus Ibsen <[email protected]> wrote: >> Hi >> >> I have been working on a patch for >> https://issues.apache.org/jira/browse/AMQ-3997 >> >> And stumbled across some code in activemq-pool I cannot understand the >> reason. I guess my coffee isn't strong enough. So I am asking here on >> @dev first. >> >> Its the code in createConnection method in PooledConnectionFactory. I >> am pasting it below, and marking a **** where I am puzzled >> >> >> public synchronized Connection createConnection(String userName, >> String password) throws JMSException { >> if (stopped.get()) { >> LOG.debug("PooledConnectionFactory is stopped, skip create >> new connection."); >> return null; >> } >> >> ConnectionKey key = new ConnectionKey(userName, password); >> LinkedList<ConnectionPool> pools = cache.get(key); >> >> if (pools == null) { >> pools = new LinkedList<ConnectionPool>(); >> cache.put(key, pools); >> } >> >> ConnectionPool connection = null; >> if (pools.size() == maxConnections) { >> ********* >> connection = pools.removeFirst(); >> } >> >> // Now.. we might get a connection, but it might be that we need to >> // dump it.. >> if (connection != null && connection.expiredCheck()) { >> connection = null; >> } >> >> if (connection == null) { >> ActiveMQConnection delegate = createConnection(key); >> connection = createConnectionPool(delegate); >> } >> pools.add(connection); >> return new PooledConnection(connection); >> } >> >> 1) >> If you look at the ****** spot in the code above. Then the >> ConnectionPool connection instance only get set if the pools.size() == >> maxConnections. So if this condition is false, then we never get a >> pooled connection, and would always have to create a new connection, >> in the code just below. >> >> Now a reason this may not have spotted before, is that the default >> value for maxConnections is 1. And therefore the condition is true in >> those. Then we remove the pooled connection, which mean the pools size >> is one less (= zero). But then just before existing the method, we add >> it back to the pools, so the size will become 1 again. >> (Now this code is a bit inefficient as we will keep removing and >> adding the same connection over and over again.) >> >> So my question is also, what should happen when the maxConnections is >> larger than 1, eg maxConnections=8. Then we would create 8 connections >> and put in the pools. But only the first in the pools will ever be >> used (eg removeFirst). Notice the method is synchronized. >> >> >> 2) >> Another issues is that we wrap the connection in a PooledConnection, >> eg in the last code line. And we use the new constructor. Which mean a >> new instance is always created. And creating a new instance of >> PooledConnection is not cheap, as it has 2 internal lists. And any >> list/map is a bit expensive to create in java, especially the >> concurrent ones. >> >> During my hunt for AMQ-3997 I noticed a lot of instances of concurrent >> maps/lists and their internal instances such as $Node etc being >> created. We may optimize this further to gain speed. It seems this >> code is 10% slower than the spring cached pool (a very rough estimate >> based on times outputted from that test code, with the link from >> AMQ-3997). >> >> >> >> Anyway I guess I am going for stronger coffee. >> >> >> >> >> >> -- >> Claus Ibsen >> ----------------- >> FuseSource >> Email: [email protected] >> Web: http://fusesource.com >> Twitter: davsclaus, fusenews >> Blog: http://davsclaus.com >> Author of Camel in Action: http://www.manning.com/ibsen > > > > -- > http://fusesource.com > http://blog.garytully.com -- Claus Ibsen ----------------- FuseSource Email: [email protected] Web: http://fusesource.com Twitter: davsclaus, fusenews Blog: http://davsclaus.com Author of Camel in Action: http://www.manning.com/ibsen
