reenable for migrations on multiple servers

[disem]

Since capistrano 3.6.0 multiple database servers don't work because of:

Capistrano tasks may only be invoked once. Since task `deploy:migrating' was previously invoked, invoke("deploy:migrating") at /.../.rvm/gems/ruby-2.3.1@projectname/gems/capistrano-rails-1.2.0/lib/capistrano/tasks/migrations.rake:14 will be skipped.
If you really meant to run this task again, first call Rake::Task["deploy:migrating"].reenable
THIS BEHAVIOR MAY CHANGE IN A FUTURE VERSION OF CAPISTRANO. Please join the conversation here if this affects you.
https://github.com/capistrano/capistrano/issues/1686

[will-in-wi]

I believe that this was fixed in #168 which came out in 1.1.7.

[disem]

As you can see from the log output I'm getting this error on capistrano-rails-1.2.0

[will-in-wi]

The error is correct. It looks like you are trying to reinvoke the task on multiple servers. You should use the feature in #168 which allows you to use set :migration_servers, ['myserver1', 'myserver2'].

[disem]

That's correct, I need migrations to be invoked on multiple servers and I have following configuration to do that:

server '...', user: '...', roles: %w(app db web)
server '...', user: '...', roles: %w(app db web)
set :migration_servers, -> { release_roles(fetch(:migration_role)) }

So it tries to run migration on both servers but the second one was skipped due to this error.

[mattbrictson]

Since capistrano 3.6.0 multiple database servers don't work

Just to clarify: 3.6.0 did not change any behavior. Prior to 3.6.0 multiple invocations would simply fail silently. Starting with 3.6.0 they fail and a warning is printed.

That said, this does seem to be a legitimate bug in capistrano-rails.

The current implementation looks like this (pseudo code):

on each server:
  if migrations should run:
    on each server:
      invoke migrations

The code for multiple servers uses two nested on blocks: the first to test if migrations should be run, and then another to perform the actual migration. You can see that invoke will be called n² times – obviously not a good idea. Furthermore, there is the invoke-only-once limitation which means that out of all these attempts, only the first actually does anything.

I believe some extensive changes are needed to make multi-migrations work. Does anyone want to give it a try?

[littldr]

Is there a reason while the migration task is splitted into two separate tasks?
This Code (not tested) should do the same, but avoiding multiple invokation of tasks. Or I am missing some point here?

namespace :deploy do
  desc 'Runs rake db:migrate if migrations are set'
  task :migrate => [:set_rails_env] do
    on fetch(:migration_servers) do
      conditionally_migrate = fetch(:conditionally_migrate)
      info '[deploy:migrate] Checking changes in db' if conditionally_migrate
      if conditionally_migrate && test("diff -qr #{release_path}/db #{current_path}/db")
        info '[deploy:migrate] Skip `deploy:migrate` (nothing changed in db)'
      else
        info '[deploy:migrate] Run `rake db:migrate`'
        within release_path do
          with rails_env: fetch(:rails_env) do
            execute :rake, 'db:migrate'
          end
        end
      end
    end
  end

  after 'deploy:updated', 'deploy:migrate'
end

[mattbrictson]

Is there a reason while the migration task is splitted into two separate tasks?

I can think of a couple reasons:

1. Developers can write before/after hooks for the migrating task, which is the task that actually performs the migration. If we remove this, then hooks can only be written against migrate, which may or may not actually perform a migration due to the conditional behavior.
2. You can "force" a migration (i.e. bypass the conditional logic) by calling the migrating task directly: cap production deploy:migrating.

But first, let's back up: are we certain that the migrations are not being run on multiple servers?

When I re-read the code, I realized that even though deploy:migrating is only being invoked once, that should still be sufficient to run the migrations on all applicable servers. In other words, the Capistrano tasks may only be invoked once warning is not actually indicative of an error.

@disem Can you please verify again whether migrations are actually being run (e.g. by looking at the full Capistrano log output), despite the warning that's printed?

[disem]

@mattbrictson will check tomorrow.

[littldr]

@mattbrictson you are right! I've missed these two points.

I've stumbled over the same warning message last week and thought the migration is not executed on all servers. But as you've assumed after looking precise in the logs, the migrations is executed on all servers:

So this is not a bug, but an unsightly warning. We could avoid this warning with:

invoke :'deploy:migrating' unless Rake::Task[:'deploy:migrating'].already_invoked

What you think about this?

[mattbrictson]

@Landreas Yes, unless Rake::Task[:'deploy:migrating'].already_invoked would be a good simple fix. Please open a PR!

[littldr]

@mattbrictson i've updated #191. 🙂