Commit 0e257722 authored by Zeger-Jan van de Weg's avatar Zeger-Jan van de Weg

Move GC RPCs to mandatory

Closes https://gitlab.com/gitlab-org/gitaly/issues/354
parent 1175e23b
...@@ -6,12 +6,6 @@ class GitGarbageCollectWorker ...@@ -6,12 +6,6 @@ class GitGarbageCollectWorker
# Timeout set to 24h # Timeout set to 24h
LEASE_TIMEOUT = 86400 LEASE_TIMEOUT = 86400
GITALY_MIGRATED_TASKS = {
gc: :garbage_collect,
full_repack: :repack_full,
incremental_repack: :repack_incremental
}.freeze
def perform(project_id, task = :gc, lease_key = nil, lease_uuid = nil) def perform(project_id, task = :gc, lease_key = nil, lease_uuid = nil)
project = Project.find(project_id) project = Project.find(project_id)
active_uuid = get_lease_uuid(lease_key) active_uuid = get_lease_uuid(lease_key)
...@@ -27,21 +21,7 @@ class GitGarbageCollectWorker ...@@ -27,21 +21,7 @@ class GitGarbageCollectWorker
end end
task = task.to_sym task = task.to_sym
cmd = command(task) gitaly_call(task, project.repository.raw_repository)
gitaly_migrate(GITALY_MIGRATED_TASKS[task], status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
if is_enabled
gitaly_call(task, project.repository.raw_repository)
else
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?
end
end
# Refresh the branch cache in case garbage collection caused a ref lookup to fail # Refresh the branch cache in case garbage collection caused a ref lookup to fail
flush_ref_caches(project) if task == :gc flush_ref_caches(project) if task == :gc
...@@ -82,21 +62,12 @@ class GitGarbageCollectWorker ...@@ -82,21 +62,12 @@ class GitGarbageCollectWorker
when :incremental_repack when :incremental_repack
client.repack_incremental client.repack_incremental
end end
end rescue GRPC::NotFound => e
Gitlab::GitLogger.error("#{method} failed:\nRepository not found")
def command(task) raise Gitlab::Git::Repository::NoRepository.new(e)
case task rescue GRPC::BadStatus => e
when :gc Gitlab::GitLogger.error("#{method} failed:\n#{e}")
git(write_bitmaps: bitmaps_enabled?) + %w[gc] raise Gitlab::Git::CommandError.new(e)
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 end
def flush_ref_caches(project) def flush_ref_caches(project)
...@@ -108,19 +79,4 @@ class GitGarbageCollectWorker ...@@ -108,19 +79,4 @@ class GitGarbageCollectWorker
def bitmaps_enabled? def bitmaps_enabled?
Gitlab::CurrentSettings.housekeeping_bitmaps_enabled Gitlab::CurrentSettings.housekeeping_bitmaps_enabled
end end
def git(write_bitmaps:)
config_value = write_bitmaps ? 'true' : 'false'
%W[git -c repack.writeBitmaps=#{config_value}]
end
def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block)
Gitlab::GitalyClient.migrate(method, status: status, &block)
rescue GRPC::NotFound => e
Gitlab::GitLogger.error("#{method} failed:\nRepository not found")
raise Gitlab::Git::Repository::NoRepository.new(e)
rescue GRPC::BadStatus => e
Gitlab::GitLogger.error("#{method} failed:\n#{e}")
raise Gitlab::Git::CommandError.new(e)
end
end end
...@@ -11,36 +11,63 @@ describe GitGarbageCollectWorker do ...@@ -11,36 +11,63 @@ describe GitGarbageCollectWorker do
subject { described_class.new } subject { described_class.new }
describe "#perform" do describe "#perform" do
shared_examples 'flushing ref caches' do |gitaly| context 'with active lease_uuid' do
context 'with active lease_uuid' do before do
allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
end
it "flushes ref caches when the task if 'gc'" do
expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original
expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
.and_return(nil)
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original
expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
subject.perform(project.id, :gc, lease_key, lease_uuid)
end
end
context 'with different lease than the active one' do
before do
allow(subject).to receive(:get_lease_uuid).and_return(SecureRandom.uuid)
end
it 'returns silently' do
expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original
expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
subject.perform(project.id, :gc, lease_key, lease_uuid)
end
end
context 'with no active lease' do
before do
allow(subject).to receive(:get_lease_uuid).and_return(false)
end
context 'when is able to get the lease' do
before do before do
allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid)
end end
it "flushes ref caches when the task if 'gc'" do it "flushes ref caches when the task if 'gc'" do
expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
expect(subject).to receive(:command).with(:gc).and_return([:the, :command]) .and_return(nil)
if gitaly
expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
.and_return(nil)
else
expect(Gitlab::Popen).to receive(:popen)
.with([:the, :command], project.repository.path_to_repo).and_return(["", 0])
end
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original
expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
subject.perform(project.id, :gc, lease_key, lease_uuid) subject.perform(project.id)
end end
end end
context 'with different lease than the active one' do context 'when no lease can be obtained' do
before do before do
allow(subject).to receive(:get_lease_uuid).and_return(SecureRandom.uuid) expect(subject).to receive(:try_obtain_lease).and_return(false)
end end
it 'returns silently' do it 'returns silently' do
...@@ -49,63 +76,9 @@ describe GitGarbageCollectWorker do ...@@ -49,63 +76,9 @@ describe GitGarbageCollectWorker do
expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
subject.perform(project.id, :gc, lease_key, lease_uuid) subject.perform(project.id)
end end
end end
context 'with no active lease' do
before do
allow(subject).to receive(:get_lease_uuid).and_return(false)
end
context 'when is able to get the lease' do
before do
allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid)
end
it "flushes ref caches when the task if 'gc'" do
expect(subject).to receive(:command).with(:gc).and_return([:the, :command])
if gitaly
expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
.and_return(nil)
else
expect(Gitlab::Popen).to receive(:popen)
.with([:the, :command], project.repository.path_to_repo).and_return(["", 0])
end
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original
expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
subject.perform(project.id)
end
end
context 'when no lease can be obtained' do
before do
expect(subject).to receive(:try_obtain_lease).and_return(false)
end
it 'returns silently' do
expect(subject).not_to receive(:command)
expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original
expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
subject.perform(project.id)
end
end
end
end
context "with Gitaly turned on" do
it_should_behave_like 'flushing ref caches', true
end
context "with Gitaly turned off", :disable_gitaly do
it_should_behave_like 'flushing ref caches', false
end end
context "repack_full" do context "repack_full" do
......
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