Connection Pooling

I was curious about trying to write my own simple Database Connection Pool, where all the responsibility for freeing resources belongs to objects which use those connections.

There are:

3 classes - ConnectionPool, RecoverableConnection and ConnectionFactory

1 interface - IrresponsibleConnectionManager

Is my code (my thoughts) acceptable ? Or is it a complete junk ? It took me an hour and half to write and I know there are several better algorithms to implement. But this could server well for my pourpose. I also found this solution faster to some libraries I had downloaded, even faster than DataSource.

I'm glad to hear any comments about my code !

import java.sql.Connection;
import java.sql.SQLException;
import java.util.Properties;

/**
 * @author XY
 */
public class ConnectionFactory {

    private static ConnectionPool connectionPool;

    public static void initializeConnectionPool(String url, Properties connectionProperties, int connectionPoolSize) throws SQLException {

        if (connectionPool == null) {
            connectionPool = new ConnectionPool(url, connectionProperties, connectionPoolSize);
        }
    }

    /**
     * Returns a connection from connection pool.
     *
     * @param waitTime Time to wait for a connection.
     * @return Connection to the database.
     * @throws java.sql.SQLException When connection is not available within given wait time.
     */
    public static Connection getConnection(ConnectionPool.WaitTime waitTime) throws SQLException {
        return connectionPool.getConnection(waitTime);
    }
}

```
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.util.*;

/**
* Offers unified way to handle database connections. Provides connection pooling functionality.
*
* @author XY
* Date: 13.5.13
*/
public class ConnectionPool implements IrresponsibleConnectionPoolManager {

private int pool_size = 0;
private Queue availableConnections;

/**

1) ConnectionFactory.initializeConnectionPool() is not thread-safe since two threads could see the connectionPool as null and set it at the same time. The first one will be "lost" and garbage collected, so it is not catastrophic, but it should probably be thread-safe. I did not check other classes for threadsafety, but they might also have issues.

2) ConnectionFactory.getConnection() should probably return some useful error message if initializeConnectionPool() had not been called before. It would actually make more sense to replace initializeConnectionPool() by the constructor. In such a case you would not use the singleton pattern, since you might have many instances of ConnectionFactory (hopefully to different databases).

3) The constructor ConnectionPool() calls initializeConnections() before setting pool_size, but it should be the other way around

4) Instead of using a Queue in ConnectionPool, you should use a BlockingQueue. They have poll and offer methods that take a time out. You somewhat recreated that behavior with thread.sleep. However thread.sleep can sometimes wake up before its intended time and it is in general preferable to use existing classes that provide some functionality since you know they are bulletproof.

5) It does not smell quite right that RecoverableConnection contains a connection manager as a member. That class is very long but just seems to redefine all Connection methods. It is probably better to get rid of RecoverableConnection and just use normal Connections.

I have not read all your classes, but I hope those comments will be useful.

StackExchange Code Review Q#26139, answer score: 2