diff --git a/app/workers/concerns/update_repository_storage_worker.rb b/app/workers/concerns/update_repository_storage_worker.rb index ba4998c210206076f2bac2eaabe9194a2d41e3c3..fd437ebc15864544a384be0ed629713f434f6e67 100644 --- a/app/workers/concerns/update_repository_storage_worker.rb +++ b/app/workers/concerns/update_repository_storage_worker.rb @@ -38,36 +38,32 @@ def perform(*args) container_id ||= repository_storage_move.container_id - if Feature.enabled?(:use_lock_for_update_repository_storage) - # Use exclusive lock to prevent multiple storage migrations at the same time - # - # Note: instead of using a randomly generated `uuid`, we provide a worker jid value. - # That will allow to track a worker that requested a lease. - lease_key = [self.class.name.underscore, container_id].join(':') - exclusive_lease = Gitlab::ExclusiveLease.new(lease_key, uuid: jid, timeout: LEASE_TIMEOUT) - lease = exclusive_lease.try_obtain - - if lease - begin - update_repository_storage(repository_storage_move) - ensure - exclusive_lease.cancel - end - else - # If there is an ungoing storage migration, then the current one should be marked as failed - repository_storage_move.do_fail! + # Use exclusive lock to prevent multiple storage migrations at the same time + # + # Note: instead of using a randomly generated `uuid`, we provide a worker jid value. + # That will allow to track a worker that requested a lease. + lease_key = [self.class.name.underscore, container_id].join(':') + exclusive_lease = Gitlab::ExclusiveLease.new(lease_key, uuid: jid, timeout: LEASE_TIMEOUT) + lease = exclusive_lease.try_obtain - # A special case - # Sidekiq can receive an interrupt signal during the processing. - # It kills existing workers and reschedules their jobs using the same jid. - # But it can cause a situation when the migration is only half complete (see https://gitlab.com/gitlab-org/gitlab/-/issues/429049#note_1635650597) - # - # Here we detect this case and release the lock. - uuid = Gitlab::ExclusiveLease.get_uuid(lease_key) - exclusive_lease.cancel if uuid == jid + if lease + begin + update_repository_storage(repository_storage_move) + ensure + exclusive_lease.cancel end else - update_repository_storage(repository_storage_move) + # If there is an ungoing storage migration, then the current one should be marked as failed + repository_storage_move.do_fail! + + # A special case + # Sidekiq can receive an interrupt signal during the processing. + # It kills existing workers and reschedules their jobs using the same jid. + # But it can cause a situation when the migration is only half complete (see https://gitlab.com/gitlab-org/gitlab/-/issues/429049#note_1635650597) + # + # Here we detect this case and release the lock. + uuid = Gitlab::ExclusiveLease.get_uuid(lease_key) + exclusive_lease.cancel if uuid == jid end end diff --git a/config/feature_flags/development/use_lock_for_update_repository_storage.yml b/config/feature_flags/development/use_lock_for_update_repository_storage.yml deleted file mode 100644 index 8f08208bca000b3c5d18f8d1796b128e20738e80..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/use_lock_for_update_repository_storage.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: use_lock_for_update_repository_storage -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/136169 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/431198 -milestone: '16.6' -type: development -group: group::source code -default_enabled: false diff --git a/spec/support/shared_examples/workers/update_repository_move_shared_examples.rb b/spec/support/shared_examples/workers/update_repository_move_shared_examples.rb index e5a3975c0a40f33d2f6beb62bfef7888e76b6974..cb3cd81f5ce5a1d4e407a5959da17b6871230ee1 100644 --- a/spec/support/shared_examples/workers/update_repository_move_shared_examples.rb +++ b/spec/support/shared_examples/workers/update_repository_move_shared_examples.rb @@ -96,18 +96,6 @@ expect(repository_storage_move.reload).to be_failed end end - - context 'when feature flag "use_lock_for_update_repository_storage" is disabled' do - before do - stub_feature_flags(use_lock_for_update_repository_storage: false) - end - - it 'ignores lock and calls the update repository storage service' do - expect(service).to receive(:execute) - - subject - end - end end end end @@ -172,18 +160,6 @@ expect(repository_storage_move.reload).to be_failed end end - - context 'when feature flag "use_lock_for_update_repository_storage" is disabled' do - before do - stub_feature_flags(use_lock_for_update_repository_storage: false) - end - - it 'ignores lock and calls the update repository storage service' do - expect(service).to receive(:execute) - - subject - end - end end end end