migrating-ravens / RavenMigrations

A small migrations framework to help you manage your RavenDB Instance.
MIT License
53 stars 24 forks source link

Consider preventing migrations from running simultaneously from multiple web apps #50

Closed JudahGabriel closed 4 years ago

JudahGabriel commented 5 years ago

As described in Andrew Locke's post on why running migrations on startup is a bad idea, his objection boils down to having multiple apps running simultaneously. One app starts running migrations, the next app starts running migrations, and now we've got trouble.

We may want to use Raven's CompareXChange to enter a migrations cluster-wide "lock", so that no other migrations can be run from other app instances.

SbiCA commented 4 years ago

@JudahGabriel I agree that this should be a feature, I guess a simple solution could be to use a lock document since that's the common resource amongst the app instances and using session.Load<MigrationLock>("THE LOCK ID") == null ? "Good To Go" : "Wait or cancel" . Or at least provide a hook e.g. virtual method in MigrationRunner to allow overriding the run behaviour.

SbiCA commented 4 years ago

I created a small sample how this could look like let me know if I should create a PR sample code

JudahGabriel commented 4 years ago

That will work in a single node, but isn't reliable in a cluster.

For example, server A begins migration, stores the lock document in node X. Meanwhile, server B begins migration and stores the lock document in node Y. Now they're both running the migrations because node X hasn't yet propagated changes to node Y.

Instead of a lock document, you should use Raven compare/exchange, which works cluster-wide. Bonus: no need for a LockDocument type.

JudahGabriel commented 4 years ago

If you can update your sample to use compare/exchange, I'd be glad to accept that as a PR.

SbiCA commented 4 years ago

@JudahGabriel totally I agree I update the sample to use CompareExchange it's much simpler 😄 I think it could have also worked with the lock document if when using Write assurance and database groups but I prefer the Exchange approach.

JudahGabriel commented 4 years ago

Great. I'd gladly accept a PR with those changes.

SbiCA commented 4 years ago

I'll try to do create the PR this week

JudahGabriel commented 4 years ago

I've released RavenMigrations 4.3, which prevents simultaneous migrations by default. 😎