Closed lcmen closed 2 years ago
It's possible but it's code that you'd have to roll yourself right now-- I don't see a configurable option in Ecto currently to use pg_advisory_lock
or pg_advisory_xact_lock
during migrations instead of share update exclusive locks. I'm assuming you're talking about Postgres.
https://www.postgresql.org/docs/current/functions-admin.html
If you wanted to use application-side advisory locks, you could set up a a module to wrap this logic up yourself. I'm curious though, why would you prefer advisory locks?
For example (this is untested -- don't use this in production until you do way more testing on this):
defmodule MyConcurrentIndexMigration do
defmacro __using__(_opts) do
quote do
use Ecto.Migration
@disable_migration_lock true
@disable_ddl_transaction true
def up do
lock_name = "\"ecto_#{inspect(repo())}\""
try do
repo().query!("SELECT pg_advisory_lock(#{lock_name})")
do_up()
after
repo().query!("SELECT pg_advisory_unlock(#{lock_name})")
end
end
def down do
lock_name = "\"ecto_#{inspect(repo())}\""
try do
repo().query!("SELECT pg_advisory_lock(#{lock_name})")
do_down()
after
repo().query!("SELECT pg_advisory_unlock(#{lock_name})")
end
end
end
end
end
## Then in your migration
defmodule MyApp.Migrations202020202020 do
use MyConcurrentIndexMigration
def do_up do
create index("foo"), concurrently: true
end
def do_down do
drop index("foo"), concurrently: true
end
end
This is taking inspiration from the MyXQL Ecto adapter for obtaining locks: https://github.com/elixir-ecto/ecto_sql/blob/v3.7.1/lib/ecto/adapters/myxql.ex#L227-L249
@dbernheisel thank you so much for such detailed response and code examples! I really appreciate it.
I'm assuming you're talking about Postgres.
Yes, I'm sorry I wasn't clear about that.
I'm curious though, why would you prefer advisory locks?
Please correct me if I'm wrong but as fair as I understand it, advisory lock for migration makes sure only one node can execute the migration (in multi node setup) but it allows to disable ddl transaction for database schema change so i.e. adding index concurrently doesn't lock the table.
oh I see, so you still want a lock on running migrations totally, but without the database transactions. I think i understand. Then yeah! I've updated my example above to be clearer.
This would be a great example to add to the reference material. Thanks for asking about it!
I wonder if EctoSQL would consider a PR for making this a configurable option for the pg adapter. @wojtekmach do you have any suggestions here or ideas? I'd be happy to contribute.
maybe something like this?
defmodule MyMigration do
@use_pg_advisory_lock true
def change do
# my changes
end
end
The existence of the @use_pg_advisory_lock
would change the strategy for how to obtain the lock, but would also require the advisory lock to be checked for other migrations not using the @use_pg_advisory_lock
option.
the result would need to be something like this I think?
-- This is for the use_pg_advisory_lock option:
SELECT pg_advisory_lock("ecto_my_repo")
-- my changes
INSERT INTO "schema_migrations" ("version","inserted_at") VALUES ($1,$2);
SELECT pg_advisory_unlock("ecto_my_repo")
------------------
-- This would be for the non-use_pg_advisory_lock option:
SELECT pg_advisory_lock("ecto_my_repo")
BEGIN;
-- we could remove this lock
LOCK TABLE "schema_migrations" IN SHARE UPDATE EXCLUSIVE MODE
BEGIN;
-- after_begin callback
-- my changes
-- before_commit callback
INSERT INTO "schema_migrations" ("version","inserted_at") VALUES ($1,$2);
COMMIT;
COMMIT;
SELECT pg_advisory_unlock("ecto_my_repo")
Advisory lock seems like a great option. We even have this in docs [1]
Disabling DDL transactions removes the guarantee that all of the changes in the migration will happen at once. Disabling the migration lock removes the guarantee only a single node will run a given migration if multiple nodes are attempting to migrate at the same time.
Please open up an issue on ecto_sql referencing this one and we can discuss it further. I feel like advisory lock could even be the default for PG, are there any downsides?
[1] https://hexdocs.pm/ecto_sql/Ecto.Migration.html#index/3-adding-dropping-indexes-concurrently
I feel like advisory lock could even be the default for PG, are there any downsides?
I initially asked this because that's what Rails uses by default (at least for Postgres). The only downside I see is when someone is using PGBouncer in transaction pooling mode - it doesn't support advisory locks.
@dbernheisel I've been reading Programming Ecto book and found migration_lock
option for repo's config:
config :my_app, MyApp.Repo, migration_lock: {true|false}
I believe if Ecto was to support different type of locks for inserting schema_migrations
record, that probably would be the best place to specify it, e.g. config :my_app, MyApp.Repo, migration: true
could use normal migration (current behaviour) for backward compatibility, while config :my_app, MyApp.Repo, migration: :advisory_lock
could use advisory lock way.
I'm not sure how it fits current implementation but I just recalled this thread while I was reading the book :).
@lcmen heads up, on the master branch for ecto_sql there is a new configuration option for Postgres and Ecto systems to use postgres advisory locks for managing competing nodes migrations'.
If you use config MyApp.Repo, migration_lock: :pg_advisory_lock
you should be able to run concurrent database ops such as creating concurrent indexes now without sacrificing safety.
# in config/config.exs
config MyApp.Repo, migration_lock: :pg_advisory_lock
# in the migration
defmodule MyApp.Repo.Migrations.LengthyIndex do
use Ecto.Migration
@disable_ddl_transaction true
def change do
create index(:big_table, [:foo], concurrently: true)
end
end
See https://github.com/elixir-ecto/ecto_sql/pull/428 for more details.
I'll update the guides as soon as EctoSQL is tagged with a version that includes this new config option.
migrate_test ❯ npm i -g concurrently
migrate_test ❯ CMD='mix ecto.migrate --log-migrator-sql --log-migrations-sql'
migrate_test ❯ concurrently "$CMD" "$CMD" "$CMD"
[2]
[2] 13:07:29.981 [debug] QUERY OK db=0.2ms
[2] SELECT pg_try_advisory_lock(10421502) []
[2]
[2] 13:07:30.093 [debug] QUERY OK db=0.4ms
[2] SELECT pg_advisory_unlock(10421502) []
[2]
[2] 13:07:30.241 [debug] QUERY OK db=0.7ms
[2] SELECT pg_try_advisory_lock(10421502) []
[2]
[2] 13:07:30.282 [debug] QUERY OK db=0.5ms idle=154.6ms
[2] begin []
[2] ↳ Ecto.Migrator.run_maybe_in_transaction/5, at: lib/ecto/migrator.ex:337
[2]
[2] 13:07:30.305 [info] == Running 20220720153104 MigrateTest.Repo.Migrations.BigTable.up/0 forward
[2]
[2] 13:07:30.309 [info] create table big_table
[2]
[2] 13:07:30.346 [debug] QUERY OK db=35.6ms
[2] CREATE TABLE "big_table" ("id" bigserial, "foo" text, PRIMARY KEY ("id")) []
[2]
[2] 13:07:30.350 [info] execute "INSERT INTO big_table(foo)\n SELECT md5(random()::text)\n FROM GENERATE_SERIES(1,1000000) i\n"
[0]
[0] 13:07:30.692 [debug] QUERY OK db=0.1ms
[0] SELECT pg_try_advisory_lock(10421502) []
[0]
[0] 13:07:30.711 [info] Migration lock occupied for MigrateTest.Repo. Retry 1/infinity at 5000ms intervals.
[1]
[1] 13:07:30.847 [debug] QUERY OK db=0.2ms
[1] SELECT pg_try_advisory_lock(10421502) []
[1]
[1] 13:07:30.861 [info] Migration lock occupied for MigrateTest.Repo. Retry 1/infinity at 5000ms intervals.
[0]
[0] 13:07:35.713 [debug] QUERY OK db=0.2ms
[0] SELECT pg_try_advisory_lock(10421502) []
[0]
[0] 13:07:35.713 [info] Migration lock occupied for MigrateTest.Repo. Retry 2/infinity at 5000ms intervals.
[1]
[1] 13:07:35.864 [debug] QUERY OK db=0.3ms
[1] SELECT pg_try_advisory_lock(10421502) []
[1]
[1] 13:07:35.864 [info] Migration lock occupied for MigrateTest.Repo. Retry 2/infinity at 5000ms intervals.
[2]
[2] 13:07:35.868 [debug] QUERY OK db=5516.7ms
[2] INSERT INTO big_table(foo)
[2] SELECT md5(random()::text)
[2] FROM GENERATE_SERIES(1,1000000) i
[2] []
[2]
[2] 13:07:35.872 [info] == Migrated 20220720153104 in 5.5s
[2]
[2] 13:07:35.934 [debug] QUERY OK db=9.3ms
[2] INSERT INTO "schema_migrations" ("version","inserted_at") VALUES ($1,$2) [20220720153104, ~N[2022-07-25 17:07:35]]
[2] ↳ anonymous fn/6 in Ecto.Migrator.async_migrate_maybe_in_transaction/7, at: lib/ecto/migrator.ex:320
[2]
[2] 13:07:36.026 [debug] QUERY OK db=91.8ms
[2] commit []
[2] ↳ Ecto.Migrator.run_maybe_in_transaction/5, at: lib/ecto/migrator.ex:337
[2]
[2] 13:07:36.029 [debug] QUERY OK db=0.9ms
[2] SELECT pg_advisory_unlock(10421502) []
[2]
[2] 13:07:36.032 [debug] QUERY OK db=1.0ms
[2] SELECT pg_try_advisory_lock(10421502) []
[2]
[2] 13:07:36.037 [info] == Running 20220720153111 MigrateTest.Repo.Migrations.LengthyIndex.change/0 forward
[2]
[2] 13:07:36.040 [info] create index big_table_foo_index
[2]
[2] 13:07:40.260 [debug] QUERY OK db=4218.7ms queue=1.2ms idle=11.0ms
[2] CREATE INDEX CONCURRENTLY "big_table_foo_index" ON "big_table" ("foo") []
[2]
[2] 13:07:40.260 [info] == Migrated 20220720153111 in 4.2s
[2]
[2] 13:07:40.270 [debug] QUERY OK db=6.6ms queue=2.5ms idle=0.3ms
[2] INSERT INTO "schema_migrations" ("version","inserted_at") VALUES ($1,$2) [20220720153111, ~N[2022-07-25 17:07:40]]
[2] ↳ anonymous fn/6 in Ecto.Migrator.async_migrate_maybe_in_transaction/7, at: lib/ecto/migrator.ex:320
[2]
[2] 13:07:40.271 [debug] QUERY OK db=0.3ms
[2] SELECT pg_advisory_unlock(10421502) []
[2] mix ecto.migrate --log-migrations-sql --log-migrator-sql exited with code 0
[0]
[0] 13:07:40.720 [debug] QUERY OK db=1.1ms
[0] SELECT pg_try_advisory_lock(10421502) []
[0]
[0] 13:07:40.779 [debug] QUERY OK db=0.2ms
[0] SELECT pg_advisory_unlock(10421502) []
[0]
[0] 13:07:40.779 [info] Migrations already up
[0] mix ecto.migrate --log-migrations-sql --log-migrator-sql exited with code 0
[1]
[1] 13:07:40.866 [debug] QUERY OK db=0.2ms
[1] SELECT pg_try_advisory_lock(10421502) []
[1]
[1] 13:07:40.899 [debug] QUERY OK db=0.2ms
[1] SELECT pg_advisory_unlock(10421502) []
[1]
[1] 13:07:40.900 [info] Migrations already up
[1] mix ecto.migrate --log-migrations-sql --log-migrator-sql exited with code 0
@dbernheisel thanks for the update! I've just pulled the latest master
of ecto-sql
to test it on my dummy repo. It seems to aligned with I can see in your logs.
Awesome work 🍻 ! Thank you so much for adding this option. I hugely believe this will help other folks with migrating large databases.
Updated with afb278d00e218d2f2bfb7f8a333845965f2fb3a2
Hello, Not sure if this is the best place to ask but I will shoot it anyway ;).
I'm Rails developer learning Elixir and I have a question about safe Ecto migrations. Is it possible to use advisory lock instead of share update exclusive on
schema_migrations
table (like Rails does it) when creating a migration record?I'm asking because (as fair as I understand) advisory lock would allow to disable ddl transaction for the migration itself (i.e. adding index concurrently) but it would still make sure that only one node can execute it (in multi node setup).