diff options
Diffstat (limited to 'spec/lib/gitlab/database/reindexing')
3 files changed, 409 insertions, 0 deletions
diff --git a/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb b/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb new file mode 100644 index 00000000000..2d6765aac2e --- /dev/null +++ b/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb @@ -0,0 +1,255 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Reindexing::ConcurrentReindex, '#perform' do + subject { described_class.new(index, logger: logger) } + + let(:table_name) { '_test_reindex_table' } + let(:column_name) { '_test_column' } + let(:index_name) { '_test_reindex_index' } + let(:index) { instance_double(Gitlab::Database::PostgresIndex, indexrelid: 42, name: index_name, schema: 'public', partitioned?: false, unique?: false, exclusion?: false, definition: 'CREATE INDEX _test_reindex_index ON public._test_reindex_table USING btree (_test_column)') } + let(:logger) { double('logger', debug: nil, info: nil, error: nil ) } + let(:connection) { ActiveRecord::Base.connection } + + before do + connection.execute(<<~SQL) + CREATE TABLE #{table_name} ( + id serial NOT NULL PRIMARY KEY, + #{column_name} integer NOT NULL); + + CREATE INDEX #{index.name} ON #{table_name} (#{column_name}); + SQL + end + + context 'when the index is unique' do + before do + allow(index).to receive(:unique?).and_return(true) + end + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /UNIQUE indexes are currently not supported/) + end + end + + context 'when the index is partitioned' do + before do + allow(index).to receive(:partitioned?).and_return(true) + end + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /partitioned indexes are currently not supported/) + end + end + + context 'when the index serves an exclusion constraint' do + before do + allow(index).to receive(:exclusion?).and_return(true) + end + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /indexes serving an exclusion constraint are currently not supported/) + end + end + + context 'when the index is a lingering temporary index from a previous reindexing run' do + context 'with the temporary index prefix' do + let(:index_name) { 'tmp_reindex_something' } + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /left-over temporary index/) + end + end + + context 'with the replaced index prefix' do + let(:index_name) { 'old_reindex_something' } + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /left-over temporary index/) + end + end + end + + context 'replacing the original index with a rebuilt copy' do + let(:replacement_name) { 'tmp_reindex_42' } + let(:replaced_name) { 'old_reindex_42' } + + let(:create_index) { "CREATE INDEX CONCURRENTLY #{replacement_name} ON public.#{table_name} USING btree (#{column_name})" } + let(:drop_index) do + <<~SQL + DROP INDEX CONCURRENTLY + IF EXISTS "public"."#{replacement_name}" + SQL + end + + let!(:original_index) { find_index_create_statement } + + it 'integration test: executing full index replacement without mocks' do + allow(connection).to receive(:execute).and_wrap_original do |method, sql| + method.call(sql.sub(/CONCURRENTLY/, '')) + end + + subject.perform + + check_index_exists + end + + context 'mocked specs' do + before do + allow(subject).to receive(:connection).and_return(connection) + allow(connection).to receive(:execute).and_call_original + end + + it 'replaces the existing index with an identical index' do + expect(connection).to receive(:execute).with('SET statement_timeout TO \'21600s\'').twice + + expect_to_execute_concurrently_in_order(create_index) + + expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| + expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield + end + + expect_index_rename(index.name, replaced_name) + expect_index_rename(replacement_name, index.name) + expect_index_rename(replaced_name, replacement_name) + + expect_to_execute_concurrently_in_order(drop_index) + + subject.perform + + check_index_exists + end + + context 'when a dangling index is left from a previous run' do + before do + connection.execute("CREATE INDEX #{replacement_name} ON #{table_name} (#{column_name})") + end + + it 'replaces the existing index with an identical index' do + expect(connection).to receive(:execute).with('SET statement_timeout TO \'21600s\'').exactly(3).times + + expect_to_execute_concurrently_in_order(drop_index) + expect_to_execute_concurrently_in_order(create_index) + + expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| + expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield + end + + expect_index_rename(index.name, replaced_name) + expect_index_rename(replacement_name, index.name) + expect_index_rename(replaced_name, replacement_name) + + expect_to_execute_concurrently_in_order(drop_index) + + subject.perform + + check_index_exists + end + end + + context 'when it fails to create the replacement index' do + it 'safely cleans up and signals the error' do + expect(connection).to receive(:execute).with(create_index).ordered + .and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout') + + expect_to_execute_concurrently_in_order(drop_index) + + expect { subject.perform }.to raise_error(ActiveRecord::ConnectionTimeoutError, /connect timeout/) + + check_index_exists + end + end + + context 'when the replacement index is not valid' do + it 'safely cleans up and signals the error' do + replacement_index = double('replacement index', valid_index?: false) + allow(Gitlab::Database::PostgresIndex).to receive(:find_by).with(schema: 'public', name: replacement_name).and_return(nil, replacement_index) + + expect_to_execute_concurrently_in_order(create_index) + + expect_to_execute_concurrently_in_order(drop_index) + + expect { subject.perform }.to raise_error(described_class::ReindexError, /replacement index was created as INVALID/) + + check_index_exists + end + end + + context 'when a database error occurs while swapping the indexes' do + it 'safely cleans up and signals the error' do + replacement_index = double('replacement index', valid_index?: true) + allow(Gitlab::Database::PostgresIndex).to receive(:find_by).with(schema: 'public', name: replacement_name).and_return(nil, replacement_index) + + expect_to_execute_concurrently_in_order(create_index) + + expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| + expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield + end + + expect_index_rename(index.name, replaced_name).and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout') + + expect_to_execute_concurrently_in_order(drop_index) + + expect { subject.perform }.to raise_error(ActiveRecord::ConnectionTimeoutError, /connect timeout/) + + check_index_exists + end + end + + context 'when with_lock_retries fails to acquire the lock' do + it 'safely cleans up and signals the error' do + expect_to_execute_concurrently_in_order(create_index) + + expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| + expect(instance).to receive(:run).with(raise_on_exhaustion: true) + .and_raise(::Gitlab::Database::WithLockRetries::AttemptsExhaustedError, 'exhausted') + end + + expect_to_execute_concurrently_in_order(drop_index) + + expect { subject.perform }.to raise_error(::Gitlab::Database::WithLockRetries::AttemptsExhaustedError, /exhausted/) + + check_index_exists + end + end + end + end + + def expect_to_execute_concurrently_in_order(sql) + # Indexes cannot be created CONCURRENTLY in a transaction. Since the tests are wrapped in transactions, + # verify the original call but pass through the non-concurrent form. + expect(connection).to receive(:execute).with(sql).ordered.and_wrap_original do |method, sql| + method.call(sql.sub(/CONCURRENTLY/, '')) + end + end + + def expect_index_rename(from, to) + expect(connection).to receive(:execute).with(<<~SQL).ordered + ALTER INDEX "public"."#{from}" + RENAME TO "#{to}" + SQL + end + + def find_index_create_statement + ActiveRecord::Base.connection.select_value(<<~SQL) + SELECT indexdef + FROM pg_indexes + WHERE schemaname = 'public' + AND indexname = #{ActiveRecord::Base.connection.quote(index.name)} + SQL + end + + def check_index_exists + expect(find_index_create_statement).to eq(original_index) + end +end diff --git a/spec/lib/gitlab/database/reindexing/coordinator_spec.rb b/spec/lib/gitlab/database/reindexing/coordinator_spec.rb new file mode 100644 index 00000000000..f45d959c0de --- /dev/null +++ b/spec/lib/gitlab/database/reindexing/coordinator_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Reindexing::Coordinator do + include ExclusiveLeaseHelpers + + describe '.perform' do + subject { described_class.new(indexes).perform } + + let(:indexes) { [instance_double(Gitlab::Database::PostgresIndex), instance_double(Gitlab::Database::PostgresIndex)] } + let(:reindexers) { [instance_double(Gitlab::Database::Reindexing::ConcurrentReindex), instance_double(Gitlab::Database::Reindexing::ConcurrentReindex)] } + + let!(:lease) { stub_exclusive_lease(lease_key, uuid, timeout: lease_timeout) } + let(:lease_key) { 'gitlab/database/reindexing/coordinator' } + let(:lease_timeout) { 1.day } + let(:uuid) { 'uuid' } + + before do + allow(Gitlab::Database::Reindexing::ReindexAction).to receive(:keep_track_of).and_yield + + indexes.zip(reindexers).each do |index, reindexer| + allow(Gitlab::Database::Reindexing::ConcurrentReindex).to receive(:new).with(index).and_return(reindexer) + allow(reindexer).to receive(:perform) + end + end + + it 'performs concurrent reindexing for each index' do + indexes.zip(reindexers).each do |index, reindexer| + expect(Gitlab::Database::Reindexing::ConcurrentReindex).to receive(:new).with(index).ordered.and_return(reindexer) + expect(reindexer).to receive(:perform) + end + + subject + end + + it 'keeps track of actions and creates ReindexAction records' do + indexes.each do |index| + expect(Gitlab::Database::Reindexing::ReindexAction).to receive(:keep_track_of).with(index).and_yield + end + + subject + end + + context 'locking' do + it 'acquires a lock while reindexing' do + indexes.each do |index| + expect(lease).to receive(:try_obtain).ordered.and_return(uuid) + action = instance_double(Gitlab::Database::Reindexing::ConcurrentReindex) + expect(Gitlab::Database::Reindexing::ConcurrentReindex).to receive(:new).ordered.with(index).and_return(action) + expect(action).to receive(:perform).ordered + expect(Gitlab::ExclusiveLease).to receive(:cancel).ordered.with(lease_key, uuid) + end + + subject + end + + it 'does does not perform reindexing actions if lease is not granted' do + indexes.each do |index| + expect(lease).to receive(:try_obtain).ordered.and_return(false) + expect(Gitlab::Database::Reindexing::ConcurrentReindex).not_to receive(:new) + end + + subject + end + end + end +end diff --git a/spec/lib/gitlab/database/reindexing/reindex_action_spec.rb b/spec/lib/gitlab/database/reindexing/reindex_action_spec.rb new file mode 100644 index 00000000000..efb5b8463a1 --- /dev/null +++ b/spec/lib/gitlab/database/reindexing/reindex_action_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Reindexing::ReindexAction, '.keep_track_of' do + let(:index) { double('index', identifier: 'public.something', ondisk_size_bytes: 10240, reload: nil) } + let(:size_after) { 512 } + + it 'yields to the caller' do + expect { |b| described_class.keep_track_of(index, &b) }.to yield_control + end + + def find_record + described_class.find_by(index_identifier: index.identifier) + end + + it 'creates the record with a start time and updates its end time' do + freeze_time do + described_class.keep_track_of(index) do + expect(find_record.action_start).to be_within(1.second).of(Time.zone.now) + + travel(10.seconds) + end + + duration = find_record.action_end - find_record.action_start + + expect(duration).to be_within(1.second).of(10.seconds) + end + end + + it 'creates the record with its status set to :started and updates its state to :finished' do + described_class.keep_track_of(index) do + expect(find_record).to be_started + end + + expect(find_record).to be_finished + end + + it 'creates the record with the indexes start size and updates its end size' do + described_class.keep_track_of(index) do + expect(find_record.ondisk_size_bytes_start).to eq(index.ondisk_size_bytes) + + expect(index).to receive(:reload).once + allow(index).to receive(:ondisk_size_bytes).and_return(size_after) + end + + expect(find_record.ondisk_size_bytes_end).to eq(size_after) + end + + context 'in case of errors' do + it 'sets the state to failed' do + expect do + described_class.keep_track_of(index) do + raise 'something went wrong' + end + end.to raise_error(/something went wrong/) + + expect(find_record).to be_failed + end + + it 'records the end time' do + freeze_time do + expect do + described_class.keep_track_of(index) do + raise 'something went wrong' + end + end.to raise_error(/something went wrong/) + + expect(find_record.action_end).to be_within(1.second).of(Time.zone.now) + end + end + + it 'records the resulting index size' do + expect(index).to receive(:reload).once + allow(index).to receive(:ondisk_size_bytes).and_return(size_after) + + expect do + described_class.keep_track_of(index) do + raise 'something went wrong' + end + end.to raise_error(/something went wrong/) + + expect(find_record.ondisk_size_bytes_end).to eq(size_after) + end + end +end |