0

In my Javafx application I connect to remote server on Hetzner.de using sftp. To manage connections, I use cp30 library connection pooling with the following parameters:

public Connection dbConnectSite() throws SQLException, PropertyVetoException {
    ComboPooledDataSource cpds = new ComboPooledDataSource();
    cpds.setDriverClass("com.mysql.jdbc.Driver");
    cpds.setJdbcUrl("jdbc:mysql://" + mySQLHost + ":" + mySQLPort + "/" + mySQLDBName + "?characterEncoding=UTF-8&autoReconnect=true"); // 192.168.100.100 v seti.
    cpds.setUser(mySQLUser);
    cpds.setPassword(mySQLPassword);
    cpds.setMinPoolSize(3);
    cpds.setMaxPoolSize(20); // Maximum number of Connections a pool will maintain at any given time.
    cpds.setAcquireIncrement(1);
    cpds.setTestConnectionOnCheckin(true); // If true, an operation will be performed asynchronously at every connection checkin to verify that the connection is valid.
    cpds.setTestConnectionOnCheckout(true); // If true, an operation will be performed at every connection checkout to verify that the connection is valid.
    cpds.setIdleConnectionTestPeriod(300); // If this is a number greater than 0, c3p0 will test all idle, pooled but unchecked-out connections, every this number of seconds.
    cpds.setMaxIdleTimeExcessConnections(240);
    cpds.setMaxIdleTime(3600); // Seconds a Connection can remain pooled but unused before being discarded. Zero means idle connections never expire.
    cpds.setMaxStatements(100);
    cpds.setCheckoutTimeout(0); // The number of milliseconds a client calling getConnection() will wait for a Connection to be checked-in or acquired when the pool is exhausted. Zero means wait indefinitely.
    cpds.setMaxAdministrativeTaskTime(0); // Seconds before c3p0's thread pool will try to interrupt an apparently hung task. 
    cpds.setMaxConnectionAge(saytPort); // Seconds, effectively a time to live. A Connection older than maxConnectionAge will be destroyed and purged from the pool. Zero means no maximum absolute age is enforced. 
    cpds.setPreferredTestQuery("SELECT 1");
    dsSite = cpds;
    conSayt = dsSite.getConnection();
    return conSayt;
}

Connection goes fine. The console log for connection is:

ноя 10, 2013 9:59:47 PM com.mchange.v2.log.MLog <clinit>
INFO: MLog clients using java 1.4+ standard logging.
ноя 10, 2013 9:59:47 PM com.mchange.v2.c3p0.C3P0Registry banner
INFO: Initializing c3p0-0.9.1.2 [built 21-May-2007 15:04:56; debug? true; trace: 10]
ноя 10, 2013 9:59:47 PM com.mchange.v2.c3p0.impl.AbstractPoolBackedDataSource getPoolManager
INFO: Initializing c3p0 pool... com.mchange.v2.c3p0.ComboPooledDataSource [ acquireIncrement -> 1, acquireRetryAttempts -> 30, acquireRetryDelay -> 1000, autoCommitOnClose -> false, automaticTestTable -> null, breakAfterAcquireFailure -> false, checkoutTimeout -> 0, connectionCustomizerClassName -> null, connectionTesterClassName -> com.mchange.v2.c3p0.impl.DefaultConnectionTester, dataSourceName -> z8kfsx8yiing6p1lp8nb2|3ba701c9, debugUnreturnedConnectionStackTraces -> false, description -> null, driverClass -> com.mysql.jdbc.Driver, factoryClassLocation -> null, forceIgnoreUnresolvedTransactions -> false, identityToken -> z8kfsx8yiing6p1lp8nb2|3ba701c9, idleConnectionTestPeriod -> 300, initialPoolSize -> 3, jdbcUrl -> jdbc:mysql://sql249.your-server.de:3306/kombinezonik?characterEncoding=UTF-8&autoReconnect=true, maxAdministrativeTaskTime -> 0, maxConnectionAge -> 22, maxIdleTime -> 3600, maxIdleTimeExcessConnections -> 240, maxPoolSize -> 20, maxStatements -> 100, maxStatementsPerConnection -> 0, minPoolSize -> 3, numHelperThreads -> 3, numThreadsAwaitingCheckoutDefaultUser -> 0, preferredTestQuery -> SELECT 1, properties -> {user=******, password=******}, propertyCycle -> 0, testConnectionOnCheckin -> true, testConnectionOnCheckout -> true, unreturnedConnectionTimeout -> 0, usesTraditionalReflectiveProxies -> false ]
nov 10, 2013 9:59:49 PM com.mchange.v2.c3p0.impl.AbstractPoolBackedDataSource getPoolManager
INFO: Initializing c3p0 pool... com.mchange.v2.c3p0.ComboPooledDataSource [ acquireIncrement -> 3, acquireRetryAttempts -> 30, acquireRetryDelay -> 1000, autoCommitOnClose -> false, automaticTestTable -> null, breakAfterAcquireFailure -> false, checkoutTimeout -> 0, connectionCustomizerClassName -> null, connectionTesterClassName -> com.mchange.v2.c3p0.impl.DefaultConnectionTester, dataSourceName -> z8kfsx8yiing6p1lp8nb2|2bc60511, debugUnreturnedConnectionStackTraces -> false, description -> null, driverClass -> com.mysql.jdbc.Driver, factoryClassLocation -> null, forceIgnoreUnresolvedTransactions -> false, identityToken -> z8kfsx8yiing6p1lp8nb2|2bc60511, idleConnectionTestPeriod -> 0, initialPoolSize -> 3, jdbcUrl -> jdbc:mysql://localhost:3306/kombadmin?characterEncoding=UTF-8&autoReconnect=true, maxAdministrativeTaskTime -> 0, maxConnectionAge -> 0, maxIdleTime -> 0, maxIdleTimeExcessConnections -> 0, maxPoolSize -> 15, maxStatements -> 0, maxStatementsPerConnection -> 0, minPoolSize -> 3, numHelperThreads -> 3, numThreadsAwaitingCheckoutDefaultUser -> 0, preferredTestQuery -> null, properties -> {user=******, password=******}, propertyCycle -> 0, testConnectionOnCheckin -> false, testConnectionOnCheckout -> false, unreturnedConnectionTimeout -> 0, usesTraditionalReflectiveProxies -> false ]

But if the GUI stands by for some different periods of time, I get an error:

nov 10, 2013 10:07:50 PM MyClass
SEVERE: null
com.mysql.jdbc.exceptions.jdbc4.CommunicationsException: Communications link failure

The last packet successfully received from the server was 344 144 milliseconds ago.  The last packet sent successfully to the server was 2 milliseconds ago.
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:525)
    at com.mysql.jdbc.Util.handleNewInstance(Util.java:411)
    at com.mysql.jdbc.SQLError.createCommunicationsException(SQLError.java:1117)
    at com.mysql.jdbc.MysqlIO.reuseAndReadPacket(MysqlIO.java:3603)
    at com.mysql.jdbc.MysqlIO.reuseAndReadPacket(MysqlIO.java:3492)
    at com.mysql.jdbc.MysqlIO.checkErrorPacket(MysqlIO.java:4043)
    at com.mysql.jdbc.MysqlIO.sendCommand(MysqlIO.java:2503)
    at com.mysql.jdbc.MysqlIO.sqlQueryDirect(MysqlIO.java:2664)
    at com.mysql.jdbc.ConnectionImpl.execSQL(ConnectionImpl.java:2788)
    at com.mysql.jdbc.ConnectionImpl.execSQL(ConnectionImpl.java:2738)
    at com.mysql.jdbc.StatementImpl.executeQuery(StatementImpl.java:1617)
    at com.mchange.v2.c3p0.impl.NewProxyStatement.executeQuery(NewProxyStatement.java:35)
    at MyClass.java:158
    at MyClass$1$1$1.run(MyClass.java:128)
    at com.sun.javafx.application.PlatformImpl$4$1.run(PlatformImpl.java:179)
    at com.sun.javafx.application.PlatformImpl$4$1.run(PlatformImpl.java:176)
    at java.security.AccessController.doPrivileged(Native Method)
    at com.sun.javafx.application.PlatformImpl$4.run(PlatformImpl.java:176)
    at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:76)
    at com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
    at com.sun.glass.ui.gtk.GtkApplication$3$1.run(GtkApplication.java:82)
    at java.lang.Thread.run(Thread.java:722)
Caused by: java.io.EOFException: Can not read response from server. Expected to read 4 bytes, read 0 bytes before connection was unexpectedly lost.
    at com.mysql.jdbc.MysqlIO.readFully(MysqlIO.java:3052)
    at com.mysql.jdbc.MysqlIO.reuseAndReadPacket(MysqlIO.java:3503)
    ... 18 more

Of course I close Statements and ResultSets, using try with resources:

try (Statement stmt = m.conSayt.createStatement(); ResultSet rs = stmt.executeQuery(SQL);) {
    // Contents.
} catch (SQLException ex){
    // Contents.
}
Zon
  • 18,610
  • 7
  • 91
  • 99
  • What version of c3p0? What version of Connector/J? What about relevant DB-side configuration? Are you retrying on SQLExceptions that result from timeouts as described by [Bill Karwin's answer?](http://stackoverflow.com/a/270857/139010) Your idle connection time is 1 hour. Have you tried turning that down? – Matt Ball Nov 10 '13 at 16:29
  • Also, your idle connection test period is 5 minutes. Have you tried turning that down? That would be the #1 thing to try, given that you said the disconnects happen after O(5 minutes) of standby. – Matt Ball Nov 10 '13 at 16:35
  • Do you get a connection from the pool immediately before the exception occurs? Your `dbConnectSite()` method implies you are holding onto a connection for extended periods of time. – samlewis Nov 10 '13 at 16:42
  • Matt, c3p0 is 0.9.1.2 version. Db is standard parameters MYSQL MyISAM engine. I use reconnect=true, but it is a new query that meets an error after stanby. I tried default c3p0 parameters - they resulted the same. Made cpds.setIdleConnectionTestPeriod(6000), but still looses connection and error after about 5 minutes. – Zon Nov 11 '13 at 17:19
  • Sam, actually yes, I pass "conSayt = dsSite.getConnection();" only once and then use conSayt in other classes. Error arises after standby only after user action, when connection is requested. Passive app standby shows no errors. – Zon Nov 11 '13 at 17:59

1 Answers1

5

The issue here is the way you are using (misusing, i'm afraid) c3p0.

Each time you get a Connection, you are creating a new Connection pool, then starting up the Connection pool, then letting it go out of scope. Note that in your logs you are initializing three different c3p0 pools in three seconds, once each time you try to acquire a Connection. Not good.

You never clean-up your Connection. (You don't actually clean up ResultSets and Statements in the code you provide here.) As samlewis says, it looks like you intend to just hold the Connection open.

All of that is not at all how you ought to use a Connection pool. If you want to fetch one Connection and hold it open, just fetch the Connection directly using DriverManager.getConnection( ... ) and avoid a whole lot of complexity.

Except you'll find, as you have found already, that holding open a JDBC Connection for long periods of time and expecting to reuse it is a fragile way to run an application. That is the problem Connection pools exist to solve.

With a Connection pool, stuff has to be different:

1) That ComboPooledDataSource you create? Don't let it disappear. Hold on to a reference to it somewhere, perhaps as a static field, perhaps as a member of some Object you use. You should create one DataSource one time for a typical application deployment.

2) Each time you need a Connection i) acquire a "new" Connection by calling getConnection() on the DataSource; ii) do whatever work you need to do; iii) close() that Connection immediately after use.

c3p0 will maintain a pool of Connections, so all of this will be fast. cpds.getConnection() won't make network connections to the dbms to establish a new Connection, it will simply hand you a Connection from the pool. When you close() the Connection, the connection to the dbms won't actually be destroyed, just recycled back into the pool. This is JDBC-standard transparent Connection pooling.

[Oops! I didn't notice, questioner uses java 7 try-with-resources, which should be fina, as long as Connections are among the resources!] Note that calling functions that create ResultSets, Statements, or Connnections in a try{} block is not at all sufficient to ensure that those resources are close()ed. You have to actually close them, in a finally block:

Connection con = null;
Statement stmt = null;
ResultSet rs = null;
try
{
    con = cpds.getConnection();
    stmt = cpds.createStatement();
    rs = stmt.executeQuery("SELECT something FROM somewhere");
    while ( rs.next() )
    {
       // do some stuff with results
    }
}
finally
{
   try { if (rs != null) rs.close() } catch (SQLException e) { e.printStackTrace(); }
   try { if (stmt != null) stmt.close() } catch (SQLException e) { e.printStackTrace(); }
   try { if (con != null) con.close() } catch (SQLException e) { e.printStackTrace(); }
}

Again, cpds should not be a local variable. Your Connection, on the other hand, should be closed (generally) within the scope of the method that opened it, in a finally block.

[Note that each resource close() inside the finally block is wrapped in a nested try, to ensure that an Exception within one close() doesn't prevent other resources from a best-attempt close(). You can make this cleaner by wrapping the nested tries in helper methods.]

Some other comments: acquireIncrement of 1 is generally a bad idea, there's little reason to set testConnectionOnCheckin to true if you are already testing Connections on checkout. But for the moment, none of that matters, the only config your app is actually exercising is the jdbcUrl, user, and password, because you are only ever getting a single Connection and then discarding (well, trying to discard) the pool.

It's worth explaining that "trying to discard". c3p0 pools spawn their own maintenance Threads. As you construct and initialize new pools and then let them go out of scope without close()ing them, you are creating Threads and memory leak. Whenever you are done with a c3p0 ComboPooledDataSource, you must call close() [or more rarely the static method DataSources.destroy( ... )] to shut the DataSource down. Typically this is done once, as an application is shutting down or resetting itself.

Steve Waldman
  • 13,689
  • 1
  • 35
  • 45
  • Steve, thanks for a detailed answer. I use conSayt as a public Connection object in the very first class of the application and then all other classes use it. There is only one connection in one connection pool. But I really never close it whyle app's alive. – Zon Nov 11 '13 at 17:38
  • I have used this approach for several years, but mostly on localhost and never faced this problem before. I thought c3p0 bothers connection management for me as of mchange.com: "Statement and ResultSets are carefully cleaned up when pooled Connections and Statements are checked in, to prevent resource- exhaustion when clients use the lazy but common resource-management strategy of only cleaning up their Connections.... " I'll try to rework the approach referencing cpds and opening-closing new connections every time. Maybe using multiple threads in this app revealed the problem. – Zon Nov 11 '13 at 17:41
  • But I doubt your comment about try{} is correct as another approach is used - "try-with-resources" - that is try(){}, suggested by Netbeans to replace exactly the "try{}finally{}" construction. I googled it up and it is equivalent and resources are closed after that correctly. I like "try with resources" as it's more compact. – Zon Nov 11 '13 at 17:50
  • Zon -- you are right, re try-with-resources! i didn't see that you'd used that (which i've not much used!), just saw the try/catch. sorry! – Steve Waldman Nov 11 '13 at 19:20
  • Finally had time to rework the code. Apllied recommendations 1) to create only one ComboPooledDataSource (cpds), 2) to connect using it from short-code as anonymous connection in try-with-resources like try (Statement stmt = cpds.getConnection().createStatement()) {} catch (SQLException e) {} and 2) assumed that try-catch will also close Connection or poll will correctly manage them. – Zon Dec 25 '13 at 10:30
  • be careful! try (Statement stmt = cpds.getConnection().createStatement()) {} catch (SQLException e) {} won't also close the Connection; it's a leak. use try (Connection con = cpds.getConnection(); Statement stmt = con.createStatement(); ResultSet rs = stmt.executeQuery("myQuery")) {} catch (SQLException e) {} if you are doing updates, obviously you can omit the ResultSet generation. If you are using PreparedStatements and need to customize params, acquire Connection and PreparedStatement first, then ResultSet in a nested try-with-resources after customization. – Steve Waldman Dec 25 '13 at 11:22
  • Thank you, Steve. Now it works, there is no hang up and error after standby for several hours. – Zon Dec 25 '13 at 17:11