From 41fe97390ceddf945f3d967b8fdb3de4c66b7dea Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 18 Mar 2022 20:02:30 +0000 Subject: Add latest changes from gitlab-org/gitlab@14-9-stable-ee --- ..._background_migration_worker_shared_examples.rb | 198 +++++++++++++++++++++ .../git_garbage_collect_methods_shared_examples.rb | 70 ++++++-- .../workers/concerns/reenqueuer_shared_examples.rb | 15 +- 3 files changed, 259 insertions(+), 24 deletions(-) create mode 100644 spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb (limited to 'spec/support/shared_examples/workers') diff --git a/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb b/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb new file mode 100644 index 00000000000..d202c4e00f0 --- /dev/null +++ b/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb @@ -0,0 +1,198 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'it runs batched background migration jobs' do |tracking_database| + include ExclusiveLeaseHelpers + + describe 'defining the job attributes' do + it 'defines the data_consistency as always' do + expect(described_class.get_data_consistency).to eq(:always) + end + + it 'defines the feature_category as database' do + expect(described_class.get_feature_category).to eq(:database) + end + + it 'defines the idempotency as true' do + expect(described_class.idempotent?).to be_truthy + end + end + + describe '.tracking_database' do + it 'does not raise an error' do + expect { described_class.tracking_database }.not_to raise_error + end + + it 'overrides the method to return the tracking database' do + expect(described_class.tracking_database).to eq(tracking_database) + end + end + + describe '.lease_key' do + let(:lease_key) { described_class.name.demodulize.underscore } + + it 'does not raise an error' do + expect { described_class.lease_key }.not_to raise_error + end + + it 'returns the lease key' do + expect(described_class.lease_key).to eq(lease_key) + end + end + + describe '#perform' do + subject(:worker) { described_class.new } + + context 'when the base model does not exist' do + before do + if Gitlab::Database.has_config?(tracking_database) + skip "because the base model for #{tracking_database} exists" + end + end + + it 'does nothing' do + expect(worker).not_to receive(:active_migration) + expect(worker).not_to receive(:run_active_migration) + + expect { worker.perform }.not_to raise_error + end + + it 'logs a message indicating execution is skipped' do + expect(Sidekiq.logger).to receive(:info) do |payload| + expect(payload[:class]).to eq(described_class.name) + expect(payload[:database]).to eq(tracking_database) + expect(payload[:message]).to match(/skipping migration execution/) + end + + expect { worker.perform }.not_to raise_error + end + end + + context 'when the base model does exist' do + before do + unless Gitlab::Database.has_config?(tracking_database) + skip "because the base model for #{tracking_database} does not exist" + end + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(execute_batched_migrations_on_schedule: false) + end + + it 'does nothing' do + expect(worker).not_to receive(:active_migration) + expect(worker).not_to receive(:run_active_migration) + + worker.perform + end + end + + context 'when the feature flag is enabled' do + before do + stub_feature_flags(execute_batched_migrations_on_schedule: true) + + allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration).and_return(nil) + end + + context 'when no active migrations exist' do + it 'does nothing' do + expect(worker).not_to receive(:run_active_migration) + + worker.perform + end + end + + context 'when active migrations exist' do + let(:job_interval) { 5.minutes } + let(:lease_timeout) { 15.minutes } + let(:lease_key) { described_class.name.demodulize.underscore } + let(:migration) { build(:batched_background_migration, :active, interval: job_interval) } + let(:interval_variance) { described_class::INTERVAL_VARIANCE } + + before do + allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration) + .and_return(migration) + + allow(migration).to receive(:interval_elapsed?).with(variance: interval_variance).and_return(true) + allow(migration).to receive(:reload) + end + + context 'when the reloaded migration is no longer active' do + it 'does not run the migration' do + expect_to_obtain_exclusive_lease(lease_key, timeout: lease_timeout) + + expect(migration).to receive(:reload) + expect(migration).to receive(:active?).and_return(false) + + expect(worker).not_to receive(:run_active_migration) + + worker.perform + end + end + + context 'when the interval has not elapsed' do + it 'does not run the migration' do + expect_to_obtain_exclusive_lease(lease_key, timeout: lease_timeout) + + expect(migration).to receive(:interval_elapsed?).with(variance: interval_variance).and_return(false) + + expect(worker).not_to receive(:run_active_migration) + + worker.perform + end + end + + context 'when the reloaded migration is still active and the interval has elapsed' do + it 'runs the migration' do + expect_to_obtain_exclusive_lease(lease_key, timeout: lease_timeout) + + expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |instance| + expect(instance).to receive(:run_migration_job).with(migration) + end + + expect(worker).to receive(:run_active_migration).and_call_original + + worker.perform + end + end + + context 'when the calculated timeout is less than the minimum allowed' do + let(:minimum_timeout) { described_class::MINIMUM_LEASE_TIMEOUT } + let(:job_interval) { 2.minutes } + + it 'sets the lease timeout to the minimum value' do + expect_to_obtain_exclusive_lease(lease_key, timeout: minimum_timeout) + + expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |instance| + expect(instance).to receive(:run_migration_job).with(migration) + end + + expect(worker).to receive(:run_active_migration).and_call_original + + worker.perform + end + end + + it 'always cleans up the exclusive lease' do + lease = stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) + + expect(lease).to receive(:try_obtain).and_return(true) + + expect(worker).to receive(:run_active_migration).and_raise(RuntimeError, 'I broke') + expect(lease).to receive(:cancel) + + expect { worker.perform }.to raise_error(RuntimeError, 'I broke') + end + + it 'receives the correct connection' do + base_model = Gitlab::Database.database_base_models[tracking_database] + + expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(base_model.connection).and_yield + + worker.perform + end + end + end + end + end +end diff --git a/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb b/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb index f2314793cb4..202606c6aa6 100644 --- a/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb +++ b/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb @@ -19,14 +19,28 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true| end shared_examples 'it calls Gitaly' do - specify do - repository_service = instance_double(Gitlab::GitalyClient::RepositoryService) + let(:repository_service) { instance_double(Gitlab::GitalyClient::RepositoryService) } - expect(subject).to receive(:get_gitaly_client).with(task, repository.raw_repository).and_return(repository_service) - expect(repository_service).to receive(gitaly_task) + specify do + expect_next_instance_of(Gitlab::GitalyClient::RepositoryService, repository.raw_repository) do |instance| + expect(instance).to receive(:optimize_repository).and_call_original + end subject.perform(*params) end + + context 'when optimized_housekeeping feature is disabled' do + before do + stub_feature_flags(optimized_housekeeping: false) + end + + specify do + expect(subject).to receive(:get_gitaly_client).with(task, repository.raw_repository).and_return(repository_service) + expect(repository_service).to receive(gitaly_task) + + subject.perform(*params) + end + end end shared_examples 'it updates the resource statistics' do @@ -70,12 +84,31 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true| end it 'handles gRPC errors' do - allow_next_instance_of(Gitlab::GitalyClient::RepositoryService, repository.raw_repository) do |instance| - allow(instance).to receive(:garbage_collect).and_raise(GRPC::NotFound) + repository_service = instance_double(Gitlab::GitalyClient::RepositoryService) + + allow_next_instance_of(Projects::GitDeduplicationService) do |instance| + allow(instance).to receive(:execute) end + allow(repository.raw_repository).to receive(:gitaly_repository_client).and_return(repository_service) + allow(repository_service).to receive(:optimize_repository).and_raise(GRPC::NotFound) + expect { subject.perform(*params) }.to raise_exception(Gitlab::Git::Repository::NoRepository) end + + context 'when optimized_housekeeping feature flag is disabled' do + before do + stub_feature_flags(optimized_housekeeping: false) + end + + it 'handles gRPC errors' do + allow_next_instance_of(Gitlab::GitalyClient::RepositoryService, repository.raw_repository) do |instance| + allow(instance).to receive(:garbage_collect).and_raise(GRPC::NotFound) + end + + expect { subject.perform(*params) }.to raise_exception(Gitlab::Git::Repository::NoRepository) + end + end end context 'with different lease than the active one' do @@ -152,13 +185,8 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true| expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) end - it 'calls Gitaly' do - repository_service = instance_double(Gitlab::GitalyClient::RefService) - - expect(subject).to receive(:get_gitaly_client).with(task, repository.raw_repository).and_return(repository_service) - expect(repository_service).to receive(gitaly_task) - - subject.perform(*params) + it_behaves_like 'it calls Gitaly' do + let(:repository_service) { instance_double(Gitlab::GitalyClient::RefService) } end it 'does not update the resource statistics' do @@ -180,10 +208,26 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true| it_behaves_like 'it updates the resource statistics' if update_statistics end + context 'prune' do + before do + expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) + end + + specify do + expect_next_instance_of(Gitlab::GitalyClient::RepositoryService, repository.raw_repository) do |instance| + expect(instance).to receive(:prune_unreachable_objects).and_call_original + end + + subject.perform(resource.id, 'prune', lease_key, lease_uuid) + end + end + shared_examples 'gc tasks' do before do allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) allow(subject).to receive(:bitmaps_enabled?).and_return(bitmaps_enabled) + + stub_feature_flags(optimized_housekeeping: false) end it 'incremental repack adds a new packfile' do diff --git a/spec/support/shared_examples/workers/concerns/reenqueuer_shared_examples.rb b/spec/support/shared_examples/workers/concerns/reenqueuer_shared_examples.rb index d6e96ef37d6..d9105981b4b 100644 --- a/spec/support/shared_examples/workers/concerns/reenqueuer_shared_examples.rb +++ b/spec/support/shared_examples/workers/concerns/reenqueuer_shared_examples.rb @@ -30,18 +30,11 @@ end # `job_args` to be arguments to #perform if it takes arguments RSpec.shared_examples '#perform is rate limited to 1 call per' do |minimum_duration| before do - # Allow Timecop freeze and travel without the block form - Timecop.safe_mode = false - Timecop.freeze + freeze_time time_travel_during_perform(actual_duration) end - after do - Timecop.return - Timecop.safe_mode = true - end - let(:subject_perform) { defined?(job_args) ? subject.perform(job_args) : subject.perform } context 'when the work finishes in 0 seconds' do @@ -58,7 +51,7 @@ RSpec.shared_examples '#perform is rate limited to 1 call per' do |minimum_durat let(:actual_duration) { 0.1 * minimum_duration } it 'sleeps 90% of minimum duration' do - expect(subject).to receive(:sleep).with(a_value_within(0.01).of(0.9 * minimum_duration)) + expect(subject).to receive(:sleep).with(a_value_within(1).of(0.9 * minimum_duration)) subject_perform end @@ -68,7 +61,7 @@ RSpec.shared_examples '#perform is rate limited to 1 call per' do |minimum_durat let(:actual_duration) { 0.9 * minimum_duration } it 'sleeps 10% of minimum duration' do - expect(subject).to receive(:sleep).with(a_value_within(0.01).of(0.1 * minimum_duration)) + expect(subject).to receive(:sleep).with(a_value_within(1).of(0.1 * minimum_duration)) subject_perform end @@ -111,7 +104,7 @@ RSpec.shared_examples '#perform is rate limited to 1 call per' do |minimum_durat allow(subject).to receive(:ensure_minimum_duration) do |minimum_duration, &block| original_ensure_minimum_duration.call(minimum_duration) do # Time travel inside the block inside ensure_minimum_duration - Timecop.travel(actual_duration) if actual_duration && actual_duration > 0 + travel_to(actual_duration.from_now) if actual_duration && actual_duration > 0 end end end -- cgit v1.2.3