diff options
Diffstat (limited to 'spec/lib/gitlab/database')
21 files changed, 720 insertions, 269 deletions
diff --git a/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb b/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb index 3daed2508a2..1ac9cbae036 100644 --- a/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb @@ -83,7 +83,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m describe '#execute!' do context 'when an invalid transition is applied' do - %i[finished finalizing].each do |state| + %i[finalizing finished].each do |state| it 'raises an exception' do batched_migration = create(:batched_background_migration, state) @@ -103,6 +103,48 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m end end + describe '#finish!' do + context 'when an invalid transition is applied' do + it 'raises an exception' do + batched_migration = create(:batched_background_migration, :failed) + + expect { batched_migration.finish! }.to raise_error(StateMachines::InvalidTransition, /Cannot transition status/) + end + end + + context 'when a valid transition is applied' do + %i[active paused finished finalizing].each do |state| + it 'moves to active' do + batched_migration = create(:batched_background_migration, state) + + expect(batched_migration.finish!).to be_truthy + end + end + end + end + + describe '#failure!' do + context 'when an invalid transition is applied' do + %i[paused finished].each do |state| + it 'raises an exception' do + batched_migration = create(:batched_background_migration, state) + + expect { batched_migration.failure! }.to raise_error(StateMachines::InvalidTransition, /Cannot transition status/) + end + end + end + + context 'when a valid transition is applied' do + %i[failed finalizing active].each do |state| + it 'moves to active' do + batched_migration = create(:batched_background_migration, state) + + expect(batched_migration.failure!).to be_truthy + end + end + end + end + describe '.valid_status' do valid_status = [:paused, :active, :finished, :failed, :finalizing] diff --git a/spec/lib/gitlab/database/each_database_spec.rb b/spec/lib/gitlab/database/each_database_spec.rb index 2a6eb8f779d..75b543bee85 100644 --- a/spec/lib/gitlab/database/each_database_spec.rb +++ b/spec/lib/gitlab/database/each_database_spec.rb @@ -93,12 +93,13 @@ RSpec.describe Gitlab::Database::EachDatabase do end it 'yields each model with SharedModel connected to each database connection' do - expect_yielded_models([model1, model2], [ - { model: model1, connection: ActiveRecord::Base.connection, name: 'main' }, - { model: model1, connection: Ci::ApplicationRecord.connection, name: 'ci' }, - { model: model2, connection: ActiveRecord::Base.connection, name: 'main' }, - { model: model2, connection: Ci::ApplicationRecord.connection, name: 'ci' } - ]) + expect_yielded_models([model1, model2], + [ + { model: model1, connection: ActiveRecord::Base.connection, name: 'main' }, + { model: model1, connection: Ci::ApplicationRecord.connection, name: 'ci' }, + { model: model2, connection: ActiveRecord::Base.connection, name: 'main' }, + { model: model2, connection: Ci::ApplicationRecord.connection, name: 'ci' } + ]) end context 'when the model limits connection names' do @@ -108,10 +109,11 @@ RSpec.describe Gitlab::Database::EachDatabase do end it 'only yields the model with SharedModel connected to the limited connections' do - expect_yielded_models([model1, model2], [ - { model: model1, connection: ActiveRecord::Base.connection, name: 'main' }, - { model: model2, connection: Ci::ApplicationRecord.connection, name: 'ci' } - ]) + expect_yielded_models([model1, model2], + [ + { model: model1, connection: ActiveRecord::Base.connection, name: 'main' }, + { model: model2, connection: Ci::ApplicationRecord.connection, name: 'ci' } + ]) end end end @@ -132,10 +134,11 @@ RSpec.describe Gitlab::Database::EachDatabase do end it 'yields each model after connecting SharedModel' do - expect_yielded_models([main_model, ci_model], [ - { model: main_model, connection: main_connection, name: 'main' }, - { model: ci_model, connection: ci_connection, name: 'ci' } - ]) + expect_yielded_models([main_model, ci_model], + [ + { model: main_model, connection: main_connection, name: 'main' }, + { model: ci_model, connection: ci_connection, name: 'ci' } + ]) end end @@ -154,21 +157,23 @@ RSpec.describe Gitlab::Database::EachDatabase do context 'when a single name is passed in' do it 'yields models only connected to the given database' do - expect_yielded_models([main_model, ci_model, shared_model], [ - { model: ci_model, connection: Ci::ApplicationRecord.connection, name: 'ci' }, - { model: shared_model, connection: Ci::ApplicationRecord.connection, name: 'ci' } - ], only_on: 'ci') + expect_yielded_models([main_model, ci_model, shared_model], + [ + { model: ci_model, connection: Ci::ApplicationRecord.connection, name: 'ci' }, + { model: shared_model, connection: Ci::ApplicationRecord.connection, name: 'ci' } + ], only_on: 'ci') end end context 'when a list of names are passed in' do it 'yields models only connected to the given databases' do - expect_yielded_models([main_model, ci_model, shared_model], [ - { model: main_model, connection: ActiveRecord::Base.connection, name: 'main' }, - { model: ci_model, connection: Ci::ApplicationRecord.connection, name: 'ci' }, - { model: shared_model, connection: ActiveRecord::Base.connection, name: 'main' }, - { model: shared_model, connection: Ci::ApplicationRecord.connection, name: 'ci' } - ], only_on: %i[main ci]) + expect_yielded_models([main_model, ci_model, shared_model], + [ + { model: main_model, connection: ActiveRecord::Base.connection, name: 'main' }, + { model: ci_model, connection: Ci::ApplicationRecord.connection, name: 'ci' }, + { model: shared_model, connection: ActiveRecord::Base.connection, name: 'main' }, + { model: shared_model, connection: Ci::ApplicationRecord.connection, name: 'ci' } + ], only_on: %i[main ci]) end end end diff --git a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb index 9c09253b24c..997c7a31cba 100644 --- a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb @@ -210,10 +210,25 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do end it 'uses a retry with exponential backoffs' do - expect(lb).to receive(:retry_with_backoff).and_yield + expect(lb).to receive(:retry_with_backoff).and_yield(0) lb.read_write { 10 } end + + it 'does not raise NoMethodError error when primary_only?' do + connection = ActiveRecord::Base.connection_pool.connection + expected_error = Gitlab::Database::LoadBalancing::CONNECTION_ERRORS.first + + allow(lb).to receive(:primary_only?).and_return(true) + + expect do + lb.read_write do + connection.transaction do + raise expected_error + end + end + end.to raise_error(expected_error) + end end describe '#host' do @@ -330,6 +345,19 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do expect { lb.retry_with_backoff { raise } }.to raise_error(RuntimeError) end + + it 'yields the current retry iteration' do + allow(lb).to receive(:connection_error?).and_return(true) + expect(lb).to receive(:release_primary_connection).exactly(3).times + iterations = [] + + # time: 0 so that we don't sleep and slow down the test + # rubocop: disable Style/Semicolon + expect { lb.retry_with_backoff(attempts: 3, time: 0) { |i| iterations << i; raise } }.to raise_error(RuntimeError) + # rubocop: enable Style/Semicolon + + expect(iterations).to eq([1, 2, 3]) + end end describe '#connection_error?' do diff --git a/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb index a1c141af537..713bff5feea 100644 --- a/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb @@ -9,10 +9,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do let(:single_sticking_object) { Set.new([[ActiveRecord::Base.sticking, :user, 42]]) } let(:multiple_sticking_objects) do Set.new([ - [ActiveRecord::Base.sticking, :user, 42], - [ActiveRecord::Base.sticking, :runner, '123456789'], - [ActiveRecord::Base.sticking, :runner, '1234'] - ]) + [ActiveRecord::Base.sticking, :user, 42], + [ActiveRecord::Base.sticking, :runner, '123456789'], + [ActiveRecord::Base.sticking, :runner, '1234'] + ]) end after do @@ -182,11 +182,12 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do it 'returns the sticking object' do env = { described_class::STICK_OBJECT => multiple_sticking_objects } - expect(middleware.sticking_namespaces(env)).to eq([ - [ActiveRecord::Base.sticking, :user, 42], - [ActiveRecord::Base.sticking, :runner, '123456789'], - [ActiveRecord::Base.sticking, :runner, '1234'] - ]) + expect(middleware.sticking_namespaces(env)).to eq( + [ + [ActiveRecord::Base.sticking, :user, 42], + [ActiveRecord::Base.sticking, :runner, '123456789'], + [ActiveRecord::Base.sticking, :runner, '1234'] + ]) end end diff --git a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb index 8053bd57bba..88007de53d3 100644 --- a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ let(:middleware) { described_class.new } let(:worker) { worker_class.new } let(:location) { '0/D525E3A8' } - let(:wal_locations) { { Gitlab::Database::MAIN_DATABASE_NAME.to_sym => location } } + let(:wal_locations) { { Gitlab::Database::MAIN_DATABASE_NAME.to_s => location } } let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'wal_locations' => wal_locations } } before do @@ -315,6 +315,46 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ expect(middleware.send(:databases_in_sync?, locations)) .to eq(false) end + + context 'when locations have string keys' do + it 'returns false when the load balancers are not in sync' do + locations = {} + + Gitlab::Database::LoadBalancing.each_load_balancer do |lb| + locations[lb.name.to_s] = 'foo' + + allow(lb) + .to receive(:select_up_to_date_host) + .with('foo') + .and_return(false) + end + + expect(middleware.send(:databases_in_sync?, locations)) + .to eq(false) + end + + context 'when "indifferent_wal_location_keys" FF is off' do + before do + stub_feature_flags(indifferent_wal_location_keys: false) + end + + it 'returns true when the load balancers are not in sync' do + locations = {} + + Gitlab::Database::LoadBalancing.each_load_balancer do |lb| + locations[lb.name.to_s] = 'foo' + + allow(lb) + .to receive(:select_up_to_date_host) + .with('foo') + .and_return(false) + end + + expect(middleware.send(:databases_in_sync?, locations)) + .to eq(true) + end + end + end end def process_job(job) diff --git a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb index 2ffb2c32c32..1e316c55786 100644 --- a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb @@ -41,10 +41,12 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do sticking.stick_or_unstick_request(env, :user, 42) sticking.stick_or_unstick_request(env, :runner, '123456789') - expect(env[Gitlab::Database::LoadBalancing::RackMiddleware::STICK_OBJECT].to_a).to eq([ - [sticking, :user, 42], - [sticking, :runner, '123456789'] - ]) + expect(env[Gitlab::Database::LoadBalancing::RackMiddleware::STICK_OBJECT].to_a).to eq( + [ + [sticking, :user, 42], + [sticking, :runner, + '123456789'] + ]) end end diff --git a/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb b/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb index 30e5fbbd803..6026d979bcf 100644 --- a/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe 'Load balancer behavior with errors inside a transaction', :redis, :delete do + include StubENV let(:model) { ApplicationRecord } let(:db_host) { model.connection_pool.db_config.host } @@ -19,6 +20,10 @@ RSpec.describe 'Load balancer behavior with errors inside a transaction', :redis model.connection.execute(<<~SQL) CREATE TABLE IF NOT EXISTS #{test_table_name} (id SERIAL PRIMARY KEY, value INTEGER) SQL + + # The load balancer sleeps between attempts to retry a query. + # Mocking the sleep call significantly reduces the runtime of this spec file. + allow(model.connection.load_balancer).to receive(:sleep) end after do @@ -46,36 +51,62 @@ RSpec.describe 'Load balancer behavior with errors inside a transaction', :redis conn.execute("INSERT INTO #{test_table_name} (value) VALUES (2)") end - it 'logs a warning when violating transaction semantics with writes' do - conn = model.connection + context 'with the PREVENT_LOAD_BALANCER_RETRIES_IN_TRANSACTION environment variable not set' do + it 'logs a warning when violating transaction semantics with writes' do + conn = model.connection + + expect(::Gitlab::Database::LoadBalancing::Logger).to receive(:warn).with(hash_including(event: :transaction_leak)) + + conn.transaction do + expect(conn).to be_transaction_open + + execute(conn) - expect(::Gitlab::Database::LoadBalancing::Logger).to receive(:warn).with(hash_including(event: :transaction_leak)) + expect(conn).not_to be_transaction_open + end - conn.transaction do - expect(conn).to be_transaction_open + values = conn.execute("SELECT value FROM #{test_table_name}").to_a.map { |row| row['value'] } + expect(values).to contain_exactly(2) # Does not include 1 because the transaction was aborted and leaked + end + + it 'does not log a warning when no transaction is open to be leaked' do + conn = model.connection + + expect(::Gitlab::Database::LoadBalancing::Logger) + .not_to receive(:warn).with(hash_including(event: :transaction_leak)) + + expect(conn).not_to be_transaction_open execute(conn) expect(conn).not_to be_transaction_open - end - values = conn.execute("SELECT value FROM #{test_table_name}").to_a.map { |row| row['value'] } - expect(values).to contain_exactly(2) # Does not include 1 because the transaction was aborted and leaked + values = conn.execute("SELECT value FROM #{test_table_name}").to_a.map { |row| row['value'] } + expect(values).to contain_exactly(1, 2) # Includes both rows because there was no transaction to roll back + end end - it 'does not log a warning when no transaction is open to be leaked' do - conn = model.connection - - expect(::Gitlab::Database::LoadBalancing::Logger) - .not_to receive(:warn).with(hash_including(event: :transaction_leak)) + context 'with the PREVENT_LOAD_BALANCER_RETRIES_IN_TRANSACTION environment variable set' do + before do + stub_env('PREVENT_LOAD_BALANCER_RETRIES_IN_TRANSACTION' => '1') + end - expect(conn).not_to be_transaction_open + it 'raises an exception when a retry would occur during a transaction' do + expect(::Gitlab::Database::LoadBalancing::Logger) + .not_to receive(:warn).with(hash_including(event: :transaction_leak)) - execute(conn) + expect do + model.transaction do + execute(model.connection) + end + end.to raise_error(ActiveRecord::StatementInvalid) { |e| expect(e.cause).to be_a(PG::ConnectionBad) } + end - expect(conn).not_to be_transaction_open + it 'retries when not in a transaction' do + expect(::Gitlab::Database::LoadBalancing::Logger) + .not_to receive(:warn).with(hash_including(event: :transaction_leak)) - values = conn.execute("SELECT value FROM #{test_table_name}").to_a.map { |row| row['value'] } - expect(values).to contain_exactly(1, 2) # Includes both rows because there was no transaction to roll back + expect { execute(model.connection) }.not_to raise_error + end end end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index d73b478ee7c..bcdd5646994 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -757,6 +757,58 @@ RSpec.describe Gitlab::Database::MigrationHelpers do model.add_concurrent_foreign_key(:projects, :users, column: :user_id, reverse_lock_order: true) end end + + context 'when creating foreign key for a group of columns' do + it 'references the custom target columns when provided', :aggregate_failures do + expect(model).to receive(:with_lock_retries).and_yield + expect(model).to receive(:execute).with( + "ALTER TABLE projects\n" \ + "ADD CONSTRAINT fk_multiple_columns\n" \ + "FOREIGN KEY \(partition_number, user_id\)\n" \ + "REFERENCES users \(partition_number, id\)\n" \ + "ON DELETE CASCADE\n" \ + "NOT VALID;\n" + ) + + model.add_concurrent_foreign_key( + :projects, + :users, + column: [:partition_number, :user_id], + target_column: [:partition_number, :id], + validate: false, + name: :fk_multiple_columns + ) + end + + context 'when foreign key is already defined' do + before do + expect(model).to receive(:foreign_key_exists?).with( + :projects, + :users, + { + column: [:partition_number, :user_id], + name: :fk_multiple_columns, + on_delete: :cascade, + primary_key: [:partition_number, :id] + } + ).and_return(true) + end + + it 'does not create foreign key', :aggregate_failures do + expect(model).not_to receive(:with_lock_retries).and_yield + expect(model).not_to receive(:execute).with(/FOREIGN KEY/) + + model.add_concurrent_foreign_key( + :projects, + :users, + column: [:partition_number, :user_id], + target_column: [:partition_number, :id], + validate: false, + name: :fk_multiple_columns + ) + end + end + end end end @@ -813,6 +865,15 @@ RSpec.describe Gitlab::Database::MigrationHelpers do expect(name).to be_an_instance_of(String) expect(name.length).to eq(13) end + + context 'when using multiple columns' do + it 'returns the name of the foreign key', :aggregate_failures do + result = model.concurrent_foreign_key_name(:table_name, [:partition_number, :id]) + + expect(result).to be_an_instance_of(String) + expect(result.length).to eq(13) + end + end end describe '#foreign_key_exists?' do @@ -887,6 +948,62 @@ RSpec.describe Gitlab::Database::MigrationHelpers do it 'compares by target table if no column given' do expect(model.foreign_key_exists?(:projects, :other_table)).to be_falsey end + + context 'with foreign key using multiple columns' do + before do + key = ActiveRecord::ConnectionAdapters::ForeignKeyDefinition.new( + :projects, :users, + { + column: [:partition_number, :id], + name: :fk_projects_users_partition_number_id, + on_delete: :cascade, + primary_key: [:partition_number, :id] + } + ) + allow(model).to receive(:foreign_keys).with(:projects).and_return([key]) + end + + it 'finds existing foreign keys by columns' do + expect(model.foreign_key_exists?(:projects, :users, column: [:partition_number, :id])).to be_truthy + end + + it 'finds existing foreign keys by name' do + expect(model.foreign_key_exists?(:projects, :users, name: :fk_projects_users_partition_number_id)).to be_truthy + end + + it 'finds existing foreign_keys by name and column' do + expect(model.foreign_key_exists?(:projects, :users, name: :fk_projects_users_partition_number_id, column: [:partition_number, :id])).to be_truthy + end + + it 'finds existing foreign_keys by name, column and on_delete' do + expect(model.foreign_key_exists?(:projects, :users, name: :fk_projects_users_partition_number_id, column: [:partition_number, :id], on_delete: :cascade)).to be_truthy + end + + it 'finds existing foreign keys by target table only' do + expect(model.foreign_key_exists?(:projects, :users)).to be_truthy + end + + it 'compares by column name if given' do + expect(model.foreign_key_exists?(:projects, :users, column: :id)).to be_falsey + end + + it 'compares by target column name if given' do + expect(model.foreign_key_exists?(:projects, :users, primary_key: :user_id)).to be_falsey + expect(model.foreign_key_exists?(:projects, :users, primary_key: [:partition_number, :id])).to be_truthy + end + + it 'compares by foreign key name if given' do + expect(model.foreign_key_exists?(:projects, :users, name: :non_existent_foreign_key_name)).to be_falsey + end + + it 'compares by foreign key name and column if given' do + expect(model.foreign_key_exists?(:projects, :users, name: :non_existent_foreign_key_name, column: [:partition_number, :id])).to be_falsey + end + + it 'compares by foreign key name, column and on_delete if given' do + expect(model.foreign_key_exists?(:projects, :users, name: :fk_projects_users_partition_number_id, column: [:partition_number, :id], on_delete: :nullify)).to be_falsey + end + end end describe '#disable_statement_timeout' do @@ -3359,6 +3476,73 @@ RSpec.describe Gitlab::Database::MigrationHelpers do end end + describe '#drop_constraint' do + it "executes the statement to drop the constraint" do + expect(model).to receive(:execute).with("ALTER TABLE \"test_table\" DROP CONSTRAINT \"constraint_name\" CASCADE\n") + + model.drop_constraint(:test_table, :constraint_name, cascade: true) + end + + context 'when cascade option is false' do + it "executes the statement to drop the constraint without cascade" do + expect(model).to receive(:execute).with("ALTER TABLE \"test_table\" DROP CONSTRAINT \"constraint_name\" \n") + + model.drop_constraint(:test_table, :constraint_name, cascade: false) + end + end + end + + describe '#add_primary_key_using_index' do + it "executes the statement to add the primary key" do + expect(model).to receive(:execute).with /ALTER TABLE "test_table" ADD CONSTRAINT "old_name" PRIMARY KEY USING INDEX "new_name"/ + + model.add_primary_key_using_index(:test_table, :old_name, :new_name) + end + end + + context 'when changing the primary key of a given table' do + before do + model.create_table(:test_table, primary_key: :id) do |t| + t.integer :partition_number, default: 1 + end + + model.add_index(:test_table, :id, unique: true, name: :old_index_name) + model.add_index(:test_table, [:id, :partition_number], unique: true, name: :new_index_name) + end + + describe '#swap_primary_key' do + it 'executes statements to swap primary key', :aggregate_failures do + expect(model).to receive(:with_lock_retries).with(raise_on_exhaustion: true).ordered.and_yield + expect(model).to receive(:execute).with(/ALTER TABLE "test_table" DROP CONSTRAINT "test_table_pkey" CASCADE/).and_call_original + expect(model).to receive(:execute).with(/ALTER TABLE "test_table" ADD CONSTRAINT "test_table_pkey" PRIMARY KEY USING INDEX "new_index_name"/).and_call_original + + model.swap_primary_key(:test_table, :test_table_pkey, :new_index_name) + end + + context 'when new index does not exist' do + before do + model.remove_index(:test_table, column: [:id, :partition_number]) + end + + it 'raises ActiveRecord::StatementInvalid' do + expect do + model.swap_primary_key(:test_table, :test_table_pkey, :new_index_name) + end.to raise_error(ActiveRecord::StatementInvalid) + end + end + end + + describe '#unswap_primary_key' do + it 'executes statements to unswap primary key' do + expect(model).to receive(:with_lock_retries).with(raise_on_exhaustion: true).ordered.and_yield + expect(model).to receive(:execute).with(/ALTER TABLE "test_table" DROP CONSTRAINT "test_table_pkey" CASCADE/).ordered.and_call_original + expect(model).to receive(:execute).with(/ALTER TABLE "test_table" ADD CONSTRAINT "test_table_pkey" PRIMARY KEY USING INDEX "old_index_name"/).ordered.and_call_original + + model.unswap_primary_key(:test_table, :test_table_pkey, :old_index_name) + end + end + end + describe '#drop_sequence' do it "executes the statement to drop the sequence" do expect(model).to receive(:execute).with /ALTER TABLE "test_table" ALTER COLUMN "test_column" DROP DEFAULT;\nDROP SEQUENCE IF EXISTS "test_table_id_seq"/ diff --git a/spec/lib/gitlab/database/migrations/base_background_runner_spec.rb b/spec/lib/gitlab/database/migrations/base_background_runner_spec.rb index 34c83c42056..c2dc260b2ff 100644 --- a/spec/lib/gitlab/database/migrations/base_background_runner_spec.rb +++ b/spec/lib/gitlab/database/migrations/base_background_runner_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Migrations::BaseBackgroundRunner, :freeze_time do + let(:connection) { ApplicationRecord.connection } + let(:result_dir) { Dir.mktmpdir } after do @@ -10,7 +12,7 @@ RSpec.describe Gitlab::Database::Migrations::BaseBackgroundRunner, :freeze_time end context 'subclassing' do - subject { described_class.new(result_dir: result_dir) } + subject { described_class.new(result_dir: result_dir, connection: connection) } it 'requires that jobs_by_migration_name be implemented' do expect { subject.jobs_by_migration_name }.to raise_error(NotImplementedError) diff --git a/spec/lib/gitlab/database/migrations/runner_spec.rb b/spec/lib/gitlab/database/migrations/runner_spec.rb index a37247ba0c6..f364ebfa522 100644 --- a/spec/lib/gitlab/database/migrations/runner_spec.rb +++ b/spec/lib/gitlab/database/migrations/runner_spec.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Gitlab::Database::Migrations::Runner do +RSpec.describe Gitlab::Database::Migrations::Runner, :reestablished_active_record_base do include Database::MultipleDatabases - let(:result_dir) { Pathname.new(Dir.mktmpdir) } + let(:base_result_dir) { Pathname.new(Dir.mktmpdir) } let(:migration_runs) { [] } # This list gets populated as the runner tries to run migrations @@ -26,11 +26,14 @@ RSpec.describe Gitlab::Database::Migrations::Runner do end before do - stub_const('Gitlab::Database::Migrations::Runner::BASE_RESULT_DIR', result_dir) + skip_if_multiple_databases_not_setup unless database == :main + + stub_const('Gitlab::Database::Migrations::Runner::BASE_RESULT_DIR', base_result_dir) allow(ActiveRecord::Migrator).to receive(:new) do |dir, _all_migrations, _schema_migration_class, version_to_migrate| migrator = double(ActiveRecord::Migrator) expect(migrator).to receive(:run) do - migration_runs << double('migrator', dir: dir, version_to_migrate: version_to_migrate) + config_for_migration_run = ActiveRecord::Base.connection_db_config + migration_runs << double('migrator', dir: dir, version_to_migrate: version_to_migrate, database: config_for_migration_run.name) end migrator end @@ -39,133 +42,153 @@ RSpec.describe Gitlab::Database::Migrations::Runner do migrations = applied_migrations_other_branches + applied_migrations_this_branch + pending_migrations ctx = double(ActiveRecord::MigrationContext, get_all_versions: all_versions, migrations: migrations, schema_migration: ActiveRecord::SchemaMigration) - allow(described_class).to receive(:migration_context).and_return(ctx) + allow(ActiveRecord::Base.connection).to receive(:migration_context).and_return(ctx) names_this_branch = (applied_migrations_this_branch + pending_migrations).map { |m| "db/migrate/#{m.version}_#{m.name}.rb" } allow(described_class).to receive(:migration_file_names_this_branch).and_return(names_this_branch) end after do - FileUtils.rm_rf(result_dir) + FileUtils.rm_rf(base_result_dir) end - it 'creates the results dir when one does not exist' do - FileUtils.rm_rf(result_dir) - - expect do - described_class.new(direction: :up, migrations: [], result_dir: result_dir).run - end.to change { Dir.exist?(result_dir) }.from(false).to(true) + where(:case_name, :database, :result_dir, :legacy_mode, :expected_schema_version) do + [ + ['main database', :main, lazy { base_result_dir.join('main') }, false, described_class::SCHEMA_VERSION], + ['main database (legacy mode)', :main, lazy { base_result_dir }, true, 3], + ['ci database', :ci, lazy { base_result_dir.join('ci') }, false, described_class::SCHEMA_VERSION] + ] end - describe '.up' do - context 'result directory' do - it 'uses the /up subdirectory' do - expect(described_class.up.result_dir).to eq(result_dir.join('up')) - end + with_them do + it 'creates the results dir when one does not exist' do + FileUtils.rm_rf(result_dir) + + expect do + described_class.new(direction: :up, migrations: [], database: database).run + end.to change { Dir.exist?(result_dir) }.from(false).to(true) end - context 'migrations to run' do - subject(:up) { described_class.up } + describe '.up' do + context 'result directory' do + it 'uses the /up subdirectory' do + expect(described_class.up(database: database, legacy_mode: legacy_mode).result_dir).to eq(result_dir.join('up')) + end + end + + context 'migrations to run' do + subject(:up) { described_class.up(database: database, legacy_mode: legacy_mode) } - it 'is the list of pending migrations' do - expect(up.migrations).to eq(pending_migrations) + it 'is the list of pending migrations' do + expect(up.migrations).to eq(pending_migrations) + end end - end - context 'running migrations' do - subject(:up) { described_class.up } + context 'running migrations' do + subject(:up) { described_class.up(database: database, legacy_mode: legacy_mode) } - it 'runs the unapplied migrations in version order', :aggregate_failures do - up.run + it 'runs the unapplied migrations in version order', :aggregate_failures do + up.run - expect(migration_runs.map(&:dir)).to match_array([:up, :up]) - expect(migration_runs.map(&:version_to_migrate)).to eq(pending_migrations.map(&:version)) - end + expect(migration_runs.map(&:dir)).to match_array([:up, :up]) + expect(migration_runs.map(&:version_to_migrate)).to eq(pending_migrations.map(&:version)) + end - it 'writes a metadata file with the current schema version' do - up.run + it 'writes a metadata file with the current schema version and database name' do + up.run - metadata_file = result_dir.join('up', described_class::METADATA_FILENAME) - expect(metadata_file.exist?).to be_truthy - metadata = Gitlab::Json.parse(File.read(metadata_file)) - expect(metadata).to match('version' => described_class::SCHEMA_VERSION) + metadata_file = result_dir.join('up', described_class::METADATA_FILENAME) + expect(metadata_file.exist?).to be_truthy + metadata = Gitlab::Json.parse(File.read(metadata_file)) + expect(metadata).to match('version' => expected_schema_version, 'database' => database.to_s) + end + + it 'runs the unapplied migrations on the correct database' do + up.run + + expect(migration_runs.map(&:database).uniq).to contain_exactly(database.to_s) + end end end - end - describe '.down' do - subject(:down) { described_class.down } + describe '.down' do + subject(:down) { described_class.down(database: database, legacy_mode: legacy_mode) } - context 'result directory' do - it 'is the /down subdirectory' do - expect(down.result_dir).to eq(result_dir.join('down')) + context 'result directory' do + it 'is the /down subdirectory' do + expect(down.result_dir).to eq(result_dir.join('down')) + end end - end - context 'migrations to run' do - it 'is the list of migrations that are up and on this branch' do - expect(down.migrations).to eq(applied_migrations_this_branch) + context 'migrations to run' do + it 'is the list of migrations that are up and on this branch' do + expect(down.migrations).to eq(applied_migrations_this_branch) + end end - end - context 'running migrations' do - it 'runs the applied migrations for the current branch in reverse order', :aggregate_failures do - down.run + context 'running migrations' do + it 'runs the applied migrations for the current branch in reverse order', :aggregate_failures do + down.run - expect(migration_runs.map(&:dir)).to match_array([:down, :down]) - expect(migration_runs.map(&:version_to_migrate)).to eq(applied_migrations_this_branch.reverse.map(&:version)) + expect(migration_runs.map(&:dir)).to match_array([:down, :down]) + expect(migration_runs.map(&:version_to_migrate)).to eq(applied_migrations_this_branch.reverse.map(&:version)) + end end - end - - it 'writes a metadata file with the current schema version' do - down.run - metadata_file = result_dir.join('down', described_class::METADATA_FILENAME) - expect(metadata_file.exist?).to be_truthy - metadata = Gitlab::Json.parse(File.read(metadata_file)) - expect(metadata).to match('version' => described_class::SCHEMA_VERSION) - end - end + it 'writes a metadata file with the current schema version' do + down.run - describe '.background_migrations' do - it 'is a TestBackgroundRunner' do - expect(described_class.background_migrations).to be_a(Gitlab::Database::Migrations::TestBackgroundRunner) + metadata_file = result_dir.join('down', described_class::METADATA_FILENAME) + expect(metadata_file.exist?).to be_truthy + metadata = Gitlab::Json.parse(File.read(metadata_file)) + expect(metadata).to match('version' => expected_schema_version, 'database' => database.to_s) + end end - it 'is configured with a result dir of /background_migrations' do - runner = described_class.background_migrations + describe '.background_migrations' do + it 'is a TestBackgroundRunner' do + expect(described_class.background_migrations).to be_a(Gitlab::Database::Migrations::TestBackgroundRunner) + end - expect(runner.result_dir).to eq(described_class::BASE_RESULT_DIR.join( 'background_migrations')) - end - end + it 'is configured with a result dir of /background_migrations' do + runner = described_class.background_migrations - describe '.batched_background_migrations' do - it 'is a TestBatchedBackgroundRunner' do - expect(described_class.batched_background_migrations(for_database: 'main')).to be_a(Gitlab::Database::Migrations::TestBatchedBackgroundRunner) + expect(runner.result_dir).to eq(described_class::BASE_RESULT_DIR.join( 'background_migrations')) + end end - context 'choosing the database to test against' do - it 'chooses the main database' do - runner = described_class.batched_background_migrations(for_database: 'main') + describe '.batched_background_migrations' do + it 'is a TestBatchedBackgroundRunner' do + expect(described_class.batched_background_migrations(for_database: database)).to be_a(Gitlab::Database::Migrations::TestBatchedBackgroundRunner) + end - chosen_connection_name = Gitlab::Database.db_config_name(runner.connection) + context 'choosing the database to test against' do + it 'chooses the provided database' do + runner = described_class.batched_background_migrations(for_database: database) - expect(chosen_connection_name).to eq('main') - end + chosen_connection_name = Gitlab::Database.db_config_name(runner.connection) - it 'chooses the ci database' do - skip_if_multiple_databases_not_setup + expect(chosen_connection_name).to eq(database.to_s) + end - runner = described_class.batched_background_migrations(for_database: 'ci') + it 'throws an error with an invalid name' do + expect { described_class.batched_background_migrations(for_database: 'not_a_database') } + .to raise_error(/not a valid database name/) + end - chosen_connection_name = Gitlab::Database.db_config_name(runner.connection) + it 'includes the database name in the result dir' do + runner = described_class.batched_background_migrations(for_database: database) - expect(chosen_connection_name).to eq('ci') + expect(runner.result_dir).to eq(base_result_dir.join(database.to_s, 'background_migrations')) + end end - it 'throws an error with an invalid name' do - expect { described_class.batched_background_migrations(for_database: 'not_a_database') } - .to raise_error(/not a valid database name/) + context 'legacy mode' do + it 'does not include the database name in the path' do + runner = described_class.batched_background_migrations(for_database: database, legacy_mode: true) + + expect(runner.result_dir).to eq(base_result_dir.join('background_migrations')) + end end end end diff --git a/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb b/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb index 3ac483c8ab7..07226f3d025 100644 --- a/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb +++ b/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb @@ -6,106 +6,156 @@ RSpec.describe Gitlab::Database::Migrations::TestBatchedBackgroundRunner, :freez include Gitlab::Database::MigrationHelpers include Database::MigrationTestingHelpers - let(:result_dir) { Dir.mktmpdir } - - after do - FileUtils.rm_rf(result_dir) + def queue_migration( + job_class_name, + batch_table_name, + batch_column_name, + *job_arguments, + job_interval:, + batch_size: Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers::BATCH_SIZE, + sub_batch_size: Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers::SUB_BATCH_SIZE + ) + + batch_max_value = define_batchable_model(batch_table_name, connection: connection).maximum(batch_column_name) + + Gitlab::Database::SharedModel.using_connection(connection) do + Gitlab::Database::BackgroundMigration::BatchedMigration.create!( + job_class_name: job_class_name, + table_name: batch_table_name, + column_name: batch_column_name, + job_arguments: job_arguments, + interval: job_interval, + min_value: Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers::BATCH_MIN_VALUE, + max_value: batch_max_value, + batch_class_name: Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers::BATCH_CLASS_NAME, + batch_size: batch_size, + sub_batch_size: sub_batch_size, + status_event: :execute, + max_batch_size: nil, + gitlab_schema: gitlab_schema + ) + end end - let(:migration) do - ActiveRecord::Migration.new.extend(Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers) + where(:case_name, :base_model, :gitlab_schema) do + [ + ['main database', ApplicationRecord, :gitlab_main], + ['ci database', Ci::ApplicationRecord, :gitlab_ci] + ] end - let(:connection) { ApplicationRecord.connection } + with_them do + let(:result_dir) { Dir.mktmpdir } - let(:table_name) { "_test_column_copying" } + after do + FileUtils.rm_rf(result_dir) + end - before do - connection.execute(<<~SQL) - CREATE TABLE #{table_name} ( - id bigint primary key not null, - data bigint default 0 - ); + let(:connection) { base_model.connection } - insert into #{table_name} (id) select i from generate_series(1, 1000) g(i); - SQL + let(:table_name) { "_test_column_copying" } - allow(migration).to receive(:transaction_open?).and_return(false) - end + before do + connection.execute(<<~SQL) + CREATE TABLE #{table_name} ( + id bigint primary key not null, + data bigint default 0 + ); - context 'running a real background migration' do - it 'runs sampled jobs from the batched background migration' do - migration.queue_batched_background_migration('CopyColumnUsingBackgroundMigrationJob', - table_name, :id, - :id, :data, - batch_size: 100, - job_interval: 5.minutes) # job_interval is skipped when testing - - # Expect that running sampling for this migration processes some of the rows. Sampling doesn't run - # over every row in the table, so this does not completely migrate the table. - expect { described_class.new(result_dir: result_dir, connection: connection).run_jobs(for_duration: 1.minute) } - .to change { define_batchable_model(table_name).where('id IS DISTINCT FROM data').count } - .by_at_most(-1) + insert into #{table_name} (id) select i from generate_series(1, 1000) g(i); + SQL end - end - context 'with jobs to run' do - let(:migration_name) { 'TestBackgroundMigration' } + context 'running a real background migration' do + before do + queue_migration('CopyColumnUsingBackgroundMigrationJob', + table_name, :id, + :id, :data, + batch_size: 100, + job_interval: 5.minutes) # job_interval is skipped when testing + end - it 'samples jobs' do - calls = [] - define_background_migration(migration_name) do |*args| - calls << args + subject(:sample_migration) do + described_class.new(result_dir: result_dir, connection: connection).run_jobs(for_duration: 1.minute) end - migration.queue_batched_background_migration(migration_name, table_name, :id, - job_interval: 5.minutes, - batch_size: 100) + it 'runs sampled jobs from the batched background migration' do + # Expect that running sampling for this migration processes some of the rows. Sampling doesn't run + # over every row in the table, so this does not completely migrate the table. + expect { subject }.to change { + define_batchable_model(table_name, connection: connection) + .where('id IS DISTINCT FROM data').count + }.by_at_most(-1) + end - described_class.new(result_dir: result_dir, connection: connection).run_jobs(for_duration: 3.minutes) + it 'uses the correct connection to instrument the background migration' do + expect_next_instance_of(Gitlab::Database::Migrations::Instrumentation) do |instrumentation| + expect(instrumentation).to receive(:observe).with(hash_including(connection: connection)) + .at_least(:once).and_call_original + end - expect(calls).not_to be_empty + subject + end end - context 'with multiple jobs to run' do - it 'runs all jobs created within the last 3 hours' do - old_migration = define_background_migration(migration_name) - migration.queue_batched_background_migration(migration_name, table_name, :id, - job_interval: 5.minutes, - batch_size: 100) - - travel 4.hours - - new_migration = define_background_migration('NewMigration') { travel 1.second } - migration.queue_batched_background_migration('NewMigration', table_name, :id, - job_interval: 5.minutes, - batch_size: 10, - sub_batch_size: 5) - - other_new_migration = define_background_migration('NewMigration2') { travel 2.seconds } - migration.queue_batched_background_migration('NewMigration2', table_name, :id, - job_interval: 5.minutes, - batch_size: 10, - sub_batch_size: 5) - - expect_migration_runs(new_migration => 3, other_new_migration => 2, old_migration => 0) do - described_class.new(result_dir: result_dir, connection: connection).run_jobs(for_duration: 5.seconds) + context 'with jobs to run' do + let(:migration_name) { 'TestBackgroundMigration' } + + it 'samples jobs' do + calls = [] + define_background_migration(migration_name) do |*args| + calls << args + end + + queue_migration(migration_name, table_name, :id, + job_interval: 5.minutes, + batch_size: 100) + + described_class.new(result_dir: result_dir, connection: connection).run_jobs(for_duration: 3.minutes) + + expect(calls).not_to be_empty + end + + context 'with multiple jobs to run' do + it 'runs all jobs created within the last 3 hours' do + old_migration = define_background_migration(migration_name) + queue_migration(migration_name, table_name, :id, + job_interval: 5.minutes, + batch_size: 100) + + travel 4.hours + + new_migration = define_background_migration('NewMigration') { travel 1.second } + queue_migration('NewMigration', table_name, :id, + job_interval: 5.minutes, + batch_size: 10, + sub_batch_size: 5) + + other_new_migration = define_background_migration('NewMigration2') { travel 2.seconds } + queue_migration('NewMigration2', table_name, :id, + job_interval: 5.minutes, + batch_size: 10, + sub_batch_size: 5) + + expect_migration_runs(new_migration => 3, other_new_migration => 2, old_migration => 0) do + described_class.new(result_dir: result_dir, connection: connection).run_jobs(for_duration: 5.seconds) + end end end end - end - context 'choosing uniform batches to run' do - subject { described_class.new(result_dir: result_dir, connection: connection) } + context 'choosing uniform batches to run' do + subject { described_class.new(result_dir: result_dir, connection: connection) } - describe '#uniform_fractions' do - it 'generates evenly distributed sequences of fractions' do - received = subject.uniform_fractions.take(9) - expected = [0, 1, 1.0 / 2, 1.0 / 4, 3.0 / 4, 1.0 / 8, 3.0 / 8, 5.0 / 8, 7.0 / 8] + describe '#uniform_fractions' do + it 'generates evenly distributed sequences of fractions' do + received = subject.uniform_fractions.take(9) + expected = [0, 1, 1.0 / 2, 1.0 / 4, 3.0 / 4, 1.0 / 8, 3.0 / 8, 5.0 / 8, 7.0 / 8] - # All the fraction numerators are small integers, and all denominators are powers of 2, so these - # fit perfectly into floating point numbers with zero loss of precision - expect(received).to eq(expected) + # All the fraction numerators are small integers, and all denominators are powers of 2, so these + # fit perfectly into floating point numbers with zero loss of precision + expect(received).to eq(expected) + end end end end diff --git a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb index 8a35d8149ad..b39b273bba9 100644 --- a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb +++ b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb @@ -53,15 +53,16 @@ RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns do describe '#execute' do it 'returns a list of class names and columns pairs' do travel_to(REMOVE_DATE) do - expect(subject.execute).to eq([ - ['Testing::A', { - 'unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0'), - 'also_unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-02-01'), '12.1') - }], - ['Testing::B', { - 'other' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0') - }] - ]) + expect(subject.execute).to eq( + [ + ['Testing::A', { + 'unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0'), + 'also_unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-02-01'), '12.1') + }], + ['Testing::B', { + 'other' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0') + }] + ]) end end end diff --git a/spec/lib/gitlab/database/partitioning/convert_table_to_first_list_partition_spec.rb b/spec/lib/gitlab/database/partitioning/convert_table_to_first_list_partition_spec.rb index af7d751a404..0e804b4feac 100644 --- a/spec/lib/gitlab/database/partitioning/convert_table_to_first_list_partition_spec.rb +++ b/spec/lib/gitlab/database/partitioning/convert_table_to_first_list_partition_spec.rb @@ -153,6 +153,21 @@ RSpec.describe Gitlab::Database::Partitioning::ConvertTableToFirstListPartition expect(parent_model.pluck(:id)).to match_array([1, 2, 3]) end + context 'when the existing table is owned by a different user' do + before do + connection.execute(<<~SQL) + CREATE USER other_user SUPERUSER; + ALTER TABLE #{table_name} OWNER TO other_user; + SQL + end + + let(:current_user) { model.connection.select_value('select current_user') } + + it 'partitions without error' do + expect { partition }.not_to raise_error + end + end + context 'when an error occurs during the conversion' do def fail_first_time # We can't directly use a boolean here, as we need something that will be passed by-reference to the proc diff --git a/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb b/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb index 67d80d71e2a..50115a6f3dd 100644 --- a/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb +++ b/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb @@ -29,10 +29,11 @@ RSpec.describe Gitlab::Database::Partitioning::MonthlyStrategy do end it 'detects both partitions' do - expect(subject).to eq([ - Gitlab::Database::Partitioning::TimePartition.new(table_name, nil, '2020-05-01', partition_name: '_test_partitioned_test_000000'), - Gitlab::Database::Partitioning::TimePartition.new(table_name, '2020-05-01', '2020-06-01', partition_name: '_test_partitioned_test_202005') - ]) + expect(subject).to eq( + [ + Gitlab::Database::Partitioning::TimePartition.new(table_name, nil, '2020-05-01', partition_name: '_test_partitioned_test_000000'), + Gitlab::Database::Partitioning::TimePartition.new(table_name, '2020-05-01', '2020-06-01', partition_name: '_test_partitioned_test_202005') + ]) end end diff --git a/spec/lib/gitlab/database/partitioning/sliding_list_strategy_spec.rb b/spec/lib/gitlab/database/partitioning/sliding_list_strategy_spec.rb index 07c2c6606d8..550f254c4da 100644 --- a/spec/lib/gitlab/database/partitioning/sliding_list_strategy_spec.rb +++ b/spec/lib/gitlab/database/partitioning/sliding_list_strategy_spec.rb @@ -36,14 +36,15 @@ RSpec.describe Gitlab::Database::Partitioning::SlidingListStrategy do describe '#current_partitions' do it 'detects both partitions' do - expect(strategy.current_partitions).to eq([ - Gitlab::Database::Partitioning::SingleNumericListPartition.new( - table_name, 1, partition_name: '_test_partitioned_test_1' - ), - Gitlab::Database::Partitioning::SingleNumericListPartition.new( - table_name, 2, partition_name: '_test_partitioned_test_2' - ) - ]) + expect(strategy.current_partitions).to eq( + [ + Gitlab::Database::Partitioning::SingleNumericListPartition.new( + table_name, 1, partition_name: '_test_partitioned_test_1' + ), + Gitlab::Database::Partitioning::SingleNumericListPartition.new( + table_name, 2, partition_name: '_test_partitioned_test_2' + ) + ]) end end diff --git a/spec/lib/gitlab/database/partitioning/time_partition_spec.rb b/spec/lib/gitlab/database/partitioning/time_partition_spec.rb index 700202d81c5..5a17e8d20cf 100644 --- a/spec/lib/gitlab/database/partitioning/time_partition_spec.rb +++ b/spec/lib/gitlab/database/partitioning/time_partition_spec.rb @@ -156,12 +156,13 @@ RSpec.describe Gitlab::Database::Partitioning::TimePartition do described_class.new(table, '2020-03-01', '2020-04-01') ] - expect(partitions.sort).to eq([ - described_class.new(table, nil, '2020-02-01'), - described_class.new(table, '2020-02-01', '2020-03-01'), - described_class.new(table, '2020-03-01', '2020-04-01'), - described_class.new(table, '2020-04-01', '2020-05-01') - ]) + expect(partitions.sort).to eq( + [ + described_class.new(table, nil, '2020-02-01'), + described_class.new(table, '2020-02-01', '2020-03-01'), + described_class.new(table, '2020-03-01', '2020-04-01'), + described_class.new(table, '2020-04-01', '2020-05-01') + ]) end it 'returns nil for partitions of different tables' do diff --git a/spec/lib/gitlab/database/partitioning_spec.rb b/spec/lib/gitlab/database/partitioning_spec.rb index 94cdbfb2328..db5ca890155 100644 --- a/spec/lib/gitlab/database/partitioning_spec.rb +++ b/spec/lib/gitlab/database/partitioning_spec.rb @@ -130,12 +130,14 @@ RSpec.describe Gitlab::Database::Partitioning do context 'when no partitioned models are given' do it 'manages partitions for each registered model' do described_class.register_models([models.first]) - described_class.register_tables([ - { - table_name: table_names.last, - partitioned_column: :created_at, strategy: :monthly - } - ]) + described_class.register_tables( + [ + { + table_name: table_names.last, + partitioned_column: :created_at, + strategy: :monthly + } + ]) expect { described_class.sync_partitions } .to change { find_partitions(table_names.first).size }.from(0) diff --git a/spec/lib/gitlab/database/reflection_spec.rb b/spec/lib/gitlab/database/reflection_spec.rb index efc5bd1c1e1..389e93364c8 100644 --- a/spec/lib/gitlab/database/reflection_spec.rb +++ b/spec/lib/gitlab/database/reflection_spec.rb @@ -314,6 +314,12 @@ RSpec.describe Gitlab::Database::Reflection do expect(database.flavor).to eq('Azure Database for PostgreSQL - Single Server') end + it 'recognizes AlloyDB for PostgreSQL' do + stub_statements("SELECT name FROM pg_settings WHERE name LIKE 'alloydb%'") + + expect(database.flavor).to eq('AlloyDB for PostgreSQL') + end + it 'returns nil if can not recognize the flavor' do expect(database.flavor).to be_nil end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb index e222a29c6a1..ac2de43b7c6 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb @@ -98,7 +98,9 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespa it 'moves a project for a namespace' do create(:project, :repository, :legacy_storage, namespace: namespace, path: 'hello-project') - expected_path = File.join(TestEnv.repos_path, 'bye-group', 'hello-project.git') + expected_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(TestEnv.repos_path, 'bye-group', 'hello-project.git') + end subject.move_repositories(namespace, 'hello-group', 'bye-group') @@ -109,7 +111,9 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespa child_namespace = create(:group, name: 'sub-group', parent: namespace) create(:project, :repository, :legacy_storage, namespace: child_namespace, path: 'hello-project') - expected_path = File.join(TestEnv.repos_path, 'hello-group', 'renamed-sub-group', 'hello-project.git') + expected_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(TestEnv.repos_path, 'hello-group', 'renamed-sub-group', 'hello-project.git') + end subject.move_repositories(child_namespace, 'hello-group/sub-group', 'hello-group/renamed-sub-group') @@ -119,7 +123,9 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespa it 'moves a parent namespace with subdirectories' do child_namespace = create(:group, name: 'sub-group', parent: namespace) create(:project, :repository, :legacy_storage, namespace: child_namespace, path: 'hello-project') - expected_path = File.join(TestEnv.repos_path, 'renamed-group', 'sub-group', 'hello-project.git') + expected_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(TestEnv.repos_path, 'renamed-group', 'sub-group', 'hello-project.git') + end subject.move_repositories(child_namespace, 'hello-group', 'renamed-group') @@ -170,7 +176,9 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespa describe '#rename_namespace_dependencies' do it "moves the repository for a project in the namespace" do create(:project, :repository, :legacy_storage, namespace: namespace, path: "the-path-project") - expected_repo = File.join(TestEnv.repos_path, "the-path0", "the-path-project.git") + expected_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(TestEnv.repos_path, "the-path0", "the-path-project.git") + end subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0') @@ -268,7 +276,9 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespa project.create_repository subject.rename_namespace(namespace) - expected_path = File.join(TestEnv.repos_path, 'the-path', 'a-project.git') + expected_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(TestEnv.repos_path, 'the-path', 'a-project.git') + end expect(subject).to receive(:rename_namespace_dependencies) .with( diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb index 50071e3e22b..6292f0246f7 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb @@ -126,7 +126,9 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProject let(:project) { create(:project, :repository, :legacy_storage, path: 'the-path', namespace: known_parent) } it 'moves the repository for a project' do - expected_path = File.join(TestEnv.repos_path, 'known-parent', 'new-repo.git') + expected_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(TestEnv.repos_path, 'known-parent', 'new-repo.git') + end subject.move_repository(project, 'known-parent/the-path', 'known-parent/new-repo') @@ -155,7 +157,9 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProject project.create_repository subject.rename_project(project) - expected_path = File.join(TestEnv.repos_path, 'known-parent', 'the-path.git') + expected_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(TestEnv.repos_path, 'known-parent', 'the-path.git') + end expect(subject).to receive(:move_project_folders) .with( diff --git a/spec/lib/gitlab/database/similarity_score_spec.rb b/spec/lib/gitlab/database/similarity_score_spec.rb index b7b66494390..cfee70ed208 100644 --- a/spec/lib/gitlab/database/similarity_score_spec.rb +++ b/spec/lib/gitlab/database/similarity_score_spec.rb @@ -78,10 +78,11 @@ RSpec.describe Gitlab::Database::SimilarityScore do describe 'score multiplier' do let(:order_expression) do - Gitlab::Database::SimilarityScore.build_expression(search: search, rules: [ - { column: Arel.sql('path'), multiplier: 1 }, - { column: Arel.sql('name'), multiplier: 0.8 } - ]).to_sql + Gitlab::Database::SimilarityScore.build_expression(search: search, rules: + [ + { column: Arel.sql('path'), multiplier: 1 }, + { column: Arel.sql('name'), multiplier: 0.8 } + ]).to_sql end let(:search) { 'different' } @@ -93,10 +94,11 @@ RSpec.describe Gitlab::Database::SimilarityScore do describe 'annotation' do it 'annotates the generated SQL expression' do - expression = Gitlab::Database::SimilarityScore.build_expression(search: 'test', rules: [ - { column: Arel.sql('path'), multiplier: 1 }, - { column: Arel.sql('name'), multiplier: 0.8 } - ]) + expression = Gitlab::Database::SimilarityScore.build_expression(search: 'test', rules: + [ + { column: Arel.sql('path'), multiplier: 1 }, + { column: Arel.sql('name'), multiplier: 0.8 } + ]) expect(Gitlab::Database::SimilarityScore).to be_order_by_similarity(expression) end |