Merge pull request #1286 from trisberg/SPR-15107

SPR-15107 Updating MySQLMaxValueIncrementer to not rely on MYISAM

Author: jhoeller

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 {