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

Multi schema postgres migrations lock conflict #118

Closed
vporoshok opened this issue Oct 22, 2018 · 3 comments
Closed

Multi schema postgres migrations lock conflict #118

vporoshok opened this issue Oct 22, 2018 · 3 comments

Comments

@vporoshok
Copy link
Contributor

Describe the Bug
We use schema separated multi tenant system (one database and schema per client). On client registration we create schema and run migration on it (using search_path). On concurrent registration of two or more clients migration process failed with error can't acquire lock.

Steps to Reproduce

  1. Run parallel migrations on the same database and different schemas.
  2. See error

Expected Behavior
Make LockID from database and schema names both.

Migrate Version
e.g. v4.0.2

Loaded Source Drivers
file

Loaded Database Drivers
postgres

Go Version
go1.11 linux/amd64

Additional context
For now we just copy postgres subpackage and add SchemaName to config (SELECT CURRENT_SCHEMA()) and concat DatabaseName with SchameName with separator.

@dhui
Copy link
Member

dhui commented Oct 22, 2018

The pg advisory log is designed to protect against multiple instances of migrate running against the same postgres RDBMS. The schema name isn't used to generate the lock id, but that could be changed.
However the change isn't as simple as using both the schema name and db name to generate the lock id since migrate tracks the current applied migration in a table (schema_migrations) in the default schema (unless you're using the schema search path).

How are you managing schema creation?

@vporoshok
Copy link
Contributor Author

We use search_path parameter of migration connection. So all changes is applied to specific schema and do not change public schema. So schema_migration table is created in specific schema too.

postgres=# SET search_path=local;
SET
postgres=# SELECT current_schema();
 current_schema 
----------------
 local
(1 row)

postgres=# CREATE TABLE test ();
CREATE TABLE
elma365=# \d
        List of relations
 Schema | Name | Type  |  Owner   
--------+------+-------+----------
 local  | test | table | postgres
(1 row)

@dhui
Copy link
Member

dhui commented Oct 23, 2018

In order to accommodate running multiple concurrent migrations with different schemas, we'll need an additional DB query during the locking phase to get the current schema.
The advisory lock needs to protect against concurrent migrations to the same RDBMS and schema.

I don't think the approach where the schema name is fetched from the config works since we still need to get the current schema to check it's the same as the configured schema name.

Implementation note:
It doesn't look like many DB drivers use GenerateAdvisoryLockId() (only postgres, mysql, and cockroachdb), so we can modify that method to also accept a schema name and use sane defaults for the other DB drivers. Bonus points for supporting multiple schemas on other DB drivers as well!

Feel free to open a PR with such a change (and don't forget to add tests).

@dhui dhui closed this as completed in 16d63e3 Nov 6, 2018
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

No branches or pull requests

2 participants