diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2018-12-03 14:49:58 +0100 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2018-12-07 19:18:37 +0100 |
commit | 896c0bdbfb1a83ff5a7d0a755ac249ac2a895798 (patch) | |
tree | 7f763bf296fe45d9a5bfa5c84164b2f4b06bda35 /spec | |
parent | 498e34c6a4c990ae7d90b2d09cf4e73b9f228e13 (diff) | |
download | gitlab-ce-896c0bdbfb1a83ff5a7d0a755ac249ac2a895798.tar.gz |
Allow public forks to be deduplicated
When a project is forked, the new repository used to be a deep copy of everything
stored on disk by leveraging `git clone`. This works well, and makes isolation
between repository easy. However, the clone is at the start 100% the same as the
origin repository. And in the case of the objects in the object directory, this
is almost always going to be a lot of duplication.
Object Pools are a way to create a third repository that essentially only exists
for its 'objects' subdirectory. This third repository's object directory will be
set as alternate location for objects. This means that in the case an object is
missing in the local repository, git will look in another location. This other
location is the object pool repository.
When Git performs garbage collection, it's smart enough to check the
alternate location. When objects are duplicated, it will allow git to
throw one copy away. This copy is on the local repository, where to pool
remains as is.
These pools have an origin location, which for now will always be a
repository that itself is not a fork. When the root of a fork network is
forked by a user, the fork still clones the full repository. Async, the
pool repository will be created.
Either one of these processes can be done earlier than the other. To
handle this race condition, the Join ObjectPool operation is
idempotent. Given its idempotent, we can schedule it twice, with the
same effect.
To accommodate the holding of state two migrations have been added.
1. Added a state column to the pool_repositories column. This column is
managed by the state machine, allowing for hooks on transitions.
2. pool_repositories now has a source_project_id. This column in
convenient to have for multiple reasons: it has a unique index allowing
the database to handle race conditions when creating a new record. Also,
it's nice to know who the host is. As that's a short link to the fork
networks root.
Object pools are only available for public project, which use hashed
storage and when forking from the root of the fork network. (That is,
the project being forked from itself isn't a fork)
In this commit message I use both ObjectPool and Pool repositories,
which are alike, but different from each other. ObjectPool refers to
whatever is on the disk stored and managed by Gitaly. PoolRepository is
the record in the database.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/factories/pool_repositories.rb | 23 | ||||
-rw-r--r-- | spec/lib/gitlab/git/object_pool_spec.rb | 89 | ||||
-rw-r--r-- | spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb | 46 | ||||
-rw-r--r-- | spec/models/pool_repository_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 38 | ||||
-rw-r--r-- | spec/services/projects/fork_service_spec.rb | 30 | ||||
-rw-r--r-- | spec/workers/object_pool/create_worker_spec.rb | 59 | ||||
-rw-r--r-- | spec/workers/object_pool/join_worker_spec.rb | 35 |
9 files changed, 322 insertions, 6 deletions
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 7849bec4762..576191a5788 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -279,7 +279,7 @@ describe ProjectsController do expected_query = /#{public_project.fork_network.find_forks_in(other_user.namespace).to_sql}/ expect { get(:show, namespace_id: public_project.namespace, id: public_project) } - .not_to exceed_query_limit(1).for_query(expected_query) + .not_to exceed_query_limit(2).for_query(expected_query) end end end diff --git a/spec/factories/pool_repositories.rb b/spec/factories/pool_repositories.rb index 2ed0844ed47..265a4643f46 100644 --- a/spec/factories/pool_repositories.rb +++ b/spec/factories/pool_repositories.rb @@ -1,5 +1,26 @@ FactoryBot.define do factory :pool_repository do - shard + shard { Shard.by_name("default") } + state :none + + before(:create) do |pool| + pool.source_project = create(:project, :repository) + end + + trait :scheduled do + state :scheduled + end + + trait :failed do + state :failed + end + + trait :ready do + state :ready + + after(:create) do |pool| + pool.create_object_pool + end + end end end diff --git a/spec/lib/gitlab/git/object_pool_spec.rb b/spec/lib/gitlab/git/object_pool_spec.rb new file mode 100644 index 00000000000..363c2aa67af --- /dev/null +++ b/spec/lib/gitlab/git/object_pool_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Git::ObjectPool do + let(:pool_repository) { create(:pool_repository) } + let(:source_repository) { pool_repository.source_project.repository } + + subject { pool_repository.object_pool } + + describe '#storage' do + it "equals the pool repository's shard name" do + expect(subject.storage).not_to be_nil + expect(subject.storage).to eq(pool_repository.shard_name) + end + end + + describe '#create' do + before do + subject.create + end + + context "when the pool doesn't exist yet" do + it 'creates the pool' do + expect(subject.exists?).to be(true) + end + end + + context 'when the pool already exists' do + it 'raises an FailedPrecondition' do + expect do + subject.create + end.to raise_error(GRPC::FailedPrecondition) + end + end + end + + describe '#exists?' do + context "when the object pool doesn't exist" do + it 'returns false' do + expect(subject.exists?).to be(false) + end + end + + context 'when the object pool exists' do + let(:pool) { create(:pool_repository, :ready) } + + subject { pool.object_pool } + + it 'returns true' do + expect(subject.exists?).to be(true) + end + end + end + + describe '#link' do + let!(:pool_repository) { create(:pool_repository, :ready) } + + context 'when no remotes are set' do + it 'sets a remote' do + subject.link(source_repository) + + repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Rugged::Repository.new(subject.repository.path) + end + + expect(repo.remotes.count).to be(1) + expect(repo.remotes.first.name).to eq(source_repository.object_pool_remote_name) + end + end + + context 'when the remote is already set' do + before do + subject.link(source_repository) + end + + it "doesn't raise an error" do + subject.link(source_repository) + + repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Rugged::Repository.new(subject.repository.path) + end + + expect(repo.remotes.count).to be(1) + expect(repo.remotes.first.name).to eq(source_repository.object_pool_remote_name) + end + end + end +end diff --git a/spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb b/spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb new file mode 100644 index 00000000000..149b7ec5bb0 --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::GitalyClient::ObjectPoolService do + let(:pool_repository) { create(:pool_repository) } + let(:project) { create(:project, :repository) } + let(:raw_repository) { project.repository.raw } + let(:object_pool) { pool_repository.object_pool } + + subject { described_class.new(object_pool) } + + before do + subject.create(raw_repository) + end + + describe '#create' do + it 'exists on disk' do + expect(object_pool.repository.exists?).to be(true) + end + + context 'when the pool already exists' do + it 'returns an error' do + expect do + subject.create(raw_repository) + end.to raise_error(GRPC::FailedPrecondition) + end + end + end + + describe '#delete' do + it 'removes the repository from disk' do + subject.delete + + expect(object_pool.repository.exists?).to be(false) + end + + context 'when called twice' do + it "doesn't raise an error" do + subject.delete + + expect { object_pool.delete }.not_to raise_error + end + end + end +end diff --git a/spec/models/pool_repository_spec.rb b/spec/models/pool_repository_spec.rb index 541e78507e5..3d3878b8c39 100644 --- a/spec/models/pool_repository_spec.rb +++ b/spec/models/pool_repository_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' describe PoolRepository do describe 'associations' do it { is_expected.to belong_to(:shard) } + it { is_expected.to have_one(:source_project) } it { is_expected.to have_many(:member_projects) } end @@ -12,15 +13,14 @@ describe PoolRepository do let!(:pool_repository) { create(:pool_repository) } it { is_expected.to validate_presence_of(:shard) } + it { is_expected.to validate_presence_of(:source_project) } end describe '#disk_path' do it 'sets the hashed disk_path' do pool = create(:pool_repository) - elements = File.split(pool.disk_path) - - expect(elements).to all( match(/\d{2,}/) ) + expect(pool.disk_path).to match(%r{\A@pools/\h{2}/\h{2}/\h{64}}) end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 93c83fd21fd..1c85411dc3b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -4092,6 +4092,44 @@ describe Project do end end + describe '#git_objects_poolable?' do + subject { project } + + context 'when the feature flag is turned off' do + before do + stub_feature_flags(object_pools: false) + end + + let(:project) { create(:project, :repository, :public) } + + it { is_expected.not_to be_git_objects_poolable } + end + + context 'when the feature flag is enabled' do + context 'when not using hashed storage' do + let(:project) { create(:project, :legacy_storage, :public, :repository) } + + it { is_expected.not_to be_git_objects_poolable } + end + + context 'when the project is not public' do + let(:project) { create(:project, :private) } + + it { is_expected.not_to be_git_objects_poolable } + end + + context 'when objects are poolable' do + let(:project) { create(:project, :repository, :public) } + + before do + stub_application_setting(hashed_storage_enabled: true) + end + + it { is_expected.to be_git_objects_poolable } + end + end + end + def rugged_config rugged_repo(project.repository).config end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index a3d24ae312a..26e8d829345 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' describe Projects::ForkService do include ProjectForksHelper - let(:gitlab_shell) { Gitlab::Shell.new } + include Gitlab::ShellAdapter + context 'when forking a new project' do describe 'fork by user' do before do @@ -235,6 +236,33 @@ describe Projects::ForkService do end end + context 'when forking with object pools' do + let(:fork_from_project) { create(:project, :public) } + let(:forker) { create(:user) } + + before do + stub_feature_flags(object_pools: true) + end + + context 'when no pool exists' do + it 'creates a new object pool' do + forked_project = fork_project(fork_from_project, forker) + + expect(forked_project.pool_repository).to eq(fork_from_project.pool_repository) + end + end + + context 'when a pool already exists' do + let!(:pool_repository) { create(:pool_repository, source_project: fork_from_project) } + + it 'joins the object pool' do + forked_project = fork_project(fork_from_project, forker) + + expect(forked_project.pool_repository).to eq(fork_from_project.pool_repository) + end + end + end + context 'when linking fork to an existing project' do let(:fork_from_project) { create(:project, :public) } let(:fork_to_project) { create(:project, :public) } diff --git a/spec/workers/object_pool/create_worker_spec.rb b/spec/workers/object_pool/create_worker_spec.rb new file mode 100644 index 00000000000..06416489472 --- /dev/null +++ b/spec/workers/object_pool/create_worker_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ObjectPool::CreateWorker do + let(:pool) { create(:pool_repository, :scheduled) } + + subject { described_class.new } + + describe '#perform' do + context 'when the pool creation is successful' do + it 'marks the pool as ready' do + subject.perform(pool.id) + + expect(pool.reload).to be_ready + end + end + + context 'when a the pool already exists' do + before do + pool.create_object_pool + end + + it 'cleans up the pool' do + expect do + subject.perform(pool.id) + end.to raise_error(GRPC::FailedPrecondition) + + expect(pool.reload.failed?).to be(true) + end + end + + context 'when the server raises an unknown error' do + before do + allow_any_instance_of(PoolRepository).to receive(:create_object_pool).and_raise(GRPC::Internal) + end + + it 'marks the pool as failed' do + expect do + subject.perform(pool.id) + end.to raise_error(GRPC::Internal) + + expect(pool.reload.failed?).to be(true) + end + end + + context 'when the pool creation failed before' do + let(:pool) { create(:pool_repository, :failed) } + + it 'deletes the pool first' do + expect_any_instance_of(PoolRepository).to receive(:delete_object_pool) + + subject.perform(pool.id) + + expect(pool.reload).to be_ready + end + end + end +end diff --git a/spec/workers/object_pool/join_worker_spec.rb b/spec/workers/object_pool/join_worker_spec.rb new file mode 100644 index 00000000000..906bc22c8d2 --- /dev/null +++ b/spec/workers/object_pool/join_worker_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ObjectPool::JoinWorker do + let(:pool) { create(:pool_repository, :ready) } + let(:project) { pool.source_project } + let(:repository) { project.repository } + + subject { described_class.new } + + describe '#perform' do + context "when the pool is not joinable" do + let(:pool) { create(:pool_repository, :scheduled) } + + it "doesn't raise an error" do + expect do + subject.perform(pool.id, project.id) + end.not_to raise_error + end + end + + context 'when the pool has been joined before' do + before do + pool.link_repository(repository) + end + + it 'succeeds in joining' do + expect do + subject.perform(pool.id, project.id) + end.not_to raise_error + end + end + end +end |