Commit 4e87c023 authored by Yorick Peterse's avatar Yorick Peterse

Move pushes_since_gc to 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
parent a0c46221
...@@ -13,6 +13,7 @@ v 8.12.0 (unreleased) ...@@ -13,6 +13,7 @@ v 8.12.0 (unreleased)
- Add two-factor recovery endpoint to internal API !5510 - Add two-factor recovery endpoint to internal API !5510
- Pass the "Remember me" value to the U2F authentication form - Pass the "Remember me" value to the U2F authentication form
- Remove vendor prefixes for linear-gradient CSS (ClemMakesApps) - 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) - Add font color contrast to external label in admin area (ClemMakesApps)
- Change logo animation to CSS (ClemMakesApps) - Change logo animation to CSS (ClemMakesApps)
- Instructions for enabling Git packfile bitmaps !6104 - Instructions for enabling Git packfile bitmaps !6104
......
...@@ -1288,8 +1288,24 @@ class Project < ActiveRecord::Base ...@@ -1288,8 +1288,24 @@ class Project < ActiveRecord::Base
end end
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 private
def pushes_since_gc_redis_key
"projects/#{id}/pushes_since_gc"
end
# Prevents the creation of project_feature record for every project # Prevents the creation of project_feature record for every project
def setup_project_feature def setup_project_feature
build_project_feature unless project_feature build_project_feature unless project_feature
......
...@@ -30,10 +30,8 @@ module Projects ...@@ -30,10 +30,8 @@ module Projects
end end
def increment! def increment!
if Gitlab::ExclusiveLease.new("project_housekeeping:increment!:#{@project.id}", timeout: 60).try_obtain
Gitlab::Metrics.measure(:increment_pushes_since_gc) do Gitlab::Metrics.measure(:increment_pushes_since_gc) do
update_pushes_since_gc(@project.pushes_since_gc + 1) @project.increment_pushes_since_gc
end
end end
end end
...@@ -43,12 +41,8 @@ module Projects ...@@ -43,12 +41,8 @@ module Projects
GitGarbageCollectWorker.perform_async(@project.id) GitGarbageCollectWorker.perform_async(@project.id)
ensure ensure
Gitlab::Metrics.measure(:reset_pushes_since_gc) do Gitlab::Metrics.measure(:reset_pushes_since_gc) do
update_pushes_since_gc(0) @project.reset_pushes_since_gc
end
end end
def update_pushes_since_gc(new_value)
@project.update_column(:pushes_since_gc, new_value)
end end
def try_obtain_lease def try_obtain_lease
......
# 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
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -824,7 +824,6 @@ ActiveRecord::Schema.define(version: 20160902122721) do ...@@ -824,7 +824,6 @@ ActiveRecord::Schema.define(version: 20160902122721) do
t.integer "build_timeout", default: 3600, null: false t.integer "build_timeout", default: 3600, null: false
t.boolean "pending_delete", default: false t.boolean "pending_delete", default: false
t.boolean "public_builds", default: true, null: false t.boolean "public_builds", default: true, null: false
t.integer "pushes_since_gc", default: 0
t.boolean "last_repository_check_failed" t.boolean "last_repository_check_failed"
t.datetime "last_repository_check_at" t.datetime "last_repository_check_at"
t.boolean "container_registry_enabled" t.boolean "container_registry_enabled"
......
...@@ -1497,4 +1497,56 @@ describe Project, models: true do ...@@ -1497,4 +1497,56 @@ describe Project, models: true do
project.change_head(project.default_branch) project.change_head(project.default_branch)
end end
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 end
...@@ -4,12 +4,11 @@ describe Projects::HousekeepingService do ...@@ -4,12 +4,11 @@ describe Projects::HousekeepingService do
subject { Projects::HousekeepingService.new(project) } subject { Projects::HousekeepingService.new(project) }
let(:project) { create :project } let(:project) { create :project }
describe 'execute' do after do
before do project.reset_pushes_since_gc
project.pushes_since_gc = 3
project.save!
end end
describe '#execute' do
it 'enqueues a sidekiq job' do it 'enqueues a sidekiq job' do
expect(subject).to receive(:try_obtain_lease).and_return(true) expect(subject).to receive(:try_obtain_lease).and_return(true)
expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id) expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id)
...@@ -32,12 +31,12 @@ describe Projects::HousekeepingService do ...@@ -32,12 +31,12 @@ describe Projects::HousekeepingService do
it 'does not reset pushes_since_gc' do it 'does not reset pushes_since_gc' do
expect do expect do
expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken) 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 end
end end
describe 'needed?' do describe '#needed?' do
it 'when the count is low enough' do it 'when the count is low enough' do
expect(subject.needed?).to eq(false) expect(subject.needed?).to eq(false)
end end
...@@ -48,25 +47,11 @@ describe Projects::HousekeepingService do ...@@ -48,25 +47,11 @@ describe Projects::HousekeepingService do
end end
end end
describe 'increment!' do describe '#increment!' do
let(:lease_key) { "project_housekeeping:increment!:#{project.id}" }
it 'increments the pushes_since_gc counter' 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 expect do
subject.increment! subject.increment!
end.to change { project.pushes_since_gc }.from(0).to(1) end.to change { project.pushes_since_gc }.from(0).to(1)
end 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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment