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

2 Answers 2

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

4
  • 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) {... Sep 27, 2018 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, 2018 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. issues.apache.org/jira/browse/DBCP-285 So are these measures you suggest actually needed at all ? Sep 27, 2018 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, 2018 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.:

2
  • 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.. Sep 27, 2018 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. Sep 27, 2018 at 15:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.