Commit 12353a22 authored by James Fargher's avatar James Fargher Committed by James Fargher

Only trigger housekeeping once per push

Hoist the housekeeping trigger into ProcessRefChangesService so that it
is only triggered once per push instead of for every branch. This also
allows us to trigger housekeeping for tag only pushes.

Changelog: fixed
parent 735681f8
...@@ -26,7 +26,6 @@ module Git ...@@ -26,7 +26,6 @@ module Git
enqueue_detect_repository_languages enqueue_detect_repository_languages
execute_related_hooks execute_related_hooks
perform_housekeeping
stop_environments stop_environments
unlock_artifacts unlock_artifacts
...@@ -71,13 +70,6 @@ module Git ...@@ -71,13 +70,6 @@ module Git
BranchHooksService.new(project, current_user, params).execute BranchHooksService.new(project, current_user, params).execute
end end
def perform_housekeeping
housekeeping = Repositories::HousekeepingService.new(project)
housekeeping.increment!
housekeeping.execute if housekeeping.needed?
rescue Repositories::HousekeepingService::LeaseTaken
end
def removing_branch? def removing_branch?
Gitlab::Git.blank_ref?(newrev) Gitlab::Git.blank_ref?(newrev)
end end
......
...@@ -9,6 +9,8 @@ module Git ...@@ -9,6 +9,8 @@ module Git
process_changes_by_action(:branch, changes.branch_changes) process_changes_by_action(:branch, changes.branch_changes)
process_changes_by_action(:tag, changes.tag_changes) process_changes_by_action(:tag, changes.tag_changes)
perform_housekeeping
end end
private private
...@@ -83,5 +85,12 @@ module Git ...@@ -83,5 +85,12 @@ module Git
MergeRequests::PushedBranchesService.new(project: project, current_user: current_user, params: { changes: changes }).execute MergeRequests::PushedBranchesService.new(project: project, current_user: current_user, params: { changes: changes }).execute
end end
def perform_housekeeping
housekeeping = Repositories::HousekeepingService.new(project)
housekeeping.increment!
housekeeping.execute if housekeeping.needed?
rescue Repositories::HousekeepingService::LeaseTaken
end
end end
end end
...@@ -554,44 +554,6 @@ RSpec.describe Git::BranchPushService, services: true do ...@@ -554,44 +554,6 @@ RSpec.describe Git::BranchPushService, services: true do
end end
end end
describe "housekeeping", :clean_gitlab_redis_cache, :clean_gitlab_redis_queues, :clean_gitlab_redis_shared_state do
let(:housekeeping) { Repositories::HousekeepingService.new(project) }
before do
allow(Repositories::HousekeepingService).to receive(:new).and_return(housekeeping)
end
it 'does not perform housekeeping when not needed' do
expect(housekeeping).not_to receive(:execute)
execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
end
context 'when housekeeping is needed' do
before do
allow(housekeeping).to receive(:needed?).and_return(true)
end
it 'performs housekeeping' do
expect(housekeeping).to receive(:execute)
execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
end
it 'does not raise an exception' do
allow(housekeeping).to receive(:try_obtain_lease).and_return(false)
execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
end
end
it 'increments the push counter' do
expect(housekeeping).to receive(:increment!)
execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
end
end
describe "CI environments" do describe "CI environments" do
context 'create branch' do context 'create branch' do
let(:oldrev) { blankrev } let(:oldrev) { blankrev }
......
...@@ -161,6 +161,50 @@ RSpec.describe Git::ProcessRefChangesService do ...@@ -161,6 +161,50 @@ RSpec.describe Git::ProcessRefChangesService do
end end
end end
end end
describe "housekeeping", :clean_gitlab_redis_cache, :clean_gitlab_redis_queues, :clean_gitlab_redis_shared_state do
let(:housekeeping) { Repositories::HousekeepingService.new(project) }
before do
allow(Repositories::HousekeepingService).to receive(:new).and_return(housekeeping)
allow(push_service_class)
.to receive(:new)
.with(project, project.owner, hash_including(execute_project_hooks: true, create_push_event: true))
.exactly(changes.count).times
.and_return(service)
end
it 'does not perform housekeeping when not needed' do
expect(housekeeping).not_to receive(:execute)
subject.execute
end
context 'when housekeeping is needed' do
before do
allow(housekeeping).to receive(:needed?).and_return(true)
end
it 'performs housekeeping' do
expect(housekeeping).to receive(:execute)
subject.execute
end
it 'does not raise an exception' do
allow(housekeeping).to receive(:try_obtain_lease).and_return(false)
subject.execute
end
end
it 'increments the push counter' do
expect(housekeeping).to receive(:increment!)
subject.execute
end
end
end end
context 'branch changes' do context 'branch changes' do
......
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