diff options
| -rw-r--r-- | app/controllers/projects_controller.rb | 13 | ||||
| -rw-r--r-- | app/services/git_push_service.rb | 9 | ||||
| -rw-r--r-- | app/services/projects/housekeeping_service.rb | 27 | ||||
| -rw-r--r-- | db/migrate/20160314143402_projects_add_pushes_since_gc.rb | 5 | ||||
| -rw-r--r-- | db/schema.rb | 3 | ||||
| -rw-r--r-- | spec/services/git_push_service_spec.rb | 39 | ||||
| -rw-r--r-- | spec/services/projects/housekeeping_service_spec.rb | 48 |
7 files changed, 139 insertions, 5 deletions
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index c70add86a20..36f37221c58 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 bd31a617747..4313de0ccab 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 0db85ac2142..bccd67d3dbf 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 00000000000..5d30a38bc99 --- /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 fa406f70907..5027d2ba32f 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 a7e2e1b1792..145bc937560 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 00000000000..93bf1b81fbe --- /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 |