Dselans/concurrency support

[dselans]

trafficstars

Hey folks,

So this is a semi-large PR for functionality that was requested in #45. We ran into the exact same need -- our applications run in docker containers on an ephemeral set of nodes and perform in-app migrations. The sql-migrate lib is fantastic, but when used concurrently, you are likely to run into migration collisions. Someone mentioned that this should be handled at a different layer and while that can be done, we have little control over the ops side of things and adding "cluster awareness" to each different service seems .. hacky and difficult to scale (if we're talking about microservices).

#45 suggested to use vendor-specific lock implementations -- while probably doable (and "proper"), it would be a pretty large undertaking. I went with the model of using a db-based mutex instead (ie. a lock record in a gorp_lock table), that all "migrators" attempt to insert, but only one wins. This ensures that it should work across most db's, without using any vendor-specific lock code (except for sqlite, more on that later).

Implementation and notes:

• Introduced two new functions ExecWithLock() and ExecMaxWithLock() which will utilize a db mutex to ensure only one 'master' migrator is performing migrations
• Any sql-migrate instances that were not able to acquire the lock go into a wait-state, periodically checking to see if the lock disappears.
  • If the lock disappears before waitTime is up - the 'wait state' migrators finish cleanly, with no errors (and 0 completed migrations)
  • If the lock does not get removed before waitTime is up - the additional sql-migrate instances will error with a Exceeded lock clearance wait time message. You can then tune the waitTime accordingly (it's a param passed to ExecWithLock())
• In the process of updating tests, I realized that the lock implementation won't work with sqlite due to concurrent access causing database is locked errors. It's totally possible that I just missed something obvious, but the easiest route seemed to be to just enable MySQL based integration tests, so I introduced another test suite (MySQLMigrateSuite), which thoroughly tests the lock implementation.
  • MySQL based testing can be enabled via a new flag passed to go test called -enable-mysql
  • SQLite tests can now also be disabled via -disable-sqlite (default: false); this was done to speed up testing (as the sqlite tests seemed to be pretty slow on my machine)
• I also went ahead and updated the .travis.yml to enable mysql based testing as well

With that said, please let me know if you've got any questions -- this functionality is really crucial for our team and I would much rather prefer running off of upstream than our little fork :-)

TIA!

[rubenv]

Hi, sorry for the late reply.

As mentioned in #45, I don't think this functionality has a place in sql-migrate, there's too many different ways to do this and I want to keep things simple.

There's nothing stopping you from putting this functionality in a class that decorates the functionality from sql-migrate, it can be perfectly separate.

[dselans]

Hi @rubenv, thanks for your update.

I suspect more and more folks are launching their apps in parallel -- it would be nice to support this sort of functionality out of the box (without requiring everyone to write their own, custom wrappers). The functionality I added does not modify any existing code - it adds two new functions, so folks don't have to use it if they don't want to.

With that said, this is your project, if you do not want to take on the potential tech debt, that's understandable. I still feel that others may benefit from this, so I'll keep the alternate fork running.