Move pushes_since_gc to Redispushes-since-gc-redis

This moves tracking of the pushes since the last Git GC from PostgreSQL
to Redis. This reduces the number of writes on the "projects" table.
This in turn reduces the vacuuming overhead.

The lease used for incrementing the counter has been removed. This lease
was mostly put in place to prevent high database load but this isn't
needed anymore due to the counter now being stored in Redis.

Fixes gitlab-org/gitlab-ce#22125

7 files changed, 99 insertions, 33 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 195362046dc..d1ba16c7ed3 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -13,6 +13,7 @@ v 8.12.0 (unreleased)
   - Add two-factor recovery endpoint to internal API !5510
   - Pass the "Remember me" value to the U2F authentication form
   - Remove vendor prefixes for linear-gradient CSS (ClemMakesApps)
+  - Move pushes_since_gc from the database to Redis
   - Add font color contrast to external label in admin area (ClemMakesApps)
   - Change logo animation to CSS (ClemMakesApps)
   - Instructions for enabling Git packfile bitmaps !6104
diff --git a/app/models/project.rb b/app/models/project.rb
index 4017cabe9f0..f3f3ffbbd28 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1288,8 +1288,24 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def pushes_since_gc
+    Gitlab::Redis.with { |redis| redis.get(pushes_since_gc_redis_key).to_i }
+  end
+
+  def increment_pushes_since_gc
+    Gitlab::Redis.with { |redis| redis.incr(pushes_since_gc_redis_key) }
+  end
+
+  def reset_pushes_since_gc
+    Gitlab::Redis.with { |redis| redis.del(pushes_since_gc_redis_key) }
+  end
+
   private
 
+  def pushes_since_gc_redis_key
+    "projects/#{id}/pushes_since_gc"
+  end
+
   # Prevents the creation of project_feature record for every project
   def setup_project_feature
     build_project_feature unless project_feature
diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb
index 29b3981f49f..c3dfc8cfbe8 100644
--- a/app/services/projects/housekeeping_service.rb
+++ b/app/services/projects/housekeeping_service.rb
@@ -30,10 +30,8 @@ module Projects
     end
 
     def increment!
-      if Gitlab::ExclusiveLease.new("project_housekeeping:increment!:#{@project.id}", timeout: 60).try_obtain
-        Gitlab::Metrics.measure(:increment_pushes_since_gc) do
-          update_pushes_since_gc(@project.pushes_since_gc + 1)
-        end
+      Gitlab::Metrics.measure(:increment_pushes_since_gc) do
+        @project.increment_pushes_since_gc
       end
     end
 
@@ -43,14 +41,10 @@ module Projects
       GitGarbageCollectWorker.perform_async(@project.id)
     ensure
       Gitlab::Metrics.measure(:reset_pushes_since_gc) do
-        update_pushes_since_gc(0)
+        @project.reset_pushes_since_gc
       end
     end
 
-    def update_pushes_since_gc(new_value)
-      @project.update_column(:pushes_since_gc, new_value)
-    end
-
     def try_obtain_lease
       Gitlab::Metrics.measure(:obtain_housekeeping_lease) do
         lease = ::Gitlab::ExclusiveLease.new("project_housekeeping:#{@project.id}", timeout: LEASE_TIMEOUT)
diff --git a/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb b/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb
new file mode 100644
index 00000000000..c5b8c35e961
--- /dev/null
+++ b/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb
@@ -0,0 +1,19 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveProjectsPushesSinceGc < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = true
+  DOWNTIME_REASON = 'This migration removes an existing column'
+
+  disable_ddl_transaction!
+
+  def up
+    remove_column :projects, :pushes_since_gc
+  end
+
+  def down
+    add_column_with_default! :projects, :pushes_since_gc, :integer, default: 0
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5c283141084..61873e38113 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: 20160902122721) do
+ActiveRecord::Schema.define(version: 20160913162434) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -824,7 +824,6 @@ ActiveRecord::Schema.define(version: 20160902122721) do
     t.integer  "build_timeout",                      default: 3600,      null: false
     t.boolean  "pending_delete",                     default: false
     t.boolean  "public_builds",                      default: true,      null: false
-    t.integer  "pushes_since_gc",                    default: 0
     t.boolean  "last_repository_check_failed"
     t.datetime "last_repository_check_at"
     t.boolean  "container_registry_enabled"
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 4a41fafb84d..74c1d93a460 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1497,4 +1497,56 @@ describe Project, models: true do
       project.change_head(project.default_branch)
     end
   end
+
+  describe '#pushes_since_gc' do
+    let(:project) { create(:project) }
+
+    after do
+      project.reset_pushes_since_gc
+    end
+
+    context 'without any pushes' do
+      it 'returns 0' do
+        expect(project.pushes_since_gc).to eq(0)
+      end
+    end
+
+    context 'with a number of pushes' do
+      it 'returns the number of pushes' do
+        3.times { project.increment_pushes_since_gc }
+
+        expect(project.pushes_since_gc).to eq(3)
+      end
+    end
+  end
+
+  describe '#increment_pushes_since_gc' do
+    let(:project) { create(:project) }
+
+    after do
+      project.reset_pushes_since_gc
+    end
+
+    it 'increments the number of pushes since the last GC' do
+      3.times { project.increment_pushes_since_gc }
+
+      expect(project.pushes_since_gc).to eq(3)
+    end
+  end
+
+  describe '#reset_pushes_since_gc' do
+    let(:project) { create(:project) }
+
+    after do
+      project.reset_pushes_since_gc
+    end
+
+    it 'resets the number of pushes since the last GC' do
+      3.times { project.increment_pushes_since_gc }
+
+      project.reset_pushes_since_gc
+
+      expect(project.pushes_since_gc).to eq(0)
+    end
+  end
 end
diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb
index ad0d58672b3..c6160f4fa57 100644
--- a/spec/services/projects/housekeeping_service_spec.rb
+++ b/spec/services/projects/housekeeping_service_spec.rb
@@ -4,12 +4,11 @@ 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
+  after do
+    project.reset_pushes_since_gc
+  end
 
+  describe '#execute' do
     it 'enqueues a sidekiq job' do
       expect(subject).to receive(:try_obtain_lease).and_return(true)
       expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id)
@@ -32,12 +31,12 @@ describe Projects::HousekeepingService do
       it 'does not reset pushes_since_gc' do
         expect do
           expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
-        end.not_to change { project.pushes_since_gc }.from(3)
+        end.not_to change { project.pushes_since_gc }
       end
     end
   end
 
-  describe 'needed?' do
+  describe '#needed?' do
     it 'when the count is low enough' do
       expect(subject.needed?).to eq(false)
     end
@@ -48,25 +47,11 @@ describe Projects::HousekeepingService do
     end
   end
 
-  describe 'increment!' do
-    let(:lease_key) { "project_housekeeping:increment!:#{project.id}" }
-
+  describe '#increment!' do
     it 'increments the pushes_since_gc counter' do
-      lease = double(:lease, try_obtain: true)
-      expect(Gitlab::ExclusiveLease).to receive(:new).with(lease_key, anything).and_return(lease)
-
       expect do
         subject.increment!
       end.to change { project.pushes_since_gc }.from(0).to(1)
     end
-
-    it 'does not increment when no lease can be obtained' do
-      lease = double(:lease, try_obtain: false)
-      expect(Gitlab::ExclusiveLease).to receive(:new).with(lease_key, anything).and_return(lease)
-
-      expect do
-        subject.increment!
-      end.not_to change { project.pushes_since_gc }
-    end
   end
 end