Track schema versions using filenames

Move tracking of schema_migration.versions out of the structure.sql file
and into empty files within the db/schema_migrations directory, which
should prevent conflicts when multiple migrations are added at one time

config/initializers/active_record_schema_versions.rb

 # frozen_string_literal: true
 
-# Patch to use COPY in db/structure.sql when populating schema_migrations table
+# Patch to write version information as empty files under the db/schema_migrations directory
 # This is intended to reduce potential for merge conflicts in db/structure.sql
-ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(Gitlab::Database::PostgresqlAdapter::SchemaVersionsCopyMixin)
+ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(Gitlab::Database::PostgresqlAdapter::DumpSchemaVersionsMixin)
+# Patch to load version information from empty files under the db/schema_migrations directory
+ActiveRecord::Tasks::PostgreSQLDatabaseTasks.prepend(Gitlab::Database::PostgresqlDatabaseTasks::LoadSchemaVersionsMixin)

lib/gitlab/database/postgresql_adapter/schema_versions_copy_mixin.rb → lib/gitlab/database/postgresql_adapter/dump_schema_versions_mixin.rb

@@ -3,24 +3,20 @@
 module Gitlab
   module Database
     module PostgresqlAdapter
-      module SchemaVersionsCopyMixin
+      module DumpSchemaVersionsMixin
         extend ActiveSupport::Concern
 
         def dump_schema_information # :nodoc:
           versions = schema_migration.all_versions
-          copy_versions_sql(versions) if versions.any?
+          touch_version_files(versions) if versions.any?
+
+          nil
         end
 
         private
 
-        def copy_versions_sql(versions)
-          sm_table = quote_table_name(schema_migration.table_name)
-
-          sql = +"COPY #{sm_table} (version) FROM STDIN;\n"
-          sql << versions.map { |v| Integer(v) }.sort.join("\n")
-          sql << "\n\\.\n"
-
-          sql
+        def touch_version_files(versions)
+          Gitlab::Database::SchemaVersionFiles.touch_all(versions)
         end
       end
     end

lib/gitlab/database/postgresql_database_tasks/load_schema_versions_mixin.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Database
+    module PostgresqlDatabaseTasks
+      module LoadSchemaVersionsMixin
+        extend ActiveSupport::Concern
+
+        def structure_load(*args)
+          super(*args)
+          load_version_files
+        end
+
+        private
+
+        def load_version_files
+          Gitlab::Database::SchemaVersionFiles.load_all
+        end
+      end
+    end
+  end
+end

lib/gitlab/database/schema_cleaner.rb

@@ -23,6 +23,10 @@ module Gitlab
         structure.gsub!(/\n{3,}/, "\n\n")
 
         io << structure
+        io << <<~MSG
+          -- schema_migrations.version information is no longer stored in this file,
+          -- but instead tracked in the db/schema_migrations directory
+        MSG
 
         nil
       end

lib/gitlab/database/schema_version_files.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Database
+    class SchemaVersionFiles
+      SCHEMA_DIRECTORY = "db/schema_migrations"
+
+      def self.touch_all(versions)
+        FileUtils.rm_rf(schema_dirpath)
+        FileUtils.mkdir_p(schema_dirpath)
+
+        versions.each do |version|
+          FileUtils.touch(schema_dirpath.join(version))
+        end
+      end
+
+      def self.load_all
+        version_filenames = find_version_filenames
+        return if version_filenames.empty?
+
+        values = version_filenames.map { |vf| "('#{connection.quote_string(vf)}')" }
+        connection.execute(<<~SQL)
+          INSERT INTO schema_migrations (version)
+          VALUES #{values.join(",")}
+          ON CONFLICT DO NOTHING
+        SQL
+      end
+
+      def self.schema_dirpath
+        @schema_dirpath ||= Rails.root.join(SCHEMA_DIRECTORY)
+      end
+
+      def self.find_version_filenames
+        Dir.glob("20[0-9][0-9]*", base: schema_dirpath)
+      end
+
+      def self.connection
+        ActiveRecord::Base.connection
+      end
+    end
+  end
+end

spec/fixtures/gitlab/database/structure_example.sql

@@ -77,19 +77,3 @@ ALTER TABLE ONLY public.abuse_reports
 
 CREATE INDEX index_abuse_reports_on_user_id ON public.abuse_reports USING btree (user_id);
 
-
-
-INSERT INTO "schema_migrations" (version) VALUES
-('20200305121159'),
-('20200306095654'),
-('20200306160521'),
-('20200306170211'),
-('20200306170321'),
-('20200306170531'),
-('20200309140540'),
-('20200309195209'),
-('20200309195710'),
-('20200310132654'),
-('20200310135823');
-
-

spec/fixtures/gitlab/database/structure_example_cleaned.sql

@@ -27,16 +27,5 @@ ALTER TABLE ONLY public.abuse_reports
 
 CREATE INDEX index_abuse_reports_on_user_id ON public.abuse_reports USING btree (user_id);
 
-INSERT INTO "schema_migrations" (version) VALUES
-('20200305121159'),
-('20200306095654'),
-('20200306160521'),
-('20200306170211'),
-('20200306170321'),
-('20200306170531'),
-('20200309140540'),
-('20200309195209'),
-('20200309195710'),
-('20200310132654'),
-('20200310135823');
-
+-- schema_migrations.version information is no longer stored in this file,
+-- but instead tracked in the db/schema_migrations directory

spec/lib/gitlab/database/postgresql_adapter/schema_versions_copy_mixin_spec.rb → spec/lib/gitlab/database/postgresql_adapter/dump_schema_versions_mixin_spec.rb

@@ -2,9 +2,8 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Database::PostgresqlAdapter::SchemaVersionsCopyMixin do
+describe Gitlab::Database::PostgresqlAdapter::DumpSchemaVersionsMixin do
   let(:schema_migration) { double('schem_migration', table_name: table_name, all_versions: versions) }
-  let(:versions) { %w(5 2 1000 200 4 93 2) }
   let(:table_name) { "schema_migrations" }
 
   let(:instance) do
@@ -13,30 +12,25 @@ RSpec.describe Gitlab::Database::PostgresqlAdapter::SchemaVersionsCopyMixin do
 
   before do
     allow(instance).to receive(:schema_migration).and_return(schema_migration)
-    allow(instance).to receive(:quote_table_name).with(table_name).and_return("\"#{table_name}\"")
   end
 
-  subject { instance.dump_schema_information }
+  context 'when version files exist' do
+    let(:versions) { %w(5 2 1000 200 4 93 2) }
 
-  it 'uses COPY FROM STDIN' do
-    expect(subject.split("\n").first).to match(/COPY "schema_migrations" \(version\) FROM STDIN;/)
-  end
-
-  it 'contains a sorted list of versions by their numeric value' do
-    version_lines = subject.split("\n")[1..-2].map(&:to_i)
+    it 'touches version files' do
+      expect(Gitlab::Database::SchemaVersionFiles).to receive(:touch_all).with(versions)
 
-    expect(version_lines).to eq(versions.map(&:to_i).sort)
+      instance.dump_schema_information
+    end
   end
 
-  it 'contains a end-of-data marker' do
-    expect(subject).to end_with("\\.\n")
-  end
+  context 'when version files do not exist' do
+    let(:versions) { [] }
 
-  context 'with non-Integer versions' do
-    let(:versions) { %w(5 2 4 abc) }
+    it 'does not touch version files' do
+      expect(Gitlab::Database::SchemaVersionFiles).not_to receive(:touch_all)
 
-    it 'raises an error' do
-      expect { subject }.to raise_error(/invalid value for Integer/)
+      instance.dump_schema_information
     end
   end
 end

spec/lib/gitlab/database/schema_version_files_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Database::SchemaVersionFiles do
+  describe '.touch_all' do
+    let(:versions) { %w[123 456 890] }
+
+    it 'touches a file for each version given' do
+      Dir.mktmpdir do |tmpdir|
+        schema_dirpath = Pathname.new(tmpdir).join("test")
+        FileUtils.mkdir_p(schema_dirpath)
+
+        old_version_filepath = schema_dirpath.join("001")
+        FileUtils.touch(old_version_filepath)
+
+        expect(File.exist?(old_version_filepath)).to be(true)
+
+        allow(described_class).to receive(:schema_dirpath).and_return(schema_dirpath)
+
+        described_class.touch_all(versions)
+
+        expect(File.exist?(old_version_filepath)).to be(false)
+        versions.each do |version|
+          version_filepath = schema_dirpath.join(version)
+          expect(File.exist?(version_filepath)).to be(true)
+        end
+      end
+    end
+  end
+
+  describe '.load_all' do
+    let(:connection) { double('connection') }
+
+    before do
+      allow(described_class).to receive(:connection).and_return(connection)
+      allow(described_class).to receive(:find_version_filenames).and_return(filenames)
+    end
+
+    context 'when there are no version files' do
+      let(:filenames) { [] }
+
+      it 'does nothing' do
+        expect(connection).not_to receive(:quote_string)
+        expect(connection).not_to receive(:execute)
+
+        described_class.load_all
+      end
+    end
+
+    context 'when there are version files' do
+      let(:filenames) { %w[123 456 789] }
+
+      it 'inserts the missing versions into schema_migrations' do
+        filenames.each do |filename|
+          expect(connection).to receive(:quote_string).with(filename).and_return(filename)
+        end
+
+        expect(connection).to receive(:execute).with(<<~SQL)
+          INSERT INTO schema_migrations (version)
+          VALUES ('123'),('456'),('789')
+          ON CONFLICT DO NOTHING
+        SQL
+
+        described_class.load_all
+      end
+    end
+  end
+end