Browse files

Merge branch '1.2.6/fix_race_condition'

Conflicts:
	pom.xml
	src/main/java/com/tacitknowledge/util/migration/jdbc/JdbcMigrationLauncherFactory.java
	src/main/java/com/tacitknowledge/util/migration/jdbc/PatchTable.java
	src/main/resources/com/tacitknowledge/util/migration/jdbc/hsqldb.properties
	src/main/resources/com/tacitknowledge/util/migration/jdbc/mysql.properties
	src/main/resources/com/tacitknowledge/util/migration/jdbc/oracle.properties
	src/main/resources/com/tacitknowledge/util/migration/jdbc/postgres.properties
	src/main/resources/com/tacitknowledge/util/migration/jdbc/sqlserver.properties
	src/main/resources/com/tacitknowledge/util/migration/jdbc/sybase.properties
	src/test/java/com/tacitknowledge/util/migration/jdbc/PatchTableTest.java
  • Loading branch information...
2 parents 4fce396 + da6c6a3 commit 2a9496dc2674a8f47a689fedda50114b288e0239 @ulisespulido ulisespulido committed Dec 1, 2011
View
2 pom.xml
@@ -28,7 +28,7 @@
<target.jdk>1.4</target.jdk>
<unit.tests.pattern>**/*Test.java</unit.tests.pattern>
<integration.tests.pattern>
- **/MigrationUnlockTest.java,**/MultiNodeAutoPatchTest.java,**/MissingPatchMigrationRunnerStrategyIntegrationTest.java
+ **/MigrationUnlockTest.java,**/MultiNodeAutoPatchTest.java,**/MissingPatchMigrationRunnerStrategyIntegrationTest.java,**/MultiServerRaceConditionTest.java
</integration.tests.pattern>
</properties>
View
111 ...integration-test/java/com/tacitknowledge/util/migration/MultiServerRaceConditionTest.java
@@ -0,0 +1,111 @@
+package com.tacitknowledge.util.migration;
+
+import java.sql.*;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+import junit.framework.TestCase;
+
+import com.tacitknowledge.util.migration.MigrationContext;
+import com.tacitknowledge.util.migration.jdbc.JdbcMigrationLauncher;
+import com.tacitknowledge.util.migration.jdbc.JdbcMigrationLauncherFactory;
+
+/**
+ * Pin-points a race condition when the AutoPatcher is run from multiple servers on the same schema.
+ *
+ * @author Jeff Kolesky (jeff@kolesky.com)
+ */
+public class MultiServerRaceConditionTest extends TestCase
+{
+ private static Log log = LogFactory.getLog(MultiServerRaceConditionTest.class);
+
+ /**
+ * Constructor
+ *
+ * @param name the name of the test to run
+ */
+ public MultiServerRaceConditionTest(String name)
+ {
+ super(name);
+ }
+
+ public void tearDown() throws Exception
+ {
+ super.tearDown();
+ Connection conn = DriverManager.getConnection("jdbc:hsqldb:mem:race", "sa", "");
+ Statement stmt = conn.createStatement();
+ stmt.execute("SHUTDOWN");
+ }
+
+ /**
+ * Tests that two servers both booting up the AutoPatcher will not both try to apply the same patches
+ *
+ * @exception Exception if anything goes wrong
+ */
+ public void testMultiServerRaceCondition() throws Exception
+ {
+ log.debug("Testing multi server race condition");
+ MigrationThread a = new MigrationThread("A");
+ MigrationThread b = new MigrationThread("B");
+
+ a.start();
+ b.start();
+
+ boolean success = true;
+
+ success &= finish(a);
+ success &= finish(b);
+
+ if (!success)
+ {
+ fail("shouldn't have thrown any exceptions");
+ }
+ }
+
+ private boolean finish(MigrationThread t) throws Exception
+ {
+ t.join();
+ Exception error = t.getError();
+ if (error != null)
+ {
+ return false;
+ }
+ return true;
+ }
+
+ private static class MigrationThread extends Thread
+ {
+ private JdbcMigrationLauncherFactory factory;
+ private JdbcMigrationLauncher launcher;
+ private Exception error;
+
+ private MigrationThread(String name) throws Exception
+ {
+ super(name);
+ this.factory = new JdbcMigrationLauncherFactory();
+ this.launcher = this.factory.createMigrationLauncher("race", "multiserver-inttest-migration.properties");
+ // initialize the patch table *not* in parallel
+ this.launcher.getDatabasePatchLevel((MigrationContext)this.launcher.getContexts().keySet().iterator().next());
+ }
+
+ public Exception getError()
+ {
+ return this.error;
+ }
+
+ public void run()
+ {
+ try
+ {
+ this.launcher.doMigrations();
+ }
+ catch (Exception e)
+ {
+ this.error = e;
+ log.error("Unexpected error", this.error);
+ }
+ }
+ }
+}
+
View
5 ...ces/com/tacitknowledge/util/migration/inttest-tasks/race/patch00001_create_race_table.sql
@@ -0,0 +1,5 @@
+create table race (
+ key_name varchar(20) not null,
+ field_value varchar(40) not null,
+ constraint race_uk unique (key_name, field_value)
+);
View
1 ...rces/com/tacitknowledge/util/migration/inttest-tasks/race/patch00002_insert_into_race.sql
@@ -0,0 +1 @@
+insert into race (key_name, field_value) values ('count', 'One');
View
1 ...es/com/tacitknowledge/util/migration/inttest-tasks/race/patch00003_insert_into_race_2.sql
@@ -0,0 +1 @@
+insert into race (key_name, field_value) values ('count', 'Two');
View
1 ...es/com/tacitknowledge/util/migration/inttest-tasks/race/patch00004_insert_into_race_3.sql
@@ -0,0 +1 @@
+insert into race (key_name, field_value) values ('count', 'Three');
View
10 src/integration-test/resources/multiserver-inttest-migration.properties
@@ -0,0 +1,10 @@
+#
+# Configure a context named "race"
+#
+race.jdbc.database.type=hsqldb
+race.jdbc.driver=org.hsqldb.jdbcDriver
+race.jdbc.url=jdbc:hsqldb:mem:race
+race.jdbc.username=sa
+race.jdbc.password=
+race.patch.path=com.tacitknowledge.util.migration.inttest-tasks.race
+race.lockPollMillis=3000
View
7 src/main/java/com/tacitknowledge/util/migration/jdbc/JdbcMigrationLauncherFactory.java
@@ -316,6 +316,13 @@ void configureFromMigrationProperties(JdbcMigrationLauncher launcher, String sys
launcher.setLockPollRetries(Integer.parseInt(lockPollRetries));
}
+ // See if they want to change the amount of time to wait between lock polls
+ String lockPollMillis = props.getProperty(system + ".lockPollMillis");
+ if (lockPollMillis != null)
+ {
+ launcher.setLockPollMillis(Integer.parseInt(lockPollMillis));
+ }
+
// TODO refactor the database name extraction from this and the servlet example
String databases = props.getProperty(system + ".jdbc.systems");
String[] databaseNames;
View
30 src/main/java/com/tacitknowledge/util/migration/jdbc/PatchTable.java
@@ -98,7 +98,7 @@ public void createPatchStoreIfNeeded() throws MigrationException
{
conn = context.getConnection();
- stmt = conn.prepareStatement(getSql("level.read"));
+ stmt = conn.prepareStatement(getSql("level.table.exists"));
stmt.setString(1, context.getSystemName());
rs = stmt.executeQuery();
log.debug("'patches' table already exists.");
@@ -127,6 +127,9 @@ public void createPatchStoreIfNeeded() throws MigrationException
}
stmt.execute();
context.commit();
+
+ // We don't yet have a patch record for this system; create one
+ createSystemPatchRecord();
}
catch (SQLException sqle)
{
@@ -171,8 +174,6 @@ public int getPatchLevel() throws MigrationException
stmt = null;
rs = null;
- // We don't yet have a patch record for this system; create one
- createSystemPatchRecord();
return 0;
}
catch (SQLException e)
@@ -256,11 +257,11 @@ public boolean isPatchStoreLocked() throws MigrationException
*/
public void lockPatchStore() throws MigrationException, IllegalStateException
{
- if (isPatchStoreLocked())
+ createPatchStoreIfNeeded();
+ if (!updatePatchLock(true))
{
throw new IllegalStateException("Patch table is already locked!");
}
- updatePatchLock(true);
}
/**
@@ -377,13 +378,15 @@ private void createSystemPatchRecord() throws MigrationException, SQLException
}
/**
- * Obtains or releases a lock for this system in the patches table.
+ * Obtains or releases a lock for this system in the patches table. If the lock
+ * was not obtained or released, then <code>false</code> is returned.
*
- * @param lock <code>true</code> if a lock is to be obtained, <code>false</code>
- * if it is to be removed
+ * @param lock <code>true</code> if a lock is to be obtained, <code>false</code>
+ * if it is to be removed
+ * @return true if the lock was updated successfully, otherwise false
* @throws MigrationException if an unrecoverable database error occurs
*/
- private void updatePatchLock(boolean lock) throws MigrationException
+ private boolean updatePatchLock(boolean lock) throws MigrationException
{
String sqlkey = (lock) ? "lock.obtain" : "lock.release";
Connection conn = null;
@@ -402,8 +405,15 @@ private void updatePatchLock(boolean lock) throws MigrationException
{
stmt.setString(2, context.getSystemName());
}
- stmt.execute();
+
+ int rowsUpdated = stmt.executeUpdate();
+ boolean lockUpdated = (rowsUpdated == 1);
context.commit();
+ if (log.isDebugEnabled())
+ {
+ log.debug(((lock) ? "Obtained" : "Released") + " lock? " + lockUpdated);
+ }
+ return lockUpdated;
}
catch (SQLException e)
{
View
3 src/main/resources/com/tacitknowledge/util/migration/jdbc/hsqldb.properties
@@ -9,6 +9,7 @@ patches.create=CREATE TABLE patches ( \
# Validates that a record exists for a given system
level.create=INSERT INTO patches (system_name, patch_level) VALUES ( ?, 0)
+level.table.exists=SELECT patch_level FROM patches WHERE system_name = ?
level.read=SELECT MAX(patch_level) FROM patches WHERE system_name = ?
level.rollback=DELETE FROM patches WHERE patch_level = ? and system_name = ?
level.update=INSERT INTO patches (patch_level, system_name, patch_date) VALUES ( ?, ?, NOW())
@@ -19,5 +20,5 @@ patches.all=SELECT patch_level FROM patches WHERE system_name = ?
# Since most DBs do not have a boolean type, return 0 or 1 row to determine if
# the system is currently locked.
lock.read=SELECT patch_in_progress FROM patches WHERE system_name = ? AND ( patch_in_progress <> 'F' OR patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? ))
-lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
+lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_in_progress = 'F' AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
lock.release=UPDATE patches SET patch_in_progress = 'F' WHERE system_name = ? AND patch_in_progress <> 'F'
View
3 src/main/resources/com/tacitknowledge/util/migration/jdbc/mysql.properties
@@ -9,6 +9,7 @@ patches.create=CREATE TABLE IF NOT EXISTS patches ( \
# Validates that a record exists for a given system
level.create=INSERT INTO patches (system_name, patch_level) VALUES ( ?, 0 )
+level.table.exists=SELECT patch_level FROM patches WHERE system_name = ?
level.read=SELECT MAX(patch_level) FROM patches WHERE system_name = ?
level.rollback=DELETE FROM patches WHERE patch_level = ? and system_name = ?
level.update=INSERT INTO patches (patch_level, system_name, patch_date) VALUES ( ?, ?, CURRENT_TIMESTAMP)
@@ -19,5 +20,5 @@ patches.all=SELECT patch_level FROM patches WHERE system_name = ?
# Since most DBs do not have a boolean type, return 0 or 1 row to determine if
# the system is currently locked.
lock.read=SELECT patch_in_progress FROM patches WHERE system_name = ? AND ( patch_in_progress <> 'F' OR patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? ))
-lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
+lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_in_progress = 'F' AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
lock.release=UPDATE patches SET patch_in_progress = 'F' WHERE system_name = ? AND patch_in_progress <> 'F'
View
3 src/main/resources/com/tacitknowledge/util/migration/jdbc/oracle.properties
@@ -9,6 +9,7 @@ patches.create=CREATE TABLE tk_patches (\
# Validates that a record exists for a given system
level.create=INSERT INTO tk_patches (system_name, patch_level) VALUES ( ?, 0 )
+level.table.exists=SELECT patch_level FROM patches WHERE system_name = ?
level.read=SELECT MAX(patch_level) FROM tk_patches WHERE system_name = ?
level.rollback=DELETE FROM tk_patches WHERE patch_level = ? and system_name = ?
level.update=INSERT INTO tk_patches (patch_level, system_name, patch_date) VALUES ( ?, ?, SYSDATE)
@@ -19,5 +20,5 @@ patches.all=SELECT patch_level FROM tk_patches WHERE system_name = ?
# Since most DBs do not have a boolean type, return 0 or 1 row to determine if
# the system is currently locked.
lock.read=SELECT patch_in_progress FROM tk_patches WHERE system_name = ? AND ( patch_in_progress <> 'F' OR patch_level in ( SELECT MAX(patch_level) FROM tk_patches WHERE system_name = ? ))
-lock.obtain=UPDATE tk_patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_level in ( SELECT MAX(patch_level) FROM tk_patches WHERE system_name = ? )
+lock.obtain=UPDATE tk_patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_in_progress = 'F' AND patch_level in ( SELECT MAX(patch_level) FROM tk_patches WHERE system_name = ? )
lock.release=UPDATE tk_patches SET patch_in_progress = 'F' WHERE system_name = ? AND patch_in_progress <> 'F'
View
3 src/main/resources/com/tacitknowledge/util/migration/jdbc/postgres.properties
@@ -9,6 +9,7 @@ patches.create=CREATE TABLE patches ( \
# Validates that a record exists for a given system
level.create=INSERT INTO patches (system_name, patch_level) VALUES ( ?, 0 )
+level.table.exists=SELECT patch_level FROM patches WHERE system_name = ?
level.read=SELECT MAX(patch_level) FROM patches WHERE system_name = ?
level.rollback=DELETE FROM patches WHERE patch_level = ? and system_name = ?
level.update=INSERT INTO patches (patch_level, system_name, patch_date) VALUES ( ?, ?, now())
@@ -19,5 +20,5 @@ patches.all=SELECT patch_level FROM patches WHERE system_name = ?
# Since most DBs do not have a boolean type, return 0 or 1 row to determine if
# the system is currently locked.
lock.read=SELECT patch_in_progress FROM patches WHERE system_name = ? AND ( patch_in_progress <> 'F' OR patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? ))
-lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
+lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_in_progress = 'F' AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
lock.release=UPDATE patches SET patch_in_progress = 'F' WHERE system_name = ? AND patch_in_progress <> 'F'
View
3 src/main/resources/com/tacitknowledge/util/migration/jdbc/sqlserver.properties
@@ -9,6 +9,7 @@ patches.create=CREATE TABLE patches ( \
# Validates that a record exists for a given system
level.create=INSERT INTO patches (system_name, patch_level) VALUES ( ?, 0)
+level.table.exists=SELECT patch_level FROM patches WHERE system_name = ?
level.read=SELECT MAX(patch_level) FROM patches WHERE system_name = ?
level.rollback=DELETE FROM patches WHERE patch_level = ? and system_name = ?
level.update=INSERT INTO patches (patch_level, system_name, patch_date) VALUES ( ?, ?, getDate())
@@ -19,5 +20,5 @@ patches.all=SELECT patch_level FROM patches WHERE system_name = ?
# Since most DBs do not have a boolean type, return 0 or 1 row to determine if
# the system is currently locked.
lock.read=SELECT patch_in_progress FROM patches WHERE system_name = ? AND ( patch_in_progress <> 'F' OR patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? ))
-lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
+lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_in_progress = 'F' AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
lock.release=UPDATE patches SET patch_in_progress = 'F' WHERE system_name = ? AND patch_in_progress <> 'F'
View
3 src/main/resources/com/tacitknowledge/util/migration/jdbc/sybase.properties
@@ -8,6 +8,7 @@ patches.create=CREATE TABLE patches (\
# Validates that a record exists for a given system
level.create=INSERT INTO patches (system_name, patch_level) VALUES ( ?, 0 )
+level.table.exists=SELECT patch_level FROM patches WHERE system_name = ?
level.read=SELECT MAX(patch_level) FROM patches WHERE system_name = ?
level.rollback=DELETE FROM patches WHERE patch_level = ? and system_name = ?
level.update=INSERT INTO patches (patch_level, system_name, patch_date) VALUES ( ?, ?, getdate())
@@ -18,5 +19,5 @@ patches.all=SELECT patch_level FROM patches WHERE system_name = ?
# Since most DBs do not have a boolean type, return 0 or 1 row to determine if
# the system is currently locked.
lock.read=SELECT patch_in_progress FROM patches WHERE system_name = ? AND ( patch_in_progress <> 'F' OR patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? ))
-lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
+lock.obtain=UPDATE patches SET patch_in_progress = 'T' WHERE system_name = ? AND patch_in_progress = 'F' AND patch_level in ( SELECT MAX(patch_level) FROM patches WHERE system_name = ? )
lock.release=UPDATE patches SET patch_in_progress = 'F' WHERE system_name = ? AND patch_in_progress <> 'F'
View
18 src/test/java/com/tacitknowledge/util/migration/jdbc/PatchTableTest.java
@@ -79,7 +79,7 @@ protected void setUp() throws Exception
context = new DataSourceMigrationContext();
context.setDataSource(new ConnectionWrapperDataSource(conn));
context.setSystemName("milestone");
- context.setDatabaseType(new DatabaseType("postgres"));
+ context.setDatabaseType(new DatabaseType("hsqldb"));
table = new PatchTable(context);
contextControl = MockControl.createControl(JdbcMigrationContext.class);
@@ -113,7 +113,7 @@ public void testCreatePatchesTable() throws Exception
{
// Test-specific setup
PreparedStatementResultSetHandler h = conn.getPreparedStatementResultSetHandler();
- h.prepareThrowsSQLException(table.getSql("level.read"));
+ h.prepareThrowsSQLException(table.getSql("level.table.exists"));
table.createPatchStoreIfNeeded();
@@ -204,7 +204,8 @@ public void testGetPatchLevelFirstTime() throws Exception
handler = conn.getPreparedStatementResultSetHandler();
MockResultSet rs = handler.createResultSet();
// empty result set
- handler.prepareGlobalResultSet(rs);
+ handler.prepareResultSet(table.getSql("level.read"), rs);
+ handler.prepareThrowsSQLException(table.getSql("level.table.exists"));
int i = table.getPatchLevel();
@@ -282,9 +283,7 @@ public void testLockPatchTableWhenAlreadyLocked() throws Exception
// Test-specific setup
// Return a non-empty set in response to the patch lock query
handler = conn.getPreparedStatementResultSetHandler();
- MockResultSet rs = handler.createResultSet();
- rs.addRow(new String[]{"T"});
- handler.prepareResultSet(table.getSql("lock.read"), rs, new String[]{"milestone", "milestone"});
+ handler.prepareUpdateCount(table.getSql("lock.obtain"), 0, new String[] {"milestone", "milestone"});
try
{
@@ -296,9 +295,10 @@ public void testLockPatchTableWhenAlreadyLocked() throws Exception
// Expected
}
- verifyPreparedStatementNotPresent(table.getSql("lock.obtain"));
+ verifyPreparedStatementParameter(table.getSql("lock.obtain"), 1, "milestone");
+ verifyPreparedStatementParameter(table.getSql("lock.obtain"), 2, "milestone");
commonVerifications();
- verifyNotCommitted();
+ verifyCommitted();
}
/**
@@ -313,7 +313,7 @@ public void testLockPatchTableWhenNotAlreadyLocked() throws Exception
// Return an empty set in response to the patch lock query
handler = conn.getPreparedStatementResultSetHandler();
MockResultSet rs = handler.createResultSet();
- handler.prepareResultSet(table.getSql("lock.read"), rs, new String[]{"milestone", "milestone"});
+ handler.prepareUpdateCount(table.getSql("lock.obtain"), 1, new String[] {"milestone", "milestone"});
table.lockPatchStore();
verifyPreparedStatementParameter(table.getSql("lock.obtain"), 1, "milestone");

0 comments on commit 2a9496d

Please sign in to comment.