Merge branch 'sh-fix-backup-restore-race' into 'master'

Fix ActiveRecord::IrreversibleOrderError during restore from backup

See merge request gitlab-org/gitlab!40789

app/models/application_setting.rb

@@ -427,6 +427,8 @@ class ApplicationSetting < ApplicationRecord
   end
 
   def self.create_from_defaults
+    check_schema!
+
     transaction(requires_new: true) do
       super
     end
@@ -435,6 +437,22 @@ class ApplicationSetting < ApplicationRecord
     current_without_cache
   end
 
+  # Due to the frequency with which settings are accessed, it is
+  # likely that during a backup restore a running GitLab process
+  # will insert a new `application_settings` row before the
+  # constraints have been added to the table. This would add an
+  # extra row with ID 1 and prevent the primary key constraint from
+  # being added, which made ActiveRecord throw a
+  # IrreversibleOrderError anytime the settings were accessed
+  # (https://gitlab.com/gitlab-org/gitlab/-/issues/36405).  To
+  # prevent this from happening, we do a sanity check that the
+  # primary key constraint is present before inserting a new entry.
+  def self.check_schema!
+    return if ActiveRecord::Base.connection.primary_key(self.table_name).present?
+
+    raise "The `#{self.table_name}` table is missing a primary key constraint in the database schema"
+  end
+
   # By default, the backend is Rails.cache, which uses
   # ActiveSupport::Cache::RedisStore. Since loading ApplicationSetting
   # can cause a significant amount of load on Redis, let's cache it in

changelogs/unreleased/sh-fix-backup-restore-race.yml

+---
+title: Fix ActiveRecord::IrreversibleOrderError during restore from backup
+merge_request: 40789
+author:
+type: fixed

spec/lib/gitlab/current_settings_spec.rb

@@ -115,6 +115,16 @@ RSpec.describe Gitlab::CurrentSettings do
           expect(settings).to have_attributes(settings_from_defaults)
         end
 
+        context 'when ApplicationSettings does not have a primary key' do
+          before do
+            allow(ActiveRecord::Base.connection).to receive(:primary_key).with('application_settings').and_return(nil)
+          end
+
+          it 'raises an exception if ApplicationSettings does not have a primary key' do
+            expect { described_class.current_application_settings }.to raise_error(/table is missing a primary key constraint/)
+          end
+        end
+
         context 'with pending migrations' do
           let(:current_settings) { described_class.current_application_settings }
 

spec/models/application_setting_spec.rb

@@ -650,6 +650,16 @@ RSpec.describe ApplicationSetting do
     end
   end
 
+  context 'when ApplicationSettings does not have a primary key' do
+    before do
+      allow(ActiveRecord::Base.connection).to receive(:primary_key).with(described_class.table_name).and_return(nil)
+    end
+
+    it 'raises an exception' do
+      expect { described_class.create_from_defaults }.to raise_error(/table is missing a primary key constraint/)
+    end
+  end
+
   describe '#disabled_oauth_sign_in_sources=' do
     before do
       allow(Devise).to receive(:omniauth_providers).and_return([:github])