Saving records to database using threads

Question

I have to save a record to H2 database using threads.

This is the method in my database class that's used to save a category, a class that has name and description, into database:

public static void saveNewCategory(Category newCategory, Connection connection){
    try {
        String sql = "INSERT INTO CATEGORY(NAME, DESCRIPTION) VALUES(?, ?)";
        PreparedStatement ps = connection.prepareStatement(sql);

        ps.setString(1, newCategory.getName());
        ps.setString(2, newCategory.getDescription());

        ps.executeUpdate();

    } catch (SQLException e) {
        e.printStackTrace();
    }
}

This is my thread class which is supposed to implement that principle and save it into database using threads

public class SaveCategoryThread implements Runnable{

    private static Connection connectToDatabase;
    private Category category;

    public SaveCategoryThread(Category categoryC) {
        category=categoryC;
    }

    @Override
    public void run() {
        try {
            openConnectionWithDatabase();

            Database.saveNewCategory(category,connectToDatabase);

        } catch (IOException | SQLException e) {
            e.printStackTrace();
        } finally {
            closeConnectionWithDatabase();
        }
    }

    public synchronized void openConnectionWithDatabase() throws IOException, SQLException {
        if (Database.activeConnectionWithDatabase) {
            try {
                wait();
                System.out.println("Connection is busy");
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
        connectToDatabase = Database.connectToDatabase();
    }

    public synchronized void closeConnectionWithDatabase() {
        try {
            Database.disconnectFromDatabase(connectToDatabase);
        } catch (SQLException ex) {
            ex.printStackTrace();
        }

        notifyAll();
    }
}

And this is how I am executing it in my JavaFX class which takes user input

ExecutorService es = Executors.newCachedThreadPool();
es.execute(new SaveCategoryThread(category));

But it's not working, no errors or anything, but the result isn't saved into the database.

Answer 1

DataSource

I agree with the Answer by Stephen C, except for the part advocating the use of a connection pool. IMHO, the need for connection pooling is commonly oversold, and the risks/issues underplayed. As for his main point, I concur: You are likely headed for trouble in trying to share a Connection object. I got apprehensive just seeing static Connection, as there should be no need to ever keep a Connection around via a static.

Instead of focusing on passing around a Connection object, I suggest you instead pass around a DataSource object. A DataSource holds all the information needed to connect to a database. Or, if you decide on connection pooling, a DataSource object can be the front door to accessing that pool. Either way, using a DataSource is simple, as it mainly consists of the method getConnection, returning a Connection object.

You can hard-code a DataSource object with the database info such as database server address, database username & password, and so on. Or you can externalize those details to be configured at runtime by an administrator in a directory/naming service like an LDAP server, or in a Jakarta EE server. Use JNDI in Java to obtain the externalized DataSource object.

For example, with the H2 Database Engine, use the bundled implemantion of DataSource, org.h2.jdbcx.JdbcDataSource. Here is some code showing how to hard-code the DataSource info.

public javax.sql.DataSource configureDataSource() {
    org.h2.jdbcx.JdbcDataSource ds = Objects.requireNonNull( new JdbcDataSource() );  // Implementation of `DataSource` bundled with H2.
    ds.setURL( "jdbc:h2:/path/to/database_file;" );
    ds.setUser( "scott" );
    ds.setPassword( "tiger" );
    ds.setDescription( "An example database showing how to use DataSource." );
    return ds ;
}

Keep that DataSource object around for later use. This object merely holds connection info (or access to a connection pool). Pass to your JDBC code.

By the way, a big tip for you: Use try-with-resources syntax to automatically close your resources such as Connection, Statement, and ResultSet.

I also recommend making a habit of properly terminating your SQL statements with a SEMICOLON (;).

Add a second catch, in addition to SQLException, for the more specific exception also thrown by DataSource#getConnection: SQLTimeoutException. This exception is for when the driver has determined that the timeout value specified by the setLoginTimeout method has been exceeded.

public void saveNewCategory( Category newCategory, DataSource ds ){
        String sql = "INSERT INTO CATEGORY( NAME, DESCRIPTION ) VALUES( ?, ? ) ;";
        try (
            Connection conn = ds.getConnection() ;
            PreparedStatement ps = conn.prepareStatement( sql );
        ) {
            ps.setString( 1, newCategory.getName() );
            ps.setString( 2, newCategory.getDescription() );
            ps.executeUpdate();
        } catch ( SQLTimeoutException e ) {
            … 
        } catch ( SQLException e ) {
            … 
        }
}

Notice in the code above how the try-with-resources syntax will automatically close both the Connection and PreparedStatement objects, if they are successfully opened. We do not want to leave that Connection object left open any longer than is necessary.

Executor service

As for using concurrency with threads, using a DataSource probably simplifies your code significantly.

Early on, define a ExecutorService object, and keep it around. If you want a single task to be executed at a time, use a single-threaded service. If you want concurrent tasks, use a service backed by a pool of threads.

Either way, use Executors utility class to obtain a ExecutorService as seen in your code. Keep this ExecutorService object around, for repeated use.

ExecutorService executorService = Executors.newCachedThreadPool();
…

When you are ready to persist one of your Category objects, define a task and pass to the executor service. As your code shows, the task is defined as a Runnable or a Callable.

But the name SaveCategoryThread you chose for your Runnable task suggests you are thinking about managing threads. That is not what a Runnable is about. The Runnable is the work to be done, without regard for threads. A Runnable can be executed on the same thread, as is appropriate to some situations. So the Runnable is not in charge of threads. Managing threads is the job of the ExecutorService.

public class SaveNewCategoryTask implements Runnable {
    // Member fields. 
    DataSource dataSource;
    Category newCategory ;

    // Constructor
    public SaveNewCategoryTask ( Category newCategory , DataSource dataSource ) {
        this.newCategory = newCategory ; // Remember the passed argument.
        this.dataSource = dataSource ; // Remember the passed argument.
    }

    @Override
    public void run() {
        saveNewCategory( this.newCategory , this.dataSource ) ;
    }
}

Notice how our Runnable is no longer tracking connections or exceptions. All the database exchange details are contained within the saveNewCategory method. It is generally best to keep the Runnable task class as simple as possible. Its job is to simply hold info until needed, when its run method is eventually executed.

Usage:

// Retrieve that `DataSource` object you established early on in your app’s lifecycle.
DataSource ds = … retrieve existing object … ;
// Retrieve the `ExecutorService` object you established early on in your app’s lifecycle.
ExecutorService es = … retrieve existing object … ;
es.submit( new SaveNewCategoryTask( newCategory , ds ) ) ;

I am not entirely sure of your intent with using wait and notifyAll. It seems you are trying to manage simultaneous access to a single Connection object. As Stephen C explains in other Answer, that is basically a wrong approach, fraught with peril. Hopefully I have shown that such an approach is also unnecessary and needlessly complicated.

JavaFX

All the above discussion was aimed at Java in general.

But you mentioned JavaFX. JavaFX has its own concurrency utilities. I am not familiar with those. Hopefully the above code helps establish some general concepts and guidelines, but you may need to modify to work properly with JavaFX.

Answer 2

I think that the problem is the wait / notifyAll code.

You are waiting / notifying on this. In this context that will be the Runnable instances that you are submitting to the ExecutorService. They are all different objects. So the notifyAll notifications will not be going to reach the Runnable objects that are waiting.

---

But I think that I should point out that the strategy that you are trying to implement here is (basically) wrong. It looks like you are trying to use one database connection and share it between multiple threads. Effectively, all database INSERT operations will be single-threaded; i.e. no parallelism in the database operations.

If you want parallelism, use multiple connections managed by an off-the-shelf JDBC connection pool. That will also avoid the overhead of opening and closing database connections for each INSERT ... as your current implementation might be doing. (We can't see the relevant code.)

But if you want good insert performance, don't do transactions that consist of a single INSERT statement. Instead use JDBC's batch mechanism or multi-row INSERT statements.