From 5c7aee7d38c3c2d37ae1abc61864736ba5a71c7e Mon Sep 17 00:00:00 2001 From: Thomas Risberg Date: Fri, 6 Jan 2017 16:34:45 -0500 Subject: [PATCH] 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 --- .../incrementer/MySQLMaxValueIncrementer.java | 104 +++++++++++++++--- 1 file changed, 91 insertions(+), 13 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/support/incrementer/MySQLMaxValueIncrementer.java b/spring-jdbc/src/main/java/org/springframework/jdbc/support/incrementer/MySQLMaxValueIncrementer.java index 2c3ad290a20..ec17e1a5154 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/support/incrementer/MySQLMaxValueIncrementer.java +++ b/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 @@ import org.springframework.jdbc.support.JdbcUtils; * key column should NOT be auto-increment, as the sequence table does the job. * *

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. * *

Example: * * 

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);
* * 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. + * + *

It is possible to avoid acquiring a new connection for the incrementer by setting the + * "useNewConnection" property to false. In this case you MUST 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 class MySQLMaxValueIncrementer extends AbstractColumnMaxValueIncrementer 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 @@ public class MySQLMaxValueIncrementer extends AbstractColumnMaxValueIncrementer } 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 {