diff --git a/source/ddbc/common.d b/source/ddbc/common.d index 79f7cc8..0c1c855 100644 --- a/source/ddbc/common.d +++ b/source/ddbc/common.d @@ -170,18 +170,40 @@ public: this.waitTimeOut = waitTimeOut; } + /** + * If a previously closed connection is available, test it and return it instead of creating a + * new connection. At most, one connection will be tested before creating a new connection, in + * order to prevent large delays due to having to test multiple connections. + */ override Connection getConnection() { Connection conn = null; - //writeln("getConnection(): freeConnections.length = " ~ to!string(freeConnections.length)); - if (freeConnections.length > 0) { - tracef("Retrieving database connection from pool of %s", freeConnections.length); - conn = freeConnections[freeConnections.length - 1]; // $ - 1 - auto oldSize = freeConnections.length; - myRemove(freeConnections, freeConnections.length - 1); + //writeln("getConnection(): freeConnections.length = " ~ to!string(freeConnections.length)); + if (freeConnections.length > 0) { + tracef("Retrieving database connection from pool of %s", freeConnections.length); + conn = freeConnections[freeConnections.length - 1]; // $ - 1 + auto oldSize = freeConnections.length; + myRemove(freeConnections, freeConnections.length - 1); //freeConnections.length = oldSize - 1; // some bug in remove? length is not decreased... auto newSize = freeConnections.length; assert(newSize == oldSize - 1); - } else { + + // Test the connection to make sure it is still alive. + Statement testStatement = conn.createStatement(); + scope (exit) testStatement.close(); + try { + ResultSet testResultSet = testStatement.executeQuery("SELECT 1;"); + scope (exit) testResultSet.close(); + } catch (Exception e) { + // This connection is not usable, do not add it to active connections, + // and do not cycle through the rest of the freeConnections to prevent + // excess delays. + trace("Exception trying to use freeConnection.", e); + conn = null; + } + } + + // Either there were no free connections, or the one that was picked out was invalid. + if (conn is null) { tracef("Creating new database connection (%s) %s %s", driver, url, params); try { diff --git a/test/ddbctest/testconnectionpool.d b/test/ddbctest/testconnectionpool.d new file mode 100644 index 0000000..8ec0ff3 --- /dev/null +++ b/test/ddbctest/testconnectionpool.d @@ -0,0 +1,129 @@ +import std.stdio; +import std.datetime; +import std.variant; +import std.conv; + +import core.thread : Thread; +import core.time : seconds; + +import ddbc.drivers.pgsqlddbc; +import ddbc.core; +import ddbc.common; + +import dunit; +import ddbc.test.common : DdbcTestFixture; +import ddbc.core : Connection, PreparedStatement, Statement, SQLException, TransactionIsolation; + +// Used to control our fake DB connection and when it throws errors. +bool throwOnConnect = false; +int connectCount = 0; +bool throwOnExecute = false; +int executeCount = 0; + +// A fake query result we can use to simulate errors. +class FakeResultSet : ResultSetImpl { + override + void close() { } +} + +// A fake statement we can use to simulate errors on query. +class FakeStatement : Statement { + ResultSet executeQuery(string query) { + if (throwOnExecute) { + throw new SQLException("Fake execute exception."); + } + executeCount++; + return new FakeResultSet(); + } + int executeUpdate(string query) { return 0; } + int executeUpdate(string query, out Variant insertId) { return 0; } + void close() { } + + DialectType getDialectType() { + return DialectType.SQLITE; // Just need to pick something. + } +} + +class FakeConnection : Connection { + void close() { } + void commit() { } + string getCatalog() { return ""; } + void setCatalog(string catalog) { } + bool isClosed() { return false; } + void rollback() { } + bool getAutoCommit() { return false; } + void setAutoCommit(bool autoCommit) { } + Statement createStatement() { return new FakeStatement(); } + PreparedStatement prepareStatement(string query) { return null;} + TransactionIsolation getTransactionIsolation() { return TransactionIsolation.READ_COMMITTED; } + void setTransactionIsolation(TransactionIsolation level) { } + + DialectType getDialectType() { + return DialectType.SQLITE; // Just need to pick something. + } +} + +// A fake driver we can use to simulate failures to connect. +class FakeDriver : Driver { + Connection connect(string url, string[string] params) { + if (throwOnConnect) { + throw new SQLException("Fake connect exception."); + } + connectCount++; + return new FakeConnection(); + } +} + +class ConnectionPoolTest { + mixin UnitTest; + + @Test + public void testBrokenConnection() { + Driver driver = new FakeDriver(); + DataSource dataSource = new ConnectionPoolDataSourceImpl(driver, ""); + + // Test verify that when the database is down, nothing can be done. + throwOnConnect = true; + throwOnExecute = false; + try { + Connection connection = dataSource.getConnection(); + assert(false, "Expected exception when no connection can be established."); + } catch (Exception e) { + // Ignore exception. + } + assert(connectCount == 0); + + // Obtain a working connection, and validate that it gets recycled. + throwOnConnect = false; + throwOnExecute = false; + Connection connection = dataSource.getConnection(); + connection.close(); + connection = dataSource.getConnection(); + connection.close(); + assert(connectCount == 1); + assert(executeCount == 1); + + // Create 2 connections, free them, and simulate errors when trying to use them. + Connection c1 = dataSource.getConnection(); // Use the free connection. + Connection c2 = dataSource.getConnection(); // Requres a new connection. + assert(executeCount == 2); + assert(connectCount == 2); + c1.close(); + c2.close(); + // There are now 2 connections free for re-use, simulate a network disconnect. + throwOnExecute = true; + // One connection attempts to be re-used, but it fails and a new one is created. + Connection c3 = dataSource.getConnection(); + assert(executeCount == 2); + assert(connectCount == 3); + // Restore our network and make sure the 1 remainininig free connect is re-used. + throwOnExecute = false; + Connection c4 = dataSource.getConnection(); + assert(executeCount == 3); + assert(connectCount == 3); + // We are now out of free connections, the next attempt should make a new one. + Connection c5 = dataSource.getConnection(); + assert(executeCount == 3); + assert(connectCount == 4); + } +}