Database Connection Pooling not used properly throughout the code

Question

I am working on a legacy code and since we have major database connection problems (1000+ sleeping connections at all times) and such slow return results, I decided to look into our so called 'Database Pooling' architecture closely.

Apparently we have two Database.java and 2 ConnectionPool.java, one pair in web application module and the other pair in database module.

So it looks like this:

package mil.dlas.database;

  Database.java
  ConnectionPool.java

package ti.dlas;
  Database.java
  ConnectionPool.java

package ti.dlas

public synchronized void getConn(ErrorBean myError)
{
    try
    {
        try
        {
            if (this.webBasedConnection)
            {
                if(dataSource == null)
                {
                    dataSource = (DataSource) new InitialContext().lookup("java:comp/env/jdbc/mydb");
                }

                if(dataSource == null)
                {
                    dataSource = (DataSource) new InitialContext().lookup("java:comp/env/jdbc/mydb");
                }

                conn = dataSource.getConnection();
            }
            else
            {
                getConnInterfaceBased(myError);
            }
        }
        catch (NamingException e)
        {
            e.printStackTrace();
            getConnInterfaceBased(myError);
        }
        catch(SQLException e)
        {
            e.printStackTrace();
            getConnInterfaceBased(myError);
        }
    }
    catch(Exception ex)
    {
        ex.printStackTrace();
        getConnInterfaceBased(myError);
    }
}
public synchronized void insertOrUpdatePS(String query,Object[] param, ErrorBean eb)
{
        java.sql.PreparedStatement pStat = null;
        try
        {          
            getConn(eb);
            pStat = conn.prepareStatement(query);

            for(int i=0; i<param.length;i++)
            {
                pStat.setObject(i+1,param[i]);
            }
            iRowsUpdated = pStat.executeUpdate();
        }
        catch(SQLException e)
        {e.printStackTrace();}
        catch(Exception e)
        {e.printStackTrace();}
        finally {
            if(pStat!=null)            
                try {pStat.close();}
                catch(SQLException e){}            
            returnConn();
        }
}
/**ConnectionPool .java**/
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.util.Vector;

public class ConnectionPool implements Runnable, java.io.Serializable{
  private String driver, url, username, password;
  private int maxConnections;
  private boolean waitIfBusy;
  private Vector availableConnections, busyConnections;
  private boolean connectionPending = false;

  public ConnectionPool(String driver, String url,
                        String username, String password,
                        int initialConnections,
                        int maxConnections,
                        boolean waitIfBusy)
      throws SQLException {
    this.driver = driver;
    this.url = url;
    this.username = username;
    this.password = password;
    this.maxConnections = maxConnections;
    this.waitIfBusy = waitIfBusy;
    if (initialConnections > maxConnections) {
      initialConnections = maxConnections;
    }
    availableConnections = new Vector(initialConnections);
    busyConnections = new Vector();
    for(int i=0; i<initialConnections; i++) {
      availableConnections.addElement(makeNewConnection());
    }
  }

  public synchronized Connection getConnection()
      throws SQLException {
    if (!availableConnections.isEmpty()) {
      Connection existingConnection =
        (Connection)availableConnections.lastElement();
      int lastIndex = availableConnections.size() - 1;
      availableConnections.removeElementAt(lastIndex);

      if (existingConnection.isClosed()) {
        notifyAll(); 
        return(getConnection());
      } else {
        busyConnections.addElement(existingConnection);
        return(existingConnection);
      }
    } else {
      if ((totalConnections() < maxConnections) &&
          !connectionPending) {
        makeBackgroundConnection();
      } else if (!waitIfBusy) {
        throw new SQLException("Connection limit reached");
      }

      try {
        wait();
      } catch(InterruptedException ie) {}
      return(getConnection());
    }
  }

  private void makeBackgroundConnection() {
    connectionPending = true;
    try {
      Thread connectThread = new Thread(this);
      connectThread.start();
    } catch(OutOfMemoryError oome) {}
  }

  public void run() {
    try {
      Connection connection = makeNewConnection();
      synchronized(this) {
        availableConnections.addElement(connection);
        connectionPending = false;
        notifyAll();
      }
    } catch(Exception e) { }
  }

  private Connection makeNewConnection()
      throws SQLException {
    try {
        Class.forName(driver);
      Connection connection =
        DriverManager.getConnection(url, username, password);
      return(connection);
    } catch(ClassNotFoundException cnfe) {
      throw new SQLException("Can't find class for driver: " +
                             driver);
    }
  }

  public synchronized void free(Connection connection) {
    busyConnections.removeElement(connection);
    availableConnections.addElement(connection);
    notifyAll(); 
  }

  public synchronized int totalConnections() {
    return(availableConnections.size() +
           busyConnections.size());
  }
}

package mil.dlas.database;

public synchronized void getConn()
{
    try
    {
        try
        {
            if(dataSource == null)
            {
                dataSource = (DataSource) new InitialContext().lookup("java:comp/env/jdbc/mydb");
            }
            conn = dataSource.getConnection();
            webBasedConnection = true;
        }
        catch (NamingException e){getConnInterfaceBased}
        catch(SQLException e){getConnInterfaceBased();}
    }
    catch(Exception ex){getConnInterfaceBased();}
}    
public synchronized void insertOrUpdatePS(String query, Object[] param)
{
    java.sql.PreparedStatement pStat = null;
    String jobnumber = "";

    try
    {
        getConn();
        if(!webBasedConnection)
            conn = checkOut();

        pStat = conn.prepareStatement(query);

        for(int i=0; i<param.length;i++)
        {
            if (i==0)
                jobnumber = param[i].toString();

            pStat.setObject(i+1,param[i]);
        }

        int insertUpdateReturnCode = pStat.executeUpdate();
        ResultSet rs = pStat.getGeneratedKeys();
        if (rs.next())
        {
            ResultSetMetaData rsmd = rs.getMetaData();
            int colCount = rsmd.getColumnCount();
            String columnOneName = rsmd.getColumnName(1);

            for (int x = 1; x <= colCount; x++)
            {
                String key = rs.getString(x);
                this.autoIncrementedID = key;
            }
        }
        else
        {
            this.autoIncrementedID = "";
        }

    }
    catch(SQLException e){e.printStackTrace();}
    catch(Exception e){e.printStackTrace();}
    finally
    {
        if(pStat!=null)            
           try {pStat.close();}
            catch(SQLException e){}             
        if(this.webBasedConnection)
            returnConn();
        else
            checkIn(conn);
    }
}
package mil.dlas.database;

import java.util.*; 
import java.sql.*; 
public class ConnectionPool implements Runnable 
{      
    private int m_InitialConnectionCount = 3;   
    private Vector m_AvailableConnections = new Vector(); 
    private Vector m_UsedConnections = new Vector(); 
    private String m_URLString = null; 
    private String m_UserName = null;     
    private String m_Password = null;
    private Thread m_CleanupThread = null; 

    protected ConnectionPool(String urlString, String user, String passwd) throws SQLException 
    {  
        m_URLString = urlString; 
        m_UserName = user; 
        m_Password = passwd; 

        for(int cnt=0; cnt<m_InitialConnectionCount; cnt++) 
        { 
            m_AvailableConnections.addElement(getConnection()); 
        }         
        m_CleanupThread = new Thread(this); 
        m_CleanupThread.start(); 
    }     

    private Connection getConnection() throws SQLException
    { 
        return DriverManager.getConnection(m_URLString, m_UserName, m_Password); 
    } 

    public synchronized Connection checkout() throws SQLException 
    { 
        Connection newConnxn = null; 
        if(m_AvailableConnections.size() == 0) 
        { 
             newConnxn = getConnection();  
             m_UsedConnections.addElement(newConnxn); 
        } 
        else 
        {  
            newConnxn = (Connection)m_AvailableConnections.lastElement();
            m_AvailableConnections.removeElement(newConnxn); 
            m_UsedConnections.addElement(newConnxn);  
        }         
        return newConnxn; 
    } 


    public synchronized void checkin(Connection c) 
    { 
        if(c != null) 
        { 
            m_UsedConnections.removeElement(c);
            m_AvailableConnections.addElement(c);         
        } 
    }             

    public int availableCount() 
    { 
        return m_AvailableConnections.size(); 
    } 

    public void run() 
    {
        Connection c = null;
        try { 
            while(true) 
            { 
                synchronized(this) 
                { 
                    while(m_AvailableConnections.size() > m_InitialConnectionCount) 
                    { 
                        c = (Connection)m_AvailableConnections.lastElement();
                        m_AvailableConnections.removeElement(c); 
                    } 
                }                   
                Thread.sleep(60000 * 1);
            }     
        }
        catch(Exception e) { e.printStackTrace(); } 
        finally {
           try{if (c != null) c.close();}
           catch(SQLException sqle) { sqle.printStackTrace(); } 
        }
    }

    public void closeConns() throws SQLException 
    { 
        try {
             for(int cnt=0; cnt<=m_AvailableConnections.size(); cnt++) 
             { 
                 Connection c = (Connection)m_AvailableConnections.lastElement(); 
                 c.close();
                 m_AvailableConnections.removeElement(c); 
             }
        }
         catch(Exception e) { e.printStackTrace(); } 
    } 
} 

Throughtout the code, I see calls to both of these classes:

Database db = new Database();
db.insertOrUpdatePS(qstring.toString(), param);

Database dbAddMember = (Database) ssb.getObject("db");
dbAddMember.insertOrUpdatePS(qstring, param, eb);

Is it safe to assume that we are not consistent with which Database we are using?

Since everyone that worked on this is already gone, how do I figure out why would someone use one over the other and if I should think about removing one possibly?

How do I go about finding if something in here is causing leakage?

Any suggestions on how to approach this?

Answer 1

Honestly, the code is absolutely horrible. Not so much because you have custom DB connection handling, but because the legacy code is synchronizing on the methods to get DB access, which means only one SQL statement will be executed at a time, not good.

You can't necessarily blame legacy code for attempting a solution for connection pooling, but in this day and age you can safely assume there are far better and proven solutions that anything written custom many years ago.

Many users will have their own opinions on the best library available to get the job done, but I will say I have had very good success on several projects using:

JTDS for a good DB driver (http://jtds.sourceforge.net/)

Atomikos for Transaction management (http://www.atomikos.com/Documentation/WebHome)

DBCP for connection pooling (https://commons.apache.org/proper/commons-dbcp/)

There are many many others. Regardless, from what I see you will definitely see an improvement in making the decision to replace the custom code with tried and true libraries to get the job done.