search

tacitknowledge / autopatch

An automated database patching framework for Java.
41 stars 19 forks source link

Race condition running Autopatcher from multiple servers #28

Closed jeffkole closed 12 years ago

jeffkole commented 12 years ago

I believe the original design of the Autopatcher was to allow it to be run from all app servers in a cluster without causing any problems. In this mode, you could stop all servers, deploy new code, start them all up, and they would wait until one of them successfully migrated the schema. I believe this used to work, way back when the Autopatcher was managed in CVS.

Recently, I have run into a race condition where the second of two servers will attempt to run the same set of patches while the first is running the patches itself. I have modeled this race condition behavior in an integration test that I committed to my repo. It is based off of 1.2.6, since that is what I am running in production.

Check it out.

https://github.com/jeffkole/autopatch/commit/9eeb12b5591341749fc57c9c28e229de1c89d95c

I am happy to work on a fix for this issue if it is indeed violating the design of the Autopatcher.

Jeff

jeffkole commented 12 years ago

I committed a fix against version 1.2.6. It should be relatively easy to merge into master, even with the new paradigm for tracking patches. Please look at this fix and let me know if you would like me to create a 1.3.0 version.

scottfromsf commented 12 years ago

Jeff,

You're spot on: AutoPatch was design from Day 1 to work across a cluster such that you could start all servers simultaneously and only one would execute the patches while the others waited for the schema to be up-to-date. Any deviation from this behavior is a bug.

Thanks a ton for writing a test to prove that there's an issue and for creating a patch. I'll make sure one of the developers tests it and merges it back into the core project.

-scott

scottfromsf commented 12 years ago

Ulises, can you add Jeff's repo as a remote, merge his change locally and test it? Assuming it checks out please push it to the TK repo and I'll release a new build to Maven Central. Thanks!

ulisespulido commented 12 years ago

Scott, Jeff

There were some conflicts during merge, right now the MultiServerRaceConditionTest is not passing, will verify why this is happening.

Thanks

ulisespulido commented 12 years ago

There was an issue while creating the patch table not inserting the first value due MAX returned 0 instead of an empty value for level.read query, now is working.

commit 2a9496dc2674a8f47a689fedda50114b288e0239 solves this issue.

Please verify so I can close this issue.

jeffkole commented 12 years ago

Looks good to me. Thanks.

scottfromsf commented 12 years ago

I pushed Autopatch 1.3.1 with these changes up to the Maven Central Repo. Thanks again, Jeff.