Commit fc715de9 authored by Stan Hu's avatar Stan Hu

Fix ActiveRecord::IrreversibleOrderError during restore from backup

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.

The following sequence was seen in PostgreSQL logs:

1. Restore: CREATE TABLE public.application_settings ...
2. Restore: CREATE SEQUENCE public.application_settings_id_seq
3. Puma/Sidekiq: INSERT INTO "application_settings" ...
4. Restore: COPY public.application_settings (id, ...
5. Restore: ADD CONSTRAINT application_settings_pkey PRIMARY KEY (id);

In step 3, since GitLab inserted a new row with ID 1, but shortly after
that the restore process would also copy another row with ID 1. As a
result, the ADD CONSTRAINT in step 5 would fail with the message:

ERROR:   could not create unique index "application_settings_pkey"
Key (id)=(1) is duplicated.

The right way to fix this would be to restore the database in a single
transaction via the `--single-transaction` flag in `psql`, but this
usually fails due to permission errors when extensions and schemas are
dropped and recreated. `psql` generally works best with superuser
access, but restore usually runs with limited access permissions.

GitLab normally inserts an `application_settings` row if no row is
found, but to prevent duplicate rows from being inserted we ensure that
a primary key has been set beforehand. This should help prevent primary
key collisions and duplicate rows from being added.

We still need to tell administrators to stop all processes that connect
to the database (puma, sidekiq, and gitlab-exporter), but this check
should ensure that a primary key constraint is added during a restore.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/36405
parent 4e12f87c
---
title: Fix ActiveRecord::IrreversibleOrderError during restore from backup
merge_request: 40789
author:
type: fixed
......@@ -56,6 +56,8 @@ module Gitlab
elsif current_settings.present?
current_settings
else
check_application_settings_schema!
::ApplicationSetting.create_from_defaults
end
end
......@@ -68,6 +70,22 @@ module Gitlab
@in_memory_application_settings ||= ::ApplicationSetting.build_from_defaults
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 check_application_settings_schema!
return if ActiveRecord::Base.connection.primary_key(ApplicationSetting.table_name).present?
raise "The `application_settings` table is missing a primary key constraint in the database schema"
end
def connect_to_db?
# When the DBMS is not available, an exception (e.g. PG::ConnectionBad) is raised
active_db_connection = ActiveRecord::Base.connection.active? rescue false
......
......@@ -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 }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment