Commit b2e7a330 authored by Patrick Bajao's avatar Patrick Bajao

Add merge_request_refs_cleanup feature flag

We are experiencing performance issues with the merge request
refs cleanup functionality on production and some customers
instances. This became more visible when we started backfilling
`MergeRequest::CleanupSchedule` records for old merge requests.

Disabling the worker and scheduler for now behind a feature flag
so we can test the feature once we improve it's scheduling logic
before enabling it on other instances.
parent b842ce34
...@@ -7,6 +7,8 @@ class MergeRequestCleanupRefsWorker ...@@ -7,6 +7,8 @@ class MergeRequestCleanupRefsWorker
idempotent! idempotent!
def perform(merge_request_id) def perform(merge_request_id)
return unless Feature.enabled?(:merge_request_refs_cleanup, default_enabled: false)
merge_request = MergeRequest.find_by_id(merge_request_id) merge_request = MergeRequest.find_by_id(merge_request_id)
unless merge_request unless merge_request
......
...@@ -16,6 +16,7 @@ class ScheduleMergeRequestCleanupRefsWorker ...@@ -16,6 +16,7 @@ class ScheduleMergeRequestCleanupRefsWorker
def perform def perform
return if Gitlab::Database.read_only? return if Gitlab::Database.read_only?
return unless Feature.enabled?(:merge_request_refs_cleanup, default_enabled: false)
ids = MergeRequest::CleanupSchedule.scheduled_merge_request_ids(LIMIT).map { |id| [id] } ids = MergeRequest::CleanupSchedule.scheduled_merge_request_ids(LIMIT).map { |id| [id] }
......
---
name: merge_request_refs_cleanup
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51558
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/296874
milestone: '13.8'
type: development
group: group::code review
default_enabled: false
...@@ -17,6 +17,18 @@ RSpec.describe MergeRequestCleanupRefsWorker do ...@@ -17,6 +17,18 @@ RSpec.describe MergeRequestCleanupRefsWorker do
subject subject
end end
end end
context 'when merge_request_refs_cleanup flag is disabled' do
before do
stub_feature_flags(merge_request_refs_cleanup: false)
end
it 'does not clean up the merge request' do
expect(MergeRequests::CleanupRefsService).not_to receive(:new)
perform_multiple(1)
end
end
end end
context 'when merge request does not exist' do context 'when merge request does not exist' do
......
...@@ -20,6 +20,18 @@ RSpec.describe ScheduleMergeRequestCleanupRefsWorker do ...@@ -20,6 +20,18 @@ RSpec.describe ScheduleMergeRequestCleanupRefsWorker do
worker.perform worker.perform
end end
context 'when merge_request_refs_cleanup flag is disabled' do
before do
stub_feature_flags(merge_request_refs_cleanup: false)
end
it 'does not schedule any merge request clean ups' do
expect(MergeRequestCleanupRefsWorker).not_to receive(:bulk_perform_in)
worker.perform
end
end
include_examples 'an idempotent worker' do include_examples 'an idempotent worker' do
it 'schedules MergeRequestCleanupRefsWorker to be performed by batch' do it 'schedules MergeRequestCleanupRefsWorker to be performed by batch' do
expect(MergeRequestCleanupRefsWorker) expect(MergeRequestCleanupRefsWorker)
......
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