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

Option to skip locking #215

Closed
mcandre opened this issue May 1, 2019 · 10 comments · Fixed by #439
Closed

Option to skip locking #215

mcandre opened this issue May 1, 2019 · 10 comments · Fixed by #439

Comments

@mcandre
Copy link

mcandre commented May 1, 2019

Is your feature request related to a problem? Please describe.

Some database configurations, such as distributed formulations, disable locks by default.

Describe the solution you'd like

Would be helpful for the migrate library and CLI to accept a parameter to not bother using locks, so that golang-migrate can be used to manage schemas for distributed databases.

Describe alternatives you've considered

  • Using a single instance deployment (not an option for my application)
  • Using other migration frameworks (Overall, golang-migrate is rather nice and I would prefer to stick with it)
@dhui
Copy link
Member

dhui commented May 16, 2019

Some database configurations, such as distributed formulations, disable locks by default.

What is a distributed formulation?

Would be helpful for the migrate library and CLI to accept a parameter to not bother using locks, so that golang-migrate can be used to manage schemas for distributed databases.

Locks are implemented by the driver to make running migrate via multiple app instances safe. Note, not every migrate DB driver supports locks.

Can you provide specific examples when migration locks cause problems?

@omissis
Copy link

omissis commented May 23, 2019

@dhui I think he is referring to MySQL/Percona Cluster which do not support LOCK TABLES in multi-master setup, see: https://www.percona.com/doc/percona-xtradb-cluster/LATEST/limitation.html.

Edit: for the record, we ran into this issue as well. :)

@dhui
Copy link
Member

dhui commented May 24, 2019

@omissis Thanks for the example and docs! The migrate MySQL db driver is actually using GET_LOCK() and RELEASE_LOCK(), not LOCK TABLES but that's besides the point since they're not supported either...

For now, we can add an option to the MySQL db driver to not lock when running migrations.
e.g. x-no-lock

I don't think it makes sense to make a more intrusive change (e.g. add an additional config option) that may confuse users that don't use migrate with MySQL.

@omissis
Copy link

omissis commented May 24, 2019

@dhui I started to experiment about it in my fork, but I am not sure I understand your idea: if we only add an option to the driver and not to the command line tool, those users (like me 😬) who run the binary to migrate their tables would still be blocked on cluster instances, wouldn’t they?

@dhui
Copy link
Member

dhui commented May 24, 2019

@omissis The options are specified in the DSN e.g. in the connection string

@omissis
Copy link

omissis commented May 24, 2019

I see. That would not be part of the built-in mysql.Config object though, and probably would require parsing the query param before passing the connection string to mysql.ParseDSN(), right? I'll try to open a PR later so we can talk looking at some code.

edit: @dhui ok I just found the code parsing custom query params :)

@Fapiko
Copy link

Fapiko commented Nov 14, 2019

Did a change for this ever get made pulled into the project? I'm trying to get this working with MariaDB right now - I might explore implementing the proposed solution above if it has not yet.

@omissis
Copy link

omissis commented Nov 14, 2019

@Fapiko I took a look around some time ago and found a way to implement it as @dhui suggested, but never followed through. If I can find some time maybe I can still contribute a patch, perhaps we can work together on it.

@Fapiko
Copy link

Fapiko commented Nov 19, 2019

@omissis Sure. I haven't dug into this codebase yet, can you link me to the spots in the code you were looking at to give me a head start?

@fungilation
Copy link

I'm hitting this issue as well, with PXC (Percona Cluster) failing to run migrations due to the GET_LOCKs.

Would love to have option to disable the locks, with the caveat that migrations have to be run from single instance app or cli.

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.

5 participants