diff options
| -rw-r--r-- | GITALY_SERVER_VERSION | 2 | ||||
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | Gemfile.lock | 4 | ||||
| -rw-r--r-- | app/models/pool_repository.rb | 17 | ||||
| -rw-r--r-- | app/models/project.rb | 4 | ||||
| -rw-r--r-- | app/services/projects/destroy_service.rb | 2 | ||||
| -rw-r--r-- | app/workers/all_queues.yml | 1 | ||||
| -rw-r--r-- | app/workers/object_pool/destroy_worker.rb | 16 | ||||
| -rw-r--r-- | lib/gitlab/git/object_pool.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/gitaly_client/object_pool_service.rb | 5 | ||||
| -rw-r--r-- | spec/factories/pool_repositories.rb | 4 | ||||
| -rw-r--r-- | spec/models/pool_repository_spec.rb | 21 | ||||
| -rw-r--r-- | spec/workers/object_pool/destroy_worker_spec.rb | 31 |
13 files changed, 102 insertions, 9 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index bd8bf882d06..f8e233b2733 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.7.0 +1.9.0 @@ -418,7 +418,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 1.3.0', require: 'gitaly' +gem 'gitaly-proto', '~> 1.5.0', require: 'gitaly' gem 'grpc', '~> 1.15.0' gem 'google-protobuf', '~> 3.6' diff --git a/Gemfile.lock b/Gemfile.lock index b9780d4c23f..0e5b3be7e1b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -274,7 +274,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (1.3.0) + gitaly-proto (1.5.0) grpc (~> 1.0) github-markup (1.7.0) gitlab-default_value_for (3.1.1) @@ -1007,7 +1007,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 1.3.0) + gitaly-proto (~> 1.5.0) github-markup (~> 1.7.0) gitlab-default_value_for (~> 3.1.1) gitlab-markup (~> 1.6.5) diff --git a/app/models/pool_repository.rb b/app/models/pool_repository.rb index 47da0209c2f..ad6a008dee8 100644 --- a/app/models/pool_repository.rb +++ b/app/models/pool_repository.rb @@ -18,6 +18,7 @@ class PoolRepository < ActiveRecord::Base state :scheduled state :ready state :failed + state :obsolete event :schedule do transition none: :scheduled @@ -31,6 +32,10 @@ class PoolRepository < ActiveRecord::Base transition all => :failed end + event :mark_obsolete do + transition all => :obsolete + end + state all - [:ready] do def joinable? false @@ -54,6 +59,12 @@ class PoolRepository < ActiveRecord::Base ::ObjectPool::ScheduleJoinWorker.perform_async(pool.id) end end + + after_transition any => :obsolete do |pool, _| + pool.run_after_commit do + ::ObjectPool::DestroyWorker.perform_async(pool.id) + end + end end def create_object_pool @@ -71,10 +82,10 @@ class PoolRepository < ActiveRecord::Base end # This RPC can cause data loss, as not all objects are present the local repository - # No execution path yet, will be added through: - # https://gitlab.com/gitlab-org/gitaly/issues/1415 - def delete_repository_alternate(repository) + def unlink_repository(repository) object_pool.unlink_repository(repository.raw) + + mark_obsolete unless member_projects.where.not(id: repository.project.id).exists? end def object_pool diff --git a/app/models/project.rb b/app/models/project.rb index 67262ecce85..fdb5c5d7744 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2004,6 +2004,10 @@ class Project < ActiveRecord::Base Feature.enabled?(:object_pools, self) end + def leave_pool_repository + pool_repository&.unlink_repository(repository) + end + private def create_new_pool_repository diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 210571b6b4e..336d029d330 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -137,6 +137,8 @@ module Projects raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.') end + project.leave_pool_repository + Project.transaction do log_destroy_event trash_repositories! diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index bc26b3f8ef2..f9928362290 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -88,6 +88,7 @@ - object_pool:object_pool_create - object_pool:object_pool_schedule_join - object_pool:object_pool_join +- object_pool:object_pool_destroy - default - mailers # ActionMailer::DeliveryJob.queue_name diff --git a/app/workers/object_pool/destroy_worker.rb b/app/workers/object_pool/destroy_worker.rb new file mode 100644 index 00000000000..ca00d467d9b --- /dev/null +++ b/app/workers/object_pool/destroy_worker.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module ObjectPool + class DestroyWorker + include ApplicationWorker + include ObjectPoolQueue + + def perform(pool_repository_id) + pool = PoolRepository.find_by_id(pool_repository_id) + return unless pool&.obsolete? + + pool.delete_object_pool + pool.destroy + end + end +end diff --git a/lib/gitlab/git/object_pool.rb b/lib/gitlab/git/object_pool.rb index 558699a6318..59cdcd0e7cc 100644 --- a/lib/gitlab/git/object_pool.rb +++ b/lib/gitlab/git/object_pool.rb @@ -8,7 +8,7 @@ module Gitlab GL_REPOSITORY = "" delegate :exists?, :size, to: :repository - delegate :delete, to: :object_pool_service + delegate :unlink_repository, :delete, to: :object_pool_service attr_reader :storage, :relative_path, :source_repository diff --git a/lib/gitlab/gitaly_client/object_pool_service.rb b/lib/gitlab/gitaly_client/object_pool_service.rb index 272ce73ad64..6e7ede5fd18 100644 --- a/lib/gitlab/gitaly_client/object_pool_service.rb +++ b/lib/gitlab/gitaly_client/object_pool_service.rb @@ -35,7 +35,10 @@ module Gitlab end def unlink_repository(repository) - request = Gitaly::UnlinkRepositoryFromObjectPoolRequest.new(repository: repository.gitaly_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) diff --git a/spec/factories/pool_repositories.rb b/spec/factories/pool_repositories.rb index 265a4643f46..36e54cf44b4 100644 --- a/spec/factories/pool_repositories.rb +++ b/spec/factories/pool_repositories.rb @@ -15,6 +15,10 @@ FactoryBot.define do state :failed end + trait :obsolete do + state :obsolete + end + trait :ready do state :ready diff --git a/spec/models/pool_repository_spec.rb b/spec/models/pool_repository_spec.rb index 3d3878b8c39..112d4ab56fc 100644 --- a/spec/models/pool_repository_spec.rb +++ b/spec/models/pool_repository_spec.rb @@ -23,4 +23,25 @@ describe PoolRepository do expect(pool.disk_path).to match(%r{\A@pools/\h{2}/\h{2}/\h{64}}) end end + + describe '#unlink_repository' do + let(:pool) { create(:pool_repository, :ready) } + + context 'when the last member leaves' do + it 'schedules pool removal' do + expect(::ObjectPool::DestroyWorker).to receive(:perform_async).with(pool.id).and_call_original + + pool.unlink_repository(pool.source_project.repository) + end + end + + context 'when the second member leaves' do + it 'does not schedule pool removal' do + create(:project, :repository, pool_repository: pool) + expect(::ObjectPool::DestroyWorker).not_to receive(:perform_async).with(pool.id) + + pool.unlink_repository(pool.source_project.repository) + end + end + end end diff --git a/spec/workers/object_pool/destroy_worker_spec.rb b/spec/workers/object_pool/destroy_worker_spec.rb new file mode 100644 index 00000000000..ef74f0ba87c --- /dev/null +++ b/spec/workers/object_pool/destroy_worker_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +describe ObjectPool::DestroyWorker do + describe '#perform' do + context 'when no pool is in the database' do + it "doesn't raise an error" do + expect do + described_class.new.perform(987654321) + end.not_to raise_error + end + end + + context 'when a pool is present' do + let(:pool) { create(:pool_repository, :obsolete) } + + subject { described_class.new } + + it 'requests Gitaly to remove the object pool' do + expect(Gitlab::GitalyClient).to receive(:call).with(pool.shard_name, :object_pool_service, :delete_object_pool, Object) + + subject.perform(pool.id) + end + + it 'destroys the pool' do + subject.perform(pool.id) + + expect(PoolRepository.find_by_id(pool.id)).to be_nil + end + end + end +end |