From d4c6a3af78c0ef8257c453980832c4d419aabb51 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 31 Mar 2019 06:38:23 -0700 Subject: [PATCH] Force a full GC after importing a project During a project import, it's possible that new branches are created by the importer to handle pull requests that have been created from forked projects, which would increment the `pushes_since_gc` value via `HousekeepingService.increment!` before a full garbage collection gets to run. This causes HousekeepingService to skip the full `git gc` and move to the incremental repack mode. To ensure that a garbage collection is run to pack refs and objects, explicitly execute the task. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/59477 --- app/services/projects/after_import_service.rb | 2 +- app/services/projects/housekeeping_service.rb | 5 ++++- changelogs/unreleased/sh-force-gc-after-import.yml | 5 +++++ spec/services/projects/after_import_service_spec.rb | 2 +- spec/services/projects/housekeeping_service_spec.rb | 13 +++++++++++++ 5 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/sh-force-gc-after-import.yml diff --git a/app/services/projects/after_import_service.rb b/app/services/projects/after_import_service.rb index bbdde4408d2..afb9048e87b 100644 --- a/app/services/projects/after_import_service.rb +++ b/app/services/projects/after_import_service.rb @@ -9,7 +9,7 @@ module Projects end def execute - Projects::HousekeepingService.new(@project).execute do + Projects::HousekeepingService.new(@project, :gc).execute do repository.delete_all_refs_except(RESERVED_REF_PREFIXES) end rescue Projects::HousekeepingService::LeaseTaken => e diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb index 2f6dc4207dd..10bd5363b51 100644 --- a/app/services/projects/housekeeping_service.rb +++ b/app/services/projects/housekeeping_service.rb @@ -18,8 +18,9 @@ module Projects end end - def initialize(project) + def initialize(project, task = nil) @project = project + @task = task end def execute @@ -69,6 +70,8 @@ module Projects end def task + return @task if @task + if pushes_since_gc % gc_period == 0 :gc elsif pushes_since_gc % full_repack_period == 0 diff --git a/changelogs/unreleased/sh-force-gc-after-import.yml b/changelogs/unreleased/sh-force-gc-after-import.yml new file mode 100644 index 00000000000..755d66c1607 --- /dev/null +++ b/changelogs/unreleased/sh-force-gc-after-import.yml @@ -0,0 +1,5 @@ +--- +title: Force a full GC after importing a project +merge_request: 26803 +author: +type: performance diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb index 4dd6c6dab86..765b4ffae8f 100644 --- a/spec/services/projects/after_import_service_spec.rb +++ b/spec/services/projects/after_import_service_spec.rb @@ -13,7 +13,7 @@ describe Projects::AfterImportService do describe '#execute' do before do allow(Projects::HousekeepingService) - .to receive(:new).with(project).and_return(housekeeping_service) + .to receive(:new).with(project, :gc).and_return(housekeeping_service) allow(housekeeping_service) .to receive(:execute).and_yield diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb index 18ecef1c0a1..12ae9105627 100644 --- a/spec/services/projects/housekeeping_service_spec.rb +++ b/spec/services/projects/housekeeping_service_spec.rb @@ -88,6 +88,19 @@ describe Projects::HousekeepingService do expect(project.pushes_since_gc).to eq(1) end end + + it 'runs the task specifically requested' do + housekeeping = described_class.new(project, :gc) + + allow(housekeeping).to receive(:try_obtain_lease).and_return(:gc_uuid) + allow(housekeeping).to receive(:lease_key).and_return(:gc_lease_key) + + expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :gc, :gc_lease_key, :gc_uuid).twice + + 2.times do + housekeeping.execute + end + end end describe '#needed?' do