influitive / apartment

Database multi-tenancy for Rack (and Rails) applications
2.66k stars 464 forks source link

using parallel_migration_threads fails with ActiveRecord::ConcurrentMigrationError in Rails 5.x #506

Open zzip opened 6 years ago

zzip commented 6 years ago

Steps to reproduce

Set parallel_migration_threads to 4 threads. Have a database with some migrations to be applied and multiple tenants (tested with 100 tenants and 5 migrations per tenant to be migrated)

Expected behavior

All migrations are run in parallel

Actual behavior

migrations fail quickly with following error :

Cannot run migrations because another migration process is currently running.
Tasks: TOP => apartment:migrate
(See full trace by running task with --trace)

Most likely this is caused by the following MR in Rails since v5.0.0 https://github.com/rails/rails/pull/22122

possible fix could be to generate an advisory_lock_id based on more then just a fixed salt and connection.current_database, but also include the current schema/tenant here https://github.com/rails/rails/blob/5-1-stable/activerecord/lib/active_record/migration.rb#L1324

System configuration

mikecmpbll commented 6 years ago

ach, nice catch. if concurrently migrating different schemas is kosher in pg, then it shouldn't raise an exception in rails. i think your solution is the correct solution and it seems to me as though that should be implemented upstream into rails. i've put a comment on the PR that you referenced.

pts-owentran commented 6 years ago

The Rails team closed the issue because it was stale. I added a comment that the issue still happens with rails 5.1.3.

marksiemers commented 3 years ago

@pts-owentran @mikecmpbll @zzip - If that PR looks good to you, could you give it a thumbs up? Maybe the rails team will take it.