Merge branch 'jacobvosmaer-gitlab/gitlab-ce-git-gc-improvements' into 'master'

Use more than one kind of Git garbage collection

Replaces https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6588 by @jacobvosmaer to get the builds to pass :)

Closes #22729

See merge request !7321

5 files changed, 156 insertions, 11 deletions

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 86e808314f4..52e0256943a 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -117,6 +117,11 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :send_user_confirmation_email,
       :container_registry_token_expire_delay,
       :enabled_git_access_protocol,
+      :housekeeping_enabled,
+      :housekeeping_bitmaps_enabled,
+      :housekeeping_incremental_repack_period,
+      :housekeeping_full_repack_period,
+      :housekeeping_gc_period,
       repository_storages: [],
       restricted_visibility_levels: [],
       import_sources: [],
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 9b02a68a202..bb60cc8736c 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -85,6 +85,18 @@ class ApplicationSetting < ActiveRecord::Base
             presence: { message: 'Domain blacklist cannot be empty if Blacklist is enabled.' },
             if: :domain_blacklist_enabled?
 
+  validates :housekeeping_incremental_repack_period,
+            presence: true,
+            numericality: { only_integer: true, greater_than: 0 }
+
+  validates :housekeeping_full_repack_period,
+            presence: true,
+            numericality: { only_integer: true, greater_than: :housekeeping_incremental_repack_period }
+
+  validates :housekeeping_gc_period,
+            presence: true,
+            numericality: { only_integer: true, greater_than: :housekeeping_full_repack_period }
+
   validates_each :restricted_visibility_levels do |record, attr, value|
     unless value.nil?
       value.each do |level|
@@ -168,6 +180,11 @@ class ApplicationSetting < ActiveRecord::Base
       container_registry_token_expire_delay: 5,
       repository_storages: ['default'],
       user_default_external: false,
+      housekeeping_enabled: true,
+      housekeeping_bitmaps_enabled: true,
+      housekeeping_incremental_repack_period: 10,
+      housekeeping_full_repack_period: 50,
+      housekeeping_gc_period: 200,
     )
   end
 
diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb
index c3dfc8cfbe8..4b8946f8ee2 100644
--- a/app/services/projects/housekeeping_service.rb
+++ b/app/services/projects/housekeeping_service.rb
@@ -7,6 +7,8 @@
 #
 module Projects
   class HousekeepingService < BaseService
+    include Gitlab::CurrentSettings
+
     LEASE_TIMEOUT = 3600
 
     class LeaseTaken < StandardError
@@ -20,13 +22,14 @@ module Projects
     end
 
     def execute
-      raise LeaseTaken unless try_obtain_lease
+      lease_uuid = try_obtain_lease
+      raise LeaseTaken unless lease_uuid.present?
 
-      execute_gitlab_shell_gc
+      execute_gitlab_shell_gc(lease_uuid)
     end
 
     def needed?
-      @project.pushes_since_gc >= 10
+      pushes_since_gc > 0 && period_match? && housekeeping_enabled?
     end
 
     def increment!
@@ -37,19 +40,59 @@ module Projects
 
     private
 
-    def execute_gitlab_shell_gc
-      GitGarbageCollectWorker.perform_async(@project.id)
+    def execute_gitlab_shell_gc(lease_uuid)
+      GitGarbageCollectWorker.perform_async(@project.id, task, lease_key, lease_uuid)
     ensure
-      Gitlab::Metrics.measure(:reset_pushes_since_gc) do
-        @project.reset_pushes_since_gc
+      if pushes_since_gc >= gc_period
+        Gitlab::Metrics.measure(:reset_pushes_since_gc) do
+          @project.reset_pushes_since_gc
+        end
       end
     end
 
     def try_obtain_lease
       Gitlab::Metrics.measure(:obtain_housekeeping_lease) do
-        lease = ::Gitlab::ExclusiveLease.new("project_housekeeping:#{@project.id}", timeout: LEASE_TIMEOUT)
+        lease = ::Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT)
         lease.try_obtain
       end
     end
+
+    def lease_key
+      "project_housekeeping:#{@project.id}"
+    end
+
+    def pushes_since_gc
+      @project.pushes_since_gc
+    end
+
+    def task
+      if pushes_since_gc % gc_period == 0
+        :gc
+      elsif pushes_since_gc % full_repack_period == 0
+        :full_repack
+      else
+        :incremental_repack
+      end
+    end
+
+    def period_match?
+      [gc_period, full_repack_period, repack_period].any? { |period| pushes_since_gc % period == 0 }
+    end
+
+    def housekeeping_enabled?
+      current_application_settings.housekeeping_enabled
+    end
+
+    def gc_period
+      current_application_settings.housekeeping_gc_period
+    end
+
+    def full_repack_period
+      current_application_settings.housekeeping_full_repack_period
+    end
+
+    def repack_period
+      current_application_settings.housekeeping_incremental_repack_period
+    end
   end
 end
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 28003e5f509..450ec322f2c 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -422,5 +422,44 @@
             Enable this option to include the name of the author of the issue,
             merge request or comment in the email body instead.
 
+  %fieldset
+    %legend Automatic Git repository housekeeping
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :housekeeping_enabled do
+            = f.check_box :housekeeping_enabled
+            Enable automatic repository housekeeping (git repack, git gc)
+          .help-block
+            If you keep automatic housekeeping disabled for a long time Git
+            repository access on your GitLab server will become slower and your
+            repositories will use more disk space. We recommend to always leave
+            this enabled.
+        .checkbox
+          = f.label :housekeeping_bitmaps_enabled do
+            = f.check_box :housekeeping_bitmaps_enabled
+            Enable Git pack file bitmap creation
+          .help-block
+            Creating pack file bitmaps makes housekeeping take a little longer but
+            bitmaps should accelerate 'git clone' performance.
+    .form-group
+      = f.label :housekeeping_incremental_repack_period, 'Incremental repack period', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :housekeeping_incremental_repack_period, class: 'form-control'
+        .help-block
+          Number of Git pushes after which an incremental 'git repack' is run.
+    .form-group
+      = f.label :housekeeping_full_repack_period, 'Full repack period', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :housekeeping_full_repack_period, class: 'form-control'
+        .help-block
+          Number of Git pushes after which a full 'git repack' is run.
+    .form-group
+      = f.label :housekeeping_gc_period, 'Git GC period', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :housekeeping_gc_period, class: 'form-control'
+        .help-block
+          Number of Git pushes after which 'git gc' is run.
+
   .form-actions
     = f.submit 'Save', class: 'btn btn-save'
diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb
index 65f8093b5b0..d369b639ae9 100644
--- a/app/workers/git_garbage_collect_worker.rb
+++ b/app/workers/git_garbage_collect_worker.rb
@@ -1,17 +1,58 @@
 class GitGarbageCollectWorker
   include Sidekiq::Worker
-  include Gitlab::ShellAdapter
   include DedicatedSidekiqQueue
+  include Gitlab::CurrentSettings
 
   sidekiq_options retry: false
 
-  def perform(project_id)
+  def perform(project_id, task = :gc, lease_key = nil, lease_uuid = nil)
     project = Project.find(project_id)
+    task = task.to_sym
+
+    cmd = command(task)
+    repo_path = project.repository.path_to_repo
+    description = "'#{cmd.join(' ')}' in #{repo_path}"
+
+    Gitlab::GitLogger.info(description)
+
+    output, status = Gitlab::Popen.popen(cmd, repo_path)
+    Gitlab::GitLogger.error("#{description} failed:\n#{output}") unless status.zero?
 
-    gitlab_shell.gc(project.repository_storage_path, project.path_with_namespace)
     # Refresh the branch cache in case garbage collection caused a ref lookup to fail
+    flush_ref_caches(project) if task == :gc
+  ensure
+    Gitlab::ExclusiveLease.cancel(lease_key, lease_uuid) if lease_key.present? && lease_uuid.present?
+  end
+
+  private
+
+  def command(task)
+    case task
+    when :gc
+      git(write_bitmaps: bitmaps_enabled?) + %w[gc]
+    when :full_repack
+      git(write_bitmaps: bitmaps_enabled?) + %w[repack -A -d --pack-kept-objects]
+    when :incremental_repack
+      # Normal git repack fails when bitmaps are enabled. It is impossible to
+      # create a bitmap here anyway.
+      git(write_bitmaps: false) + %w[repack -d]
+    else
+      raise "Invalid gc task: #{task.inspect}"
+    end
+  end
+
+  def flush_ref_caches(project)
     project.repository.after_create_branch
     project.repository.branch_names
     project.repository.has_visible_content?
   end
+
+  def bitmaps_enabled?
+    current_application_settings.housekeeping_bitmaps_enabled
+  end
+
+  def git(write_bitmaps:)
+    config_value = write_bitmaps ? 'true' : 'false'
+    %W[git -c repack.writeBitmaps=#{config_value}]
+  end
 end