Commit 35b9274f authored by Jacob Vosmaer's avatar Jacob Vosmaer Committed by Grzegorz Bizon

Stop calling UnlinkRepositoryFromObjectPool RPC

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/59777.

In earlier iterations of our implementation of Git object deduplication
we thought we would be making extensive use of Git remotes in pool
repositories in the future, and that we should manage these remotes
carefully from the start. We now expect we only care about one remote,
namely the source project. The other remotes are there only for forensic
purposes.

Before this MR we tried to also remove pool remotes when member projects
got deleted, with the UnlinkRepositoryFromObjectPool RPC. This is
fragile when there are race conditions (see
https://gitlab.com/gitlab-org/gitaly/issues/1568#note_153955926). We
have spent some time making this RPC less fragile in
https://gitlab.com/gitlab-org/gitaly/merge_requests/1151 but looking at
this problem again, I think we should just stop calling it.
parent 4b9dbec3
...@@ -81,10 +81,7 @@ class PoolRepository < ApplicationRecord ...@@ -81,10 +81,7 @@ class PoolRepository < ApplicationRecord
object_pool.link(repository.raw) object_pool.link(repository.raw)
end end
# This RPC can cause data loss, as not all objects are present the local repository def mark_obsolete_if_last(repository)
def unlink_repository(repository)
object_pool.unlink_repository(repository.raw)
if member_projects.where.not(id: repository.project.id).exists? if member_projects.where.not(id: repository.project.id).exists?
true true
else else
......
...@@ -2128,7 +2128,7 @@ class Project < ApplicationRecord ...@@ -2128,7 +2128,7 @@ class Project < ApplicationRecord
end end
def leave_pool_repository def leave_pool_repository
pool_repository&.unlink_repository(repository) && update_column(:pool_repository_id, nil) pool_repository&.mark_obsolete_if_last(repository) && update_column(:pool_repository_id, nil)
end end
def link_pool_repository def link_pool_repository
......
...@@ -8,7 +8,7 @@ module Gitlab ...@@ -8,7 +8,7 @@ module Gitlab
GL_REPOSITORY = "" GL_REPOSITORY = ""
delegate :exists?, :size, to: :repository delegate :exists?, :size, to: :repository
delegate :unlink_repository, :delete, to: :object_pool_service delegate :delete, to: :object_pool_service
attr_reader :storage, :relative_path, :source_repository, :gl_project_path attr_reader :storage, :relative_path, :source_repository, :gl_project_path
......
...@@ -33,16 +33,6 @@ module Gitlab ...@@ -33,16 +33,6 @@ module Gitlab
GitalyClient.call(storage, :object_pool_service, :link_repository_to_object_pool, GitalyClient.call(storage, :object_pool_service, :link_repository_to_object_pool,
request, timeout: GitalyClient.fast_timeout) request, timeout: GitalyClient.fast_timeout)
end end
def unlink_repository(repository)
request = Gitaly::UnlinkRepositoryFromObjectPoolRequest.new(
object_pool: object_pool,
repository: repository.gitaly_repository
)
GitalyClient.call(storage, :object_pool_service, :unlink_repository_from_object_pool,
request, timeout: GitalyClient.fast_timeout)
end
end end
end end
end end
...@@ -24,14 +24,14 @@ describe PoolRepository do ...@@ -24,14 +24,14 @@ describe PoolRepository do
end end
end end
describe '#unlink_repository' do describe '#mark_obsolete_if_last' do
let(:pool) { create(:pool_repository, :ready) } let(:pool) { create(:pool_repository, :ready) }
context 'when the last member leaves' do context 'when the last member leaves' do
it 'schedules pool removal' do it 'schedules pool removal' do
expect(::ObjectPool::DestroyWorker).to receive(:perform_async).with(pool.id).and_call_original expect(::ObjectPool::DestroyWorker).to receive(:perform_async).with(pool.id).and_call_original
pool.unlink_repository(pool.source_project.repository) pool.mark_obsolete_if_last(pool.source_project.repository)
end end
end end
...@@ -40,7 +40,7 @@ describe PoolRepository do ...@@ -40,7 +40,7 @@ describe PoolRepository do
create(:project, :repository, pool_repository: pool) create(:project, :repository, pool_repository: pool)
expect(::ObjectPool::DestroyWorker).not_to receive(:perform_async).with(pool.id) expect(::ObjectPool::DestroyWorker).not_to receive(:perform_async).with(pool.id)
pool.unlink_repository(pool.source_project.repository) pool.mark_obsolete_if_last(pool.source_project.repository)
end 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