Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Locking with MongoDB #435

Closed
SJrX opened this issue Sep 1, 2020 · 5 comments · Fixed by #448
Closed

Implement Locking with MongoDB #435

SJrX opened this issue Sep 1, 2020 · 5 comments · Fixed by #448

Comments

@SJrX
Copy link
Contributor

SJrX commented Sep 1, 2020

Is your feature request related to a problem? Please describe.
We would like to be able to run go-migrate in parallel on multiple containers without requiring central orchestration with MongoDB

Describe the solution you'd like
We'd like to have Lock and Unlock implemented with MongoDB

Describe alternatives you've considered
We've looked at other tools (other than golang-migrate), but they all were unsatisfactory for a number of reasons. We also don't want to make our deployments more cumbersome as well.

Additional context
We are planning on actually implementing advisory locking with Mongo DB internally, and hope to submit a PR. This Feature Request is just to track and solicit feedback. Mongo doesn't support locking natively, so we were planning on creating another collection and using the approach that MongoBee uses, to implement advisory locking.

We also planned on adding a few new options to mongo

x-advisory-locking which would enable this functionality
and
x-advisory-lock-collection which would specify the collection.

@dhui
Copy link
Member

dhui commented Sep 1, 2020

Hi @SJrX , Thanks for vetting the idea before opening a PR! A PR to implement locking would be greatly appreciated.

x-advisory-locking which would enable this functionality

Why not make locking the default? Main drawback I see is potential trickiness around metadata maintenance e.g. the collection used for tracking the lock

x-advisory-lock-collection which would specify the collection

I wouldn't make this configurable since it can cause issues. e.g. running distributed migrations and changing the lock collection will allow multiple migrations to be run

Looking forward to a PR!

@SJrX
Copy link
Contributor Author

SJrX commented Sep 1, 2020

x-advisory-locking which would enable this functionality

Why not make locking the default? Main drawback I see is potential trickiness around metadata maintenance e.g. the collection used for tracking the lock

The main reason is I thought it would increase the chance of a PR being accepted/liked. It also in the short term seems more backward compatible and safer.

x-advisory-lock-collection which would specify the collection

I wouldn't make this configurable since it can cause issues. e.g. running distributed migrations and changing the lock collection will allow multiple migrations to be run

I did this because it seemed synergistic with how other drivers let you configure the table / collection that is used for storing history. Your same concern applies there if migrations are not idempotent.

I'm fine either way, there is also the small chance that the collection name is already used.

I have no strong feelings either way honestly. Do you have a preference to a default or hard coded name.

@dhui
Copy link
Member

dhui commented Sep 2, 2020

Do you have a preference to a default or hard coded name.

I prefer to support locking by default. I'm not worried about backwards compatibility since this is an additive change. There is an extremely small chance of collection/index name conflict for the locking index/collection.

I did this because it seemed synergistic with how other drivers let you configure the table / collection that is used for storing history.

Good point. Let's make the name configurable to mitigate the small chance of the conflict mentioned above. We'll need ample documentation warning users of potential issues around changing the collection/index name.

@SJrX
Copy link
Contributor Author

SJrX commented Sep 26, 2020

@dhui question, when would the next release of go-migrate be. We are looking at ways to now consume this.

@dhui
Copy link
Member

dhui commented Sep 27, 2020

Just released v4.13.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants