From e2284cf7391aa94ed2f22662305bc14e9e51262c Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Tue, 14 Apr 2020 11:21:31 +1000 Subject: [PATCH] Revert "We have had errors reported due to migrations breaking and are reverting" This reverts commit 8b46f14744d4bab9339d075825914d86da6bd5eb. It corrects the reason for the revert: We rely on SafeMigrate existing cause we call it from migrations, Zeitwerk will autoload it. Instead of previous pattern we explicitly bypass all the hacks in production mode. We need to disable SafeMigrate cause it is not thread safe. A thread safe implementation is possible but not worth the effort, we catch the issues in dev and test. --- lib/freedom_patches/safe_migrations.rb | 12 ++- lib/migration/safe_migrate.rb | 5 ++ lib/tasks/db.rake | 100 +++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 3 deletions(-) diff --git a/lib/freedom_patches/safe_migrations.rb b/lib/freedom_patches/safe_migrations.rb index 718ffffb89f..cdf989fdd5b 100644 --- a/lib/freedom_patches/safe_migrations.rb +++ b/lib/freedom_patches/safe_migrations.rb @@ -1,5 +1,11 @@ # frozen_string_literal: true -require_dependency 'migration/safe_migrate' - -Migration::SafeMigrate.patch_active_record! +# We do not run this in production cause it is intrusive and has +# potential to break stuff, it also breaks under concurrent use +# which rake:multisite_migrate uses +# +# The protection is only needed in Dev and Test +if ENV['RAILS_ENV'] != "production" + require_dependency 'migration/safe_migrate' + Migration::SafeMigrate.patch_active_record! +end diff --git a/lib/migration/safe_migrate.rb b/lib/migration/safe_migrate.rb index d9adb518f1b..cf6c43be912 100644 --- a/lib/migration/safe_migrate.rb +++ b/lib/migration/safe_migrate.rb @@ -73,6 +73,7 @@ class Migration::SafeMigrate def self.enable! return if PG::Connection.method_defined?(:exec_migrator_unpatched) + return if ENV['RAILS_ENV'] == "production" PG::Connection.class_eval do alias_method :exec_migrator_unpatched, :exec @@ -92,6 +93,8 @@ class Migration::SafeMigrate def self.disable! return if !PG::Connection.method_defined?(:exec_migrator_unpatched) + return if ENV['RAILS_ENV'] == "production" + PG::Connection.class_eval do alias_method :exec, :exec_migrator_unpatched alias_method :async_exec, :async_exec_migrator_unpatched @@ -102,6 +105,8 @@ class Migration::SafeMigrate end def self.patch_active_record! + return if ENV['RAILS_ENV'] == "production" + ActiveSupport.on_load(:active_record) do ActiveRecord::Migration.prepend(SafeMigration) end diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index 14b2eb4e202..81087473a0e 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -66,6 +66,106 @@ task 'db:rollback' => ['environment', 'set_locale'] do |_, args| Rake::Task['db:_dump'].invoke end +# our optimized version of multisite migrate, we have many sites and we have seeds +# this ensures we can run migrations concurrently to save huge amounts of time +Rake::Task['multisite:migrate'].clear + +class StdOutDemux + def initialize(stdout) + @stdout = stdout + @data = {} + end + + def write(data) + (@data[Thread.current] ||= +"") << data + end + + def close + finish_chunk + end + + def finish_chunk + data = @data[Thread.current] + if data + @stdout.write(data) + @data.delete Thread.current + end + end +end + +task 'multisite:migrate' => ['db:load_config', 'environment', 'set_locale'] do |_, args| + if ENV["RAILS_ENV"] != "production" + raise "Multisite migrate is only supported in production" + end + + concurrency = (ENV['MIGRATE_CONCURRENCY'].presence || "20").to_i + + puts "Multisite migrator is running using #{concurrency} threads" + puts + + queue = Queue.new + exceptions = Queue.new + + old_stdout = $stdout + $stdout = StdOutDemux.new($stdout) + + RailsMultisite::ConnectionManagement.each_connection do |db| + queue << db + end + + concurrency.times { queue << :done } + + SeedFu.quiet = true + + (1..concurrency).map do + Thread.new { + while true + db = queue.pop + break if db == :done + + RailsMultisite::ConnectionManagement.with_connection(db) do + begin + puts "Migrating #{db}" + ActiveRecord::Tasks::DatabaseTasks.migrate + SeedFu.seed(DiscoursePluginRegistry.seed_paths) + if !Discourse.skip_post_deployment_migrations? && ENV['SKIP_OPTIMIZE_ICONS'] != '1' + SiteIconManager.ensure_optimized! + end + rescue => e + exceptions << [db, e] + ensure + begin + $stdout.finish_chunk + rescue => ex + STDERR.puts ex.inspect + STDERR.puts ex.backtrace + end + end + end + end + } + end.each(&:join) + + $stdout = old_stdout + + if exceptions.length > 0 + STDERR.puts + STDERR.puts "-" * 80 + STDERR.puts "#{exceptions.length} migrations failed!" + while !exceptions.empty? + db, e = exceptions.pop + STDERR.puts + STDERR.puts "Failed to migrate #{db}" + STDERR.puts e.inspect + STDERR.puts e.backtrace + STDERR.puts + end + exit 1 + end + + Rake::Task['db:_dump'].invoke +end + # we need to run seed_fu every time we run rake db:migrate task 'db:migrate' => ['load_config', 'environment', 'set_locale'] do |_, args|