-2

I use ExecutorService with FixedThreadPool to execute some SQL via JDBC. However when I profile my app it seems that thread count is just raising and so does the memory of course. The problem is that is is somehow related to JDBC because when I remove creating statements and connections inside my task for the thread pool, the thread count is not raising at all.

Here is how I sumbit tasks into my thread pool:

       new Thread() {
            public void run() {
                ExecutorService executorService = Executors.newFixedThreadPool(5);
                    while (!isCancelled) {
                        executorService.submit(RunnableTask.this);
                        Thread.sleep(interval);
                    }
                    executorService.shutdown(); //wait for all tasks to finish and then shutdown executor
                    executorService.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS); //wait till shutdown finished
                } catch (InterruptedException ex) {
                    //
                }
            }
        };

Here is what I do in the task:

    try (Connection con = pool.getConnection(); PreparedStatement st = (this.isCall ? con.prepareCall(this.sql) : con.prepareStatement(this.sql))) {
        st.execute();
    } catch (Exception e) {
        //
    }

Here is the ConnectionPool, that is used in above mentioned code (pool.getConnection(), I use apache DBCP2:

import java.sql.Connection;
import java.sql.SQLException;
import org.apache.commons.dbcp2.BasicDataSource;

public class MySQLPool {

private BasicDataSource dataSource;

public MySQLPool setup(String driver, String uri, String user, String password, int maxConnections) throws Exception {
    if (this.dataSource == null) {
        BasicDataSource ds = new BasicDataSource();
        ds.setDriverClassName(Main.driver);
        ds.setUrl(uri);
        ds.setUsername(user);
        ds.setPassword(password);
        ds.setMaxTotal(maxConnections);
        ds.setMaxWaitMillis(2000);
        this.dataSource = ds;
    }
    return this;
}

Here is a example from profiler (imgur)

It seems that the threads are not ended properly, which is very weird because the ExecutorService should run out of them if it is a fixed pool of 5 connections right ? So I have no idea how the hreads are still there, they are causing quite large memory leak.

The problem is in creating Connection and PreparedStatement objects because when I comment it out, the number of threads stays at a fixed value.

Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
Dakado
  • 11
  • 2
  • 6
  • 1
    Please provide a [mcve]. – Mark Rotteveel Mar 21 '19 at 18:18
  • What? you are creating `new Thread()` in which `run` method you are creating `ExecutorService` where you submit just creating `Thread` instance? oh man... When you submit such instance into ExecutorService its run method will be eventually called. That will create another ExecutorService where you again submit ... – rkosegi Mar 21 '19 at 18:35

1 Answers1

1

You do not show all your code such as isCancelled. So we can’t help specifically. But your approach seems to be off-kilter, so read on.

ScheduledExecutorService

You should not be trying to manage the timing of your executor service. If you are calling Thread.sleep in conjunction with an executor service, you are likely doing something wrong.

Nor should you be invoking new Thread. The whole point of executor service is to let the framework manage the details of threading. You are working too hard.

For repeated invocations of a task, use a ScheduledExecutorService.

For more info, see the Oracle Tutorial, the class JavaDoc, and search Stack Overflow. This topic has been addressed many times already.

Example app

Here is a brief example.

Use the Executors utility class to create your thread pool. We need only a single thread for you to make repeated calls to the database. Looking at your partial code example, I cannot see why you were trying to run 5 threads. If you are making a series of sequential calls to the database, you need only a single thread.

Make your Runnable to call the database.

package work.basil.example;

import java.sql.Connection;
import java.time.Instant;

public class DatabaseCaller implements Runnable
{
    private Connection connection = null;

    public DatabaseCaller ( Connection connection )
    {
        this.connection = connection;
    }

    @Override
    public void run ()
    {
        // Query the database. Report results, etc.
        System.out.println( "Querying the database now. " + Instant.now() );
    }
}

CAUTION: Always wrap your run method’s code with a try catch to catch any unexpected Exception or Error (a Throwable). Any uncaught throwable reaching the executor will cause it to cease work. The task will no longer be scheduled for further runs.

Instantiate that Runnable, and schedule it to run repeatedly.

package work.basil.example;

import java.sql.Connection;
import java.time.Instant;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

public class DbRepeat
{

    public static void main ( String[] args )
    {
        DbRepeat app = new DbRepeat();
        app.doIt();
    }

    private void doIt ()
    {
        System.out.println( "Starting app. " + Instant.now() );

        Connection conn = null; // FIXME: Instantiate a `Connection` object here.
        Runnable runnable = new DatabaseCaller( conn );

        ScheduledExecutorService ses = Executors.newSingleThreadScheduledExecutor();

        long initialDelay = 0;
        long delay = 5;
        ScheduledFuture future = ses.scheduleWithFixedDelay( runnable , initialDelay , delay , TimeUnit.SECONDS );

        // Let our demo run a few minutes.
        try
        {
            Thread.sleep( TimeUnit.MINUTES.toMillis( 2 ) ); // Let this app run a few minutes as a demo.
        } catch ( InterruptedException e )
        {
            System.out.println( "Somebody woke up our sleeping thread. Message # 1b296f04-3721-48de-82a8-d03b986a4b55." );
        }

        // Always shutdown your scheduled executor service. Otherwise its backing thread pool could continue to run, outliving the lifecycle of your app.
        ses.shutdown();

        System.out.println( "Ending app. " + Instant.now() );
    }
}

Notice how simple this is.

  • We defined a task, in an instantiated Runnable object.
  • We established an executor service backed by a single thread.
  • We told that service to run our task repeatedly on our behalf, and we specified how often to run that task.
  • Eventually, we told the service to stop scheduling further executions of that task, and to close its thread pool.

At no point did we deal directly with threads. We let the executors framework handle all the gnarly details of threading.

Caution: You still need to make your Runnable code thread-safe if using multiple threads. The executor framework is extremely slick and helpful, but it is not magic. To learn about thread-safety and concurrency in Java, read this excellent book annually: Java Concurrency in Practice by Brian Goetz, et al.

When run.

Starting app. 2019-03-21T19:46:09.531740Z

Querying the database now. 2019-03-21T19:46:09.579573Z

Querying the database now. 2019-03-21T19:46:14.585629Z

Querying the database now. 2019-03-21T19:47:59.647485Z

Querying the database now. 2019-03-21T19:48:04.650555Z

Ending app. 2019-03-21T19:48:09.579407Z

Skip the connection pool

In my experience, the need for a database connection pool is exaggerated by many folks. There are several pitfalls to using a connection pool. And I find establishing a database connection is not nearly as expensive as many claim, especially if local on the same machine.

So I suggest you skip the connection pool for now. Get your code working reliably while using fresh connections.

If you can later prove a bottleneck in performance because of database connections, then consider a pool. And verify it actually helps. Otherwise you would be committing the sin of premature optimization.

Community
  • 1
  • 1
Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154