Advisory lock for migration transactions about safe-ecto-migrations HOT 10 CLOSED

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

from safe-ecto-migrations.

@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 :).

from safe-ecto-migrations.

@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.

from safe-ecto-migrations.

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

from safe-ecto-migrations.

@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.

from safe-ecto-migrations.

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!

from safe-ecto-migrations.

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")

from safe-ecto-migrations.

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.

from safe-ecto-migrations.

@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 elixir-ecto/ecto_sql#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

from safe-ecto-migrations.

Updated with afb278d

from safe-ecto-migrations.