Updating MySQLMaxValueIncrementer to not rely on MYISAM

We should no longer rely on MYISAM for the sequence table since this
engine might not always be available. After this change the storage
engine used by the sequence table can be MYISAM or INNODB since the
sequences are allocated using a new connection without being
affected by any other transactions that might be in progress.

To allow users to fall back on the original functionality of using
MYISAM tables for the incrementer, we add a `useNewConnection` flag to
indicate whether or not to use a new connection for the incrementer.
This flag defaults to true.

Issue: SPR-15107

Author: trisberg

spring-jdbc/src/main/java/org/springframework/jdbc/support/incrementer/MySQLMaxValueIncrementer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -33,20 +33,24 @@
  * key column should <i>NOT</i> be auto-increment, as the sequence table does the job.
  *
  * <p>The sequence is kept in a table; there should be one sequence table per
- * table that needs an auto-generated key. The table type of the sequence table
- * should be MyISAM so the sequences are allocated without regard to any
- * transactions that might be in progress.
+ * table that needs an auto-generated key. The storage engine used by the sequence table
+ * can be MYISAM or INNODB since the sequences are allocated using a separate connection 
+ * without being affected by any other transactions that might be in progress.
  *
  * <p>Example:
  *
  * <pre class="code">create table tab (id int unsigned not null primary key, text varchar(100));
- * create table tab_sequence (value int not null) type=MYISAM;
+ * create table tab_sequence (value int not null);
  * insert into tab_sequence values(0);</pre>
  *
  * If "cacheSize" is set, the intermediate values are served without querying the
  * database. If the server or your application is stopped or crashes or a transaction
  * is rolled back, the unused values will never be served. The maximum hole size in
  * numbering is consequently the value of cacheSize.
+ * 
+ * <p>It is possible to avoid acquiring a new connection for the incrementer by setting the
+ * "useNewConnection" property to false. In this case you <i>MUST</i> use a non-transactional
+ * storage engine like MYISAM when defining the incrementer table.
  *
  * @author Jean-Pierre Pawlak
  * @author Thomas Risberg
@@ -63,6 +67,14 @@ public class MySQLMaxValueIncrementer extends AbstractColumnMaxValueIncrementer
 	/** The max id to serve */
 	private long maxId = 0;
 
+	/**
+	 * Whether or not to use a new connection for the incrementer. Defaults to true
+	 * in order to support transactional storage engines. Set this to false if the storage engine
+	 * for the  incrementer table is non-transactional like MYISAM and you prefer to not acquire
+	 * an additional database connection
+	 */
+	private boolean useNewConnection = true;
+
 
 	/**
 	 * Default constructor for bean property style usage.
@@ -83,24 +95,73 @@ public MySQLMaxValueIncrementer(DataSource dataSource, String incrementerName, S
 		super(dataSource, incrementerName, columnName);
 	}
 
+	/**
+	 * Convenience constructor for setting whether to use a new connection for the incrementer.
+	 * @param dataSource the DataSource to use
+	 * @param incrementerName the name of the sequence/table to use
+	 * @param columnName the name of the column in the sequence table to use
+	 * @param useNewConnection whether to use a new connection for the incrementer
+	 */
+	public MySQLMaxValueIncrementer(DataSource dataSource, String incrementerName, String columnName,
+	                                boolean useNewConnection) {
+		super(dataSource, incrementerName, columnName);
+		this.useNewConnection = useNewConnection;
+	}
+
+
+	/**
+	 * Return whether to use a new connection for the incrementer.
+	 */
+	public boolean isUseNewConnection() {
+		return useNewConnection;
+	}
+
+	/**
+	 * Set whether to use a new connection for the incrementer.
+	 */
+	public void setUseNewConnection(boolean useNewConnection) {
+		this.useNewConnection = useNewConnection;
+	}
 
 	@Override
 	protected synchronized long getNextKey() throws DataAccessException {
 		if (this.maxId == this.nextId) {
 			/*
-			* Need to use straight JDBC code because we need to make sure that the insert and select
-			* are performed on the same connection (otherwise we can't be sure that last_insert_id()
-			* returned the correct value)
+			* If useNewConnection is true, then we obtain a non-managed connection so our modifications
+			* are handled in a separate transaction. If it is false, then we use the current transaction's
+			* connection relying on the use of a non-transactional storage engine like MYISAM for the
+			* incrementer table. We also use straight JDBC code because we need to make sure that the insert
+			* and select are performed on the same connection (otherwise we can't be sure that last_insert_id()
+			* returned the correct value).
 			*/
-			Connection con = DataSourceUtils.getConnection(getDataSource());
+			Connection con = null;
 			Statement stmt = null;
+			boolean mustRestoreAutoCommit = false;
 			try {
+				if (useNewConnection) {
+					con = getDataSource().getConnection();
+					if (con.getAutoCommit()) {
+						mustRestoreAutoCommit = true;
+						con.setAutoCommit(false);
+					}
+				}
+				else {
+					con = DataSourceUtils.getConnection(getDataSource());
+				}
 				stmt = con.createStatement();
-				DataSourceUtils.applyTransactionTimeout(stmt, getDataSource());
+				if (!useNewConnection) {
+					DataSourceUtils.applyTransactionTimeout(stmt, getDataSource());
+				}
 				// Increment the sequence column...
 				String columnName = getColumnName();
-				stmt.executeUpdate("update "+ getIncrementerName() + " set " + columnName +
-						" = last_insert_id(" + columnName + " + " + getCacheSize() + ")");
+				try {
+					stmt.executeUpdate("update " + getIncrementerName() + " set " + columnName +
+							" = last_insert_id(" + columnName + " + " + getCacheSize() + ")");
+				}
+				catch (SQLException ex) {
+					throw new DataAccessResourceFailureException("Could not increment " + columnName + " for " +
+							getIncrementerName() + " sequence table", ex);
+				}
 				// Retrieve the new max of the sequence column...
 				ResultSet rs = stmt.executeQuery(VALUE_SQL);
 				try {
@@ -119,7 +180,24 @@ protected synchronized long getNextKey() throws DataAccessException {
 			}
 			finally {
 				JdbcUtils.closeStatement(stmt);
-				DataSourceUtils.releaseConnection(con, getDataSource());
+				if (useNewConnection) {
+					try {
+						con.commit();
+						if (mustRestoreAutoCommit) {
+							con.setAutoCommit(true);
+						}
+					}
+					catch (SQLException ignore) {
+						throw new DataAccessResourceFailureException(
+								"Unable to commit new sequence value changes for " + getIncrementerName());
+					}
+					try {
+						con.close();
+					} catch (SQLException ignore) {}
+				}
+				else {
+					DataSourceUtils.releaseConnection(con, getDataSource());
+				}
 			}
 		}
 		else {