=== modified file 'CHANGES' --- CHANGES 2010-03-03 00:50:27 +0000 +++ CHANGES 2010-03-05 21:20:26 +0000 @@ -20,7 +20,25 @@ - Fixed BUG#51666 - StatementInterceptors were never "un-safed" after connection establishment, causing interceptors which returned result sets pre/post execution would not work. + + - Fixed BUG#51783 - Load-balanced connections could throw a SQLException + incorrectly on commit() or rollback(). This was not caused by failures in commit + or rollback, but rather by the possibility that the newly-selected physical + connection was stale. Added logic to catch and retry if this happens, up to + the number of hosts specified for load-balancing. Also added new property, + loadBalanceValidateConnectionOnSwapServer, which controls whether to explicitly + ping the selected host (otherwise, the host is presumed to be up, and will only + be noticed if auto-commit or transaction isolation state needs to be set and + fails). + + - Added loadBalancePingTimeout property to allow a specific timeout to be set + for each ping executed against the servers. This ping is executed when the + physical connections are rebalanced (commit/rollback or communication exception), + or when a query starting with (exactly) "/* ping */" is executed. The latter + causes each open underlying physical connection to be pinged. + - Fixed BUG#51776 - Connection.rollback() could swallow exceptions incorrectly. + 02-18-10 - Version 5.1.12 - NO_INDEX_USED and NO_GOOD_INDEX used were only being set when profileSQL === modified file 'src/com/mysql/jdbc/ConnectionImpl.java' --- src/com/mysql/jdbc/ConnectionImpl.java 2010-03-02 23:39:26 +0000 +++ src/com/mysql/jdbc/ConnectionImpl.java 2010-03-05 20:19:05 +0000 @@ -4854,9 +4854,11 @@ } catch (SQLException sqlEx) { // We ignore non-transactional tables if told to do so if (getIgnoreNonTxTables() - && (sqlEx.getErrorCode() != SQLError.ER_WARNING_NOT_COMPLETE_ROLLBACK)) { - throw sqlEx; + && (sqlEx.getErrorCode() == SQLError.ER_WARNING_NOT_COMPLETE_ROLLBACK)) { + return; } + throw sqlEx; + } } } catch (SQLException sqlException) { === modified file 'src/com/mysql/jdbc/ConnectionProperties.java' --- src/com/mysql/jdbc/ConnectionProperties.java 2010-03-02 23:09:37 +0000 +++ src/com/mysql/jdbc/ConnectionProperties.java 2010-03-05 17:24:20 +0000 @@ -1640,4 +1640,13 @@ public int getMaxAllowedPacket(); boolean getRetainStatementAfterResultSetClose(); + + public abstract int getLoadBalancePingTimeout(); + + public abstract void setLoadBalancePingTimeout(int loadBalancePingTimeout); + + public abstract boolean getLoadBalanceValidateConnectionOnSwapServer(); + + public abstract void setLoadBalanceValidateConnectionOnSwapServer(boolean loadBalanceValidateConnectionOnSwapServer); + } === modified file 'src/com/mysql/jdbc/ConnectionPropertiesImpl.java' --- src/com/mysql/jdbc/ConnectionPropertiesImpl.java 2010-03-02 23:09:37 +0000 +++ src/com/mysql/jdbc/ConnectionPropertiesImpl.java 2010-03-05 17:43:45 +0000 @@ -1029,6 +1029,18 @@ Messages.getString("ConnectionProperties.loadBalanceBlacklistTimeout"), //$NON-NLS-1$ "5.1.0", MISC_CATEGORY, Integer.MIN_VALUE); //$NON-NLS-1$ + private IntegerConnectionProperty loadBalancePingTimeout = new IntegerConnectionProperty( + "loadBalancePingTimeout", 0, //$NON-NLS-1$ + 0, Integer.MAX_VALUE, + Messages.getString("ConnectionProperties.loadBalancePingTimeout"), //$NON-NLS-1$ + "5.1.13", MISC_CATEGORY, Integer.MIN_VALUE); //$NON-NLS-1$ + + private BooleanConnectionProperty loadBalanceValidateConnectionOnSwapServer = new BooleanConnectionProperty( + "loadBalanceValidateConnectionOnSwapServer", + false, + Messages.getString("ConnectionProperties.loadBalanceValidateConnectionOnSwapServer"), //$NON-NLS-1$ + "5.1.13", MISC_CATEGORY, Integer.MIN_VALUE); //$NON-NLS-1$ + private StringConnectionProperty localSocketAddress = new StringConnectionProperty("localSocketAddress", //$NON-NLS-1$ null, Messages.getString("ConnectionProperties.localSocketAddress"), //$NON-NLS-1$ "5.0.5", CONNECTION_AND_AUTH_CATEGORY, Integer.MIN_VALUE); //$NON-NLS-1$ @@ -4436,6 +4448,14 @@ public void setLoadBalanceBlacklistTimeout(int loadBalanceBlacklistTimeout) { this.loadBalanceBlacklistTimeout.setValue(loadBalanceBlacklistTimeout); } + + public int getLoadBalancePingTimeout() { + return loadBalancePingTimeout.getValueAsInt(); + } + + public void setLoadBalancePingTimeout(int loadBalancePingTimeout) { + this.loadBalancePingTimeout.setValue(loadBalancePingTimeout); + } public void setRetriesAllDown(int retriesAllDown) { this.retriesAllDown.setValue(retriesAllDown); @@ -4484,4 +4504,14 @@ public void setQueryTimeoutKillsConnection(boolean queryTimeoutKillsConnection) { this.queryTimeoutKillsConnection.setValue(queryTimeoutKillsConnection); } + + public boolean getLoadBalanceValidateConnectionOnSwapServer() { + return this.loadBalanceValidateConnectionOnSwapServer.getValueAsBoolean(); + } + + public void setLoadBalanceValidateConnectionOnSwapServer( + boolean loadBalanceValidateConnectionOnSwapServer) { + this.loadBalanceValidateConnectionOnSwapServer.setValue(loadBalanceValidateConnectionOnSwapServer); + + } } === modified file 'src/com/mysql/jdbc/LoadBalancingConnectionProxy.java' --- src/com/mysql/jdbc/LoadBalancingConnectionProxy.java 2010-03-02 23:09:37 +0000 +++ src/com/mysql/jdbc/LoadBalancingConnectionProxy.java 2010-03-05 21:12:49 +0000 @@ -290,6 +290,7 @@ // connection error, close up shop on current // connection invalidateCurrentConnection(); + pickNewConnection(); } } } @@ -380,7 +381,7 @@ if ("close".equals(methodName)) { closeAllConnections(); - + this.isClosed = true; return null; } @@ -436,7 +437,6 @@ - this.transactionStartTime; } } - pickNewConnection(); } } @@ -466,26 +466,58 @@ return; } - - ConnectionImpl newConn = (ConnectionImpl) this.balancer.pickConnection( - this, Collections.unmodifiableList(this.hostList), Collections - .unmodifiableMap(this.liveConnections), - (long[]) this.responseTimes.clone(), this.retriesAllDown); - - MySQLConnection thisAsConnection = (MySQLConnection) Proxy - .newProxyInstance(getClass().getClassLoader(), - new Class[] { com.mysql.jdbc.MySQLConnection.class }, - this); - - newConn.setProxy(thisAsConnection); - - if (this.currentConn != null) { - newConn.setTransactionIsolation(this.currentConn - .getTransactionIsolation()); - newConn.setAutoCommit(this.currentConn.getAutoCommit()); - } - - this.currentConn = newConn; + + if(this.currentConn.isClosed()){ + invalidateCurrentConnection(); + } + + boolean currentAutoCommit = this.currentConn.getAutoCommit(); + int currentTransIsolation = this.currentConn.getTransactionIsolation(); + int pingTimeout = this.currentConn.getLoadBalancePingTimeout(); + boolean pingBeforeReturn = this.currentConn.getLoadBalanceValidateConnectionOnSwapServer(); + for(int hostsTried = 0, hostsToTry = this.hostList.size(); hostsTried <= hostsToTry; hostsTried++){ + try{ + ConnectionImpl newConn = (ConnectionImpl) this.balancer.pickConnection( + + this, Collections.unmodifiableList(this.hostList), Collections + .unmodifiableMap(this.liveConnections), + (long[]) this.responseTimes.clone(), this.retriesAllDown); + + MySQLConnection thisAsConnection = (MySQLConnection) Proxy + .newProxyInstance(getClass().getClassLoader(), + new Class[] { com.mysql.jdbc.MySQLConnection.class }, + this); + + newConn.setProxy(thisAsConnection); + if (this.currentConn != null) { + if(pingBeforeReturn){ + if(pingTimeout == 0){ + newConn.ping(); + } else { + newConn.pingInternal(true, pingTimeout); + } + } + newConn.setTransactionIsolation(currentTransIsolation); + newConn.setAutoCommit(currentAutoCommit); + } + + this.currentConn = newConn; + return; + } catch (SQLException e){ + String sqlState = e.getSQLState(); + + if (sqlState != null) { + if (sqlState.startsWith("08")) { + // connection error, close up shop on current + // connection + invalidateCurrentConnection(); + } + } + } + + } + // no hosts available to swap connection to, close up. + this.isClosed = true; } /** @@ -546,6 +578,7 @@ public synchronized void doPing() throws SQLException { SQLException se = null; boolean foundHost = false; + int pingTimeout = this.currentConn.getLoadBalancePingTimeout(); synchronized (this) { for (Iterator i = this.hostList.iterator(); i.hasNext();) { String host = (String) i.next(); @@ -554,7 +587,11 @@ continue; } try { - conn.ping(); + if(pingTimeout == 0){ + conn.ping(); + } else { + ((ConnectionImpl) conn).pingInternal(true, pingTimeout); + } foundHost = true; } catch (SQLException e) { // give up if it is the current connection, otherwise NPE === modified file 'src/com/mysql/jdbc/LocalizedErrorMessages.properties' --- src/com/mysql/jdbc/LocalizedErrorMessages.properties 2009-08-13 20:45:08 +0000 +++ src/com/mysql/jdbc/LocalizedErrorMessages.properties 2010-03-05 20:53:15 +0000 @@ -501,6 +501,8 @@ ConnectionProperties.largeRowSizeThreshold=What size result set row should the JDBC driver consider "large", and thus use a more memory-efficient way of representing the row internally? ConnectionProperties.loadBalanceStrategy=If using a load-balanced connection to connect to SQL nodes in a MySQL Cluster/NDB configuration (by using the URL prefix "jdbc:mysql:loadbalance://"), which load balancing algorithm should the driver use: (1) "random" - the driver will pick a random host for each request. This tends to work better than round-robin, as the randomness will somewhat account for spreading loads where requests vary in response time, while round-robin can sometimes lead to overloaded nodes if there are variations in response times across the workload. (2) "bestResponseTime" - the driver will route the request to the host that had the best response time for the previous transaction. ConnectionProperties.loadBalanceBlacklistTimeout=Time in milliseconds between checks of servers which are unavailable. +ConnectionProperties.loadBalancePingTimeout=Time in milliseconds to wait for ping response from each of load-balanced physical connections when using load-balanced Connection. +ConnectionProperties.loadBalanceValidateConnectionOnSwapServer=Should the load-balanced Connection explicitly check whether the connection is live when swapping to a new physical connection at commit/rollback? ConnectionProperties.localSocketAddress=Hostname or IP address given to explicitly configure the interface that the driver will bind the client side of the TCP/IP connection to when connecting. ConnectionProperties.locatorFetchBufferSize=If 'emulateLocators' is configured to 'true', what size buffer should be used when fetching BLOB data for getBinaryInputStream? ConnectionProperties.logger=The name of a class that implements \"{0}\" that will be used to log messages to. (default is \"{1}\", which logs to STDERR) === modified file 'src/com/mysql/jdbc/ReplicationConnection.java' --- src/com/mysql/jdbc/ReplicationConnection.java 2010-03-02 23:09:37 +0000 +++ src/com/mysql/jdbc/ReplicationConnection.java 2010-03-05 17:28:07 +0000 @@ -2394,6 +2394,22 @@ public void setLoadBalanceBlacklistTimeout(int loadBalanceBlacklistTimeout) { this.currentConnection.setLoadBalanceBlacklistTimeout(loadBalanceBlacklistTimeout); } + + public int getLoadBalancePingTimeout() { + return this.currentConnection.getLoadBalancePingTimeout(); + } + + public void setLoadBalancePingTimeout(int loadBalancePingTimeout) { + this.currentConnection.setLoadBalancePingTimeout(loadBalancePingTimeout); + } + + public boolean getLoadBalanceValidateConnectionOnSwapServer() { + return this.currentConnection.getLoadBalanceValidateConnectionOnSwapServer(); + } + + public void setLoadBalanceValidateConnectionOnSwapServer(boolean loadBalanceValidateConnectionOnSwapServer) { + this.currentConnection.setLoadBalanceValidateConnectionOnSwapServer(loadBalanceValidateConnectionOnSwapServer); + } public int getRetriesAllDown() { return this.currentConnection.getRetriesAllDown(); === modified file 'src/com/mysql/jdbc/jdbc2/optional/ConnectionWrapper.java' --- src/com/mysql/jdbc/jdbc2/optional/ConnectionWrapper.java 2010-03-02 23:09:37 +0000 +++ src/com/mysql/jdbc/jdbc2/optional/ConnectionWrapper.java 2010-03-05 17:45:58 +0000 @@ -2595,6 +2595,22 @@ public void setLoadBalanceBlacklistTimeout(int loadBalanceBlacklistTimeout) { this.mc.setLoadBalanceBlacklistTimeout(loadBalanceBlacklistTimeout); } + public int getLoadBalancePingTimeout() { + return this.mc.getLoadBalancePingTimeout(); + } + + public void setLoadBalancePingTimeout(int loadBalancePingTimeout) { + this.mc.setLoadBalancePingTimeout(loadBalancePingTimeout); + } + + public boolean getLoadBalanceValidateConnectionOnSwapServer() { + return this.mc.getLoadBalanceValidateConnectionOnSwapServer(); + } + + public void setLoadBalanceValidateConnectionOnSwapServer( + boolean loadBalanceValidateConnectionOnSwapServer) { + this.mc.setLoadBalanceValidateConnectionOnSwapServer(loadBalanceValidateConnectionOnSwapServer); + } public void setRetriesAllDown(int retriesAllDown) { this.mc.setRetriesAllDown(retriesAllDown); === modified file 'src/testsuite/regression/ConnectionRegressionTest.java' --- src/testsuite/regression/ConnectionRegressionTest.java 2010-03-02 23:09:37 +0000 +++ src/testsuite/regression/ConnectionRegressionTest.java 2010-03-05 20:49:06 +0000 @@ -62,11 +62,15 @@ import testsuite.BaseTestCase; import testsuite.UnreliableSocketFactory; +import com.mysql.jdbc.BalanceStrategy; +import com.mysql.jdbc.ConnectionImpl; import com.mysql.jdbc.Driver; +import com.mysql.jdbc.LoadBalancingConnectionProxy; import com.mysql.jdbc.Messages; import com.mysql.jdbc.MySQLConnection; import com.mysql.jdbc.MysqlDataTruncation; import com.mysql.jdbc.NonRegisteringDriver; +import com.mysql.jdbc.RandomBalanceStrategy; import com.mysql.jdbc.ReplicationConnection; import com.mysql.jdbc.SQLError; import com.mysql.jdbc.StandardSocketFactory; @@ -2700,4 +2704,84 @@ getUnreliableLoadBalancedConnection(new String[]{"first", "second"}, props, downedHosts).close(); } } + + public void testBug51783() throws Exception { + Properties props = new Properties(); + props.setProperty("loadBalanceStrategy", ForcedLoadBalanceStrategy.class.getName()); + props.setProperty("loadBalanceBlacklistTimeout", "5000"); + props.setProperty("loadBalancePingTimeout", "100"); + props.setProperty("loadBalanceValidateConnectionOnSwapServer", "true"); + ForcedLoadBalanceStrategy.forceFutureServer("first:3306", -1); + Connection conn2 = this.getUnreliableLoadBalancedConnection(new String[]{"first", "second"}, props); + conn2.setAutoCommit(false); + conn2.createStatement().execute("SELECT 1"); + ForcedLoadBalanceStrategy.forceFutureServer("second:3306", -1); + UnreliableSocketFactory.downHost("second"); + try{ + conn2.commit(); // will be on second after this + assertTrue("Connection should be closed", conn2.isClosed()); + } catch (SQLException e){ + fail("Should not error because failure to get another server."); + } + conn2.close(); + + props = new Properties(); + props.setProperty("loadBalanceStrategy", ForcedLoadBalanceStrategy.class.getName()); + props.setProperty("loadBalanceBlacklistTimeout", "5000"); + props.setProperty("loadBalancePingTimeout", "100"); + props.setProperty("loadBalanceValidateConnectionOnSwapServer", "false"); + ForcedLoadBalanceStrategy.forceFutureServer("first:3306", -1); + conn2 = this.getUnreliableLoadBalancedConnection(new String[]{"first", "second"}, props); + conn2.setAutoCommit(false); + conn2.createStatement().execute("SELECT 1"); + ForcedLoadBalanceStrategy.forceFutureServer("second:3306", 1); + UnreliableSocketFactory.downHost("second"); + try{ + conn2.commit(); // will be on second after this + assertFalse("Connection should not be closed, should be able to connect to first", conn2.isClosed()); + } catch (SQLException e){ + fail("Should not error because failure to get another server."); + } + } + + public static class ForcedLoadBalanceStrategy extends RandomBalanceStrategy { + + private static String forcedFutureServer = null; + private static int forceFutureServerTimes = 0; + public static void forceFutureServer(String host, int times){ + forcedFutureServer = host; + forceFutureServerTimes = times; + } + + public com.mysql.jdbc.Connection pickConnection( + LoadBalancingConnectionProxy proxy, List configuredHosts, + Map liveConnections, long[] responseTimes, int numRetries) + throws SQLException { + if(forcedFutureServer == null || forceFutureServerTimes == 0){ + return super.pickConnection(proxy, configuredHosts, liveConnections, responseTimes, numRetries); + } + if(forceFutureServerTimes > 0){ + forceFutureServerTimes--; + } + ConnectionImpl conn = (ConnectionImpl) liveConnections.get(forcedFutureServer); + + if (conn == null) { + conn = proxy.createConnectionForHost(forcedFutureServer); + + } + return conn; + } + + public void destroy() { + super.destroy(); + + } + + public void init(com.mysql.jdbc.Connection conn, Properties props) + throws SQLException { + super.init(conn, props); + + } + + } } === modified file 'src/testsuite/regression/StatementRegressionTest.java' --- src/testsuite/regression/StatementRegressionTest.java 2010-03-03 00:50:27 +0000 +++ src/testsuite/regression/StatementRegressionTest.java 2010-03-05 20:18:28 +0000 @@ -70,9 +70,11 @@ import java.util.TimeZone; import testsuite.BaseTestCase; +import testsuite.UnreliableSocketFactory; import com.mysql.jdbc.CachedResultSetMetaData; import com.mysql.jdbc.Field; +import com.mysql.jdbc.NonRegisteringDriver; import com.mysql.jdbc.ParameterBindings; import com.mysql.jdbc.ResultSetInternalMethods; import com.mysql.jdbc.SQLError; @@ -6371,6 +6373,34 @@ assertEquals(s+ 1, rs.getInt(2)); } + + public void testBug51776() throws Exception { + + Properties props = new Properties(); + NonRegisteringDriver d = new NonRegisteringDriver(); + this.copyBasePropertiesIntoProps(props, d); + props.setProperty("socketFactory", "testsuite.UnreliableSocketFactory"); + Properties parsed = d.parseURL(BaseTestCase.dbUrl, props); + String db = parsed.getProperty(NonRegisteringDriver.DBNAME_PROPERTY_KEY); + String port = parsed.getProperty(NonRegisteringDriver.PORT_PROPERTY_KEY); + String host = getPortFreeHostname(props, d); + UnreliableSocketFactory.flushAllHostLists(); + UnreliableSocketFactory.mapHost("first", host); + props.remove(NonRegisteringDriver.HOST_PROPERTY_KEY); + + Connection testConn = getConnectionWithProps("jdbc:mysql://first:" + port + "/" + db, props); + testConn.setAutoCommit(false); + testConn.createStatement().execute("SELECT 1" ); + UnreliableSocketFactory.downHost("first"); + try{ + testConn.rollback(); + fail("Should receive SQLException on rollback()."); + } catch (SQLException e){ + + } + + } + public static class IncrementStatementCountInterceptor implements StatementInterceptorV2{