1

Could I please ask whether the below code would be correctly using connection pooling (DBCP) ?

My utility class providing the BasicDataSource is as below (almost identical to the apache example)

public class DatabaseUtility {

    private static BasicDataSource dataSource;

    public static BasicDataSource getDataSource(Properties prop) {

        if (dataSource == null)
        {
            BasicDataSource ds = new BasicDataSource();
            ds.setUrl("jdbc:oracle:thin:@"+ prop.getProperty("db") + ":" + prop.getProperty("dbPort") + "/" + 
                    prop.getProperty("dbService"));
            ds.setUsername(prop.getProperty("dbUser"));
            ds.setPassword(prop.getProperty("dbPassword"));


            ds.setMinIdle(5);
            ds.setMaxIdle(10);
            ds.setMaxOpenPreparedStatements(100);

            dataSource = ds;
        }
        return dataSource;
    }

I am then using the above as :

public class MyClass {

    public static boolean isNew(Properties prop, String label) {

        Connection connection = null;
        PreparedStatement ps = null;

        try {
            BasicDataSource dataSource = DatabaseUtility.getDataSource(prop);
            connection = dataSource.getConnection();
            ps = connection.prepareStatement("Select * from my_table where LABEL = CAST( ? AS CHAR(35))");
            ps.setString(1, label);
            if (ps.executeQuery().isBeforeFirst()) {
                return false;
            }
        } catch (Exception e) {
            e.printStackTrace();
        } finally {
            try {
                if (ps != null)
                    ps.close();
                if (connection != null)
                    connection.close();
            } catch (SQLException e) {
                System.out.println("Error while closing resource :");
                e.printStackTrace();
            }
        }
        return true;
    }
}

Class MyClass could be possibly be used by multiple spawned threads. Any potential issues with this codes that I am not seeing ?

Many thanks

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
nullPointer
  • 4,419
  • 1
  • 15
  • 27

2 Answers2

5

You can face problems if several different threads will call DatabaseUtility.getDataSource for the first time. You may end up with several instances of your datasource. Read this link for thread-safe lazy singleton initialization: https://www.geeksforgeeks.org/java-singleton-design-pattern-practices-examples

Ivan
  • 8,508
  • 2
  • 19
  • 30
  • so you are basically suggesting to just change the method declaration in `DatabaseUtility` class as below (eg make it synchronized ?) `public synchronized static BasicDataSource getDataSource(Properties prop) {...` – nullPointer Sep 27 '18 at 14:21
  • 1
    @funkyjelly Making method `synchronized` is the solution with the worst performance. I'd better prefer holder class idiom (Bill Pugh Singleton Implementation from the link provided) – Ivan Sep 27 '18 at 14:44
  • after checking around a bit it seems that `BasicDataSource` is thread safe as (almost) all its variables are read/written in sync. methods. https://issues.apache.org/jira/browse/DBCP-285 So are these measures you suggest actually needed at all ? – nullPointer Sep 27 '18 at 14:52
  • @funkyjelly the issue that I've pointed out could happen when two threads call `BasicDataSource getDataSource()` at the same time. Both will see that `datasource == null` and will create two instances of `BasicDataSource` – Ivan Sep 27 '18 at 15:03
1

if you switch to HikariCP pool the connection validation settings will be taken care of automatically. Take a look at few gotchas in the pools e.g.:

Karol Dowbecki
  • 43,645
  • 9
  • 78
  • 111
  • as per the DBCP docs the testOnBorrow defaults to true. Do you see any other critical configuration that may be missing? If I'm not mistaken I should be good with the default set up.. – nullPointer Sep 27 '18 at 14:40
  • @funkyjelly you are right, removed this part of the answer. I'm not sure how the new 2.5+ DBCP compares to HikariCP. In older versions DBCP had some issues, I added few links to highlight the problem. – Karol Dowbecki Sep 27 '18 at 15:13