diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index c70add86a20f926a524c5aee2484b451c2044bdb..36f37221c584f68fd5f96b1679777e24ed09cf2e 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -172,10 +172,15 @@ class ProjectsController < ApplicationController
def housekeeping
::Projects::HousekeepingService.new(@project).execute
- respond_to do |format|
- flash[:notice] = "Housekeeping successfully started."
- format.html { redirect_to project_path(@project) }
- end
+ redirect_to(
+ project_path(@project),
+ notice: "Housekeeping successfully started"
+ )
+ rescue ::Projects::HousekeepingService::LeaseTaken => ex
+ redirect_to(
+ edit_project_path(@project),
+ alert: ex.to_s
+ )
end
def toggle_star
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index bd31a617747c0c60a81cafcd5419f2c50c3cc3a2..4313de0ccab4314049051422a4270e35b3837043 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -49,6 +49,8 @@ class GitPushService < BaseService
# Update merge requests that may be affected by this push. A new branch
# could cause the last commit of a merge request to change.
update_merge_requests
+
+ perform_housekeeping
end
def update_main_language
@@ -73,6 +75,13 @@ class GitPushService < BaseService
ProjectCacheWorker.perform_async(@project.id)
end
+ def perform_housekeeping
+ housekeeping = Projects::HousekeepingService.new(@project)
+ housekeeping.increment!
+ housekeeping.execute if housekeeping.needed?
+ rescue Projects::HousekeepingService::LeaseTaken
+ end
+
def process_default_branch
@push_commits = project.repository.commits(params[:newrev])
diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb
index 0db85ac2142ed7ab3f98e9df5898443e44651b3e..bccd67d3dbf49df02c1cf15ca1ef104cd7eda8e1 100644
--- a/app/services/projects/housekeeping_service.rb
+++ b/app/services/projects/housekeeping_service.rb
@@ -9,12 +9,39 @@ module Projects
class HousekeepingService < BaseService
include Gitlab::ShellAdapter
+ LEASE_TIMEOUT = 3600
+
+ class LeaseTaken < StandardError
+ def to_s
+ "Somebody already triggered housekeeping for this project in the past #{LEASE_TIMEOUT / 60} minutes"
+ end
+ end
+
def initialize(project)
@project = project
end
def execute
+ raise LeaseTaken if !try_obtain_lease
+
GitlabShellWorker.perform_async(:gc, @project.path_with_namespace)
+ ensure
+ @project.update_column(:pushes_since_gc, 0)
+ end
+
+ def needed?
+ @project.pushes_since_gc >= 10
+ end
+
+ def increment!
+ @project.increment!(:pushes_since_gc)
+ end
+
+ private
+
+ def try_obtain_lease
+ lease = ::Gitlab::ExclusiveLease.new("project_housekeeping:#{@project.id}", timeout: LEASE_TIMEOUT)
+ lease.try_obtain
end
end
end
diff --git a/db/migrate/20160314143402_projects_add_pushes_since_gc.rb b/db/migrate/20160314143402_projects_add_pushes_since_gc.rb
new file mode 100644
index 0000000000000000000000000000000000000000..5d30a38bc99bd5aaecda032b74ee9d8eab6434b0
--- /dev/null
+++ b/db/migrate/20160314143402_projects_add_pushes_since_gc.rb
@@ -0,0 +1,5 @@
+class ProjectsAddPushesSinceGc < ActiveRecord::Migration
+ def change
+ add_column :projects, :pushes_since_gc, :integer, default: 0
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index fa406f70907a5ea1cdf047bf47fefbc7c2574913..5027d2ba32fc5f2cf2c0d8a6c4f3eee6664ed4b6 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20160309140734) do
+ActiveRecord::Schema.define(version: 20160314143402) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -720,6 +720,7 @@ ActiveRecord::Schema.define(version: 20160309140734) do
t.boolean "pending_delete", default: false
t.boolean "public_builds", default: true, null: false
t.string "main_language"
+ t.integer "pushes_since_gc", default: 0
end
add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index a7e2e1b1792014af87e0feb0b47a00cb72c1eeb2..145bc9375605763eb0c62dccc9a4fb2f43f9e8c5 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -401,6 +401,45 @@ describe GitPushService, services: true do
end
end
+ describe "housekeeping" do
+ let(:housekeeping) { Projects::HousekeepingService.new(project) }
+
+ before do
+ allow(Projects::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, @newrev, @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, @newrev, @ref)
+ end
+
+ it 'does not raise an exception' do
+ allow(housekeeping).to receive(:try_obtain_lease).and_return(false)
+
+ execute_service(project, user, @oldrev, @newrev, @ref)
+ end
+ end
+
+
+ it 'increments the push counter' do
+ expect(housekeeping).to receive(:increment!)
+
+ execute_service(project, user, @oldrev, @newrev, @ref)
+ end
+ end
+
def execute_service(project, user, oldrev, newrev, ref)
service = described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref )
service.execute
diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..93bf1b81fbe65017bea26ec11bb554cd45798dbc
--- /dev/null
+++ b/spec/services/projects/housekeeping_service_spec.rb
@@ -0,0 +1,48 @@
+require 'spec_helper'
+
+describe Projects::HousekeepingService do
+ subject { Projects::HousekeepingService.new(project) }
+ let(:project) { create :project }
+
+ describe 'execute' do
+ before do
+ project.pushes_since_gc = 3
+ project.save!
+ end
+
+ it 'enqueues a sidekiq job' do
+ expect(subject).to receive(:try_obtain_lease).and_return(true)
+ expect(GitlabShellWorker).to receive(:perform_async).with(:gc, project.path_with_namespace)
+
+ subject.execute
+ expect(project.pushes_since_gc).to eq(0)
+ end
+
+ it 'does not enqueue a job when no lease can be obtained' do
+ expect(subject).to receive(:try_obtain_lease).and_return(false)
+ expect(GitlabShellWorker).not_to receive(:perform_async)
+
+ expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
+ expect(project.pushes_since_gc).to eq(0)
+ end
+ end
+
+ describe 'needed?' do
+ it 'when the count is low enough' do
+ expect(subject.needed?).to eq(false)
+ end
+
+ it 'when the count is high enough' do
+ allow(project).to receive(:pushes_since_gc).and_return(10)
+ expect(subject.needed?).to eq(true)
+ end
+ end
+
+ describe 'increment!' do
+ it 'increments the pushes_since_gc counter' do
+ expect(project.pushes_since_gc).to eq(0)
+ subject.increment!
+ expect(project.pushes_since_gc).to eq(1)
+ end
+ end
+end