Revert "We have had errors reported due to migrations breaking and are reverting"

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.

diff --git a/lib/freedom_patches/safe_migrations.rb b/lib/freedom_patches/safe_migrations.rb
index 718ffff..cdf989f 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 d9adb51..cf6c43b 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 14b2eb4..8108747 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|
 

GitHub sha: e2284cf7