percolatestudio / meteor-migrations

Simple migration system for Meteor
https://atmospherejs.com/percolate/migrations
MIT License
245 stars 58 forks source link

implement atomic locking #38

Closed meonkeys closed 9 years ago

meonkeys commented 9 years ago

This gracefully handles the case where many Meteor apps (perhaps a cluster) connect to the same database and simultaneously attempt to run [the same] database migration(s).

I split setLocked() into two functions because they are now implemented quite differently. I didn't see an easy way to reuse _setControl() in lock(), but there surely is some way to achieve that. When I added a selector it just seemed to make the function too messy, and I didn't want to always update the version, so then I added a modifier too... and then I'm basically passing exactly what I'd pass to a vanilla update(). Some work could be done to reduce the number of side effects in the code--that might improve reusability.

Previously the lock was only checked in one place, but locked and unlocked in many places. I didn't see a benefit from the fine-grained lock except a greater chance for a race condition.

I expanded the locked region, making the lock usage more coarse. This provides safer and faster failing and simplifies trying to track down when we're locked and unlocked while reading the _migrateTo() code. There's no performance implication here; migrations should be run once on server startup.

At any rate, this works. All tests pass. Closes #37.

meonkeys commented 9 years ago

@zol any thoughts on this PR?

meonkeys commented 9 years ago

:bug:

zol commented 9 years ago

Looks great @meonkeys , thanks!