From 954f87286581e3e04798a2a0b342f44c5294b712 Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <git@zjvandeweg.nl>
Date: Mon, 3 Dec 2018 14:49:58 +0100
Subject: [PATCH 1/2] 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.
---
 app/models/pool_repository.rb                 | 77 ++++++++++++++++
 app/models/project.rb                         | 41 +++++++++
 app/services/projects/fork_service.rb         |  2 +
 app/workers/all_queues.yml                    |  4 +
 app/workers/concerns/object_pool_queue.rb     | 12 +++
 app/workers/git_garbage_collect_worker.rb     |  2 +
 app/workers/object_pool/create_worker.rb      | 46 ++++++++++
 app/workers/object_pool/join_worker.rb        | 20 +++++
 .../object_pool/schedule_join_worker.rb       | 19 ++++
 .../zj-pool-repository-creation.yml           |  5 ++
 config/sidekiq_queues.yml                     |  1 +
 ...1128123704_add_state_to_pool_repository.rb | 19 ++++
 db/schema.rb                                  |  4 +
 doc/administration/geo/replication/index.md   |  1 +
 .../repository_storage_types.md               | 17 ++++
 lib/gitlab/git/object_pool.rb                 | 62 +++++++++++++
 lib/gitlab/git/repository.rb                  |  7 ++
 .../gitaly_client/object_pool_service.rb      | 45 ++++++++++
 spec/controllers/projects_controller_spec.rb  |  2 +-
 spec/factories/pool_repositories.rb           | 23 ++++-
 spec/lib/gitlab/git/object_pool_spec.rb       | 89 +++++++++++++++++++
 .../gitaly_client/object_pool_service_spec.rb | 46 ++++++++++
 spec/models/pool_repository_spec.rb           |  6 +-
 spec/models/project_spec.rb                   | 38 ++++++++
 spec/services/projects/fork_service_spec.rb   | 30 ++++++-
 .../workers/object_pool/create_worker_spec.rb | 59 ++++++++++++
 spec/workers/object_pool/join_worker_spec.rb  | 35 ++++++++
 27 files changed, 706 insertions(+), 6 deletions(-)
 create mode 100644 app/workers/concerns/object_pool_queue.rb
 create mode 100644 app/workers/object_pool/create_worker.rb
 create mode 100644 app/workers/object_pool/join_worker.rb
 create mode 100644 app/workers/object_pool/schedule_join_worker.rb
 create mode 100644 changelogs/unreleased/zj-pool-repository-creation.yml
 create mode 100644 db/migrate/20181128123704_add_state_to_pool_repository.rb
 create mode 100644 lib/gitlab/git/object_pool.rb
 create mode 100644 lib/gitlab/gitaly_client/object_pool_service.rb
 create mode 100644 spec/lib/gitlab/git/object_pool_spec.rb
 create mode 100644 spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb
 create mode 100644 spec/workers/object_pool/create_worker_spec.rb
 create mode 100644 spec/workers/object_pool/join_worker_spec.rb

diff --git a/app/models/pool_repository.rb b/app/models/pool_repository.rb
index bad0e30ceb56f7..dbde00b55840fa 100644
--- a/app/models/pool_repository.rb
+++ b/app/models/pool_repository.rb
@@ -1,12 +1,89 @@
 # frozen_string_literal: true
 
+# The PoolRepository model is the database equivalent of an ObjectPool for Gitaly
+# That is; PoolRepository is the record in the database, ObjectPool is the
+# repository on disk
 class PoolRepository < ActiveRecord::Base
   include Shardable
+  include AfterCommitQueue
+
+  has_one :source_project, class_name: 'Project'
+  validates :source_project, presence: true
 
   has_many :member_projects, class_name: 'Project'
 
   after_create :correct_disk_path
 
+  state_machine :state, initial: :none do
+    state :scheduled
+    state :ready
+    state :failed
+
+    event :schedule do
+      transition none: :scheduled
+    end
+
+    event :mark_ready do
+      transition [:scheduled, :failed] => :ready
+    end
+
+    event :mark_failed do
+      transition all => :failed
+    end
+
+    state all - [:ready] do
+      def joinable?
+        false
+      end
+    end
+
+    state :ready do
+      def joinable?
+        true
+      end
+    end
+
+    after_transition none: :scheduled do |pool, _|
+      pool.run_after_commit do
+        ::ObjectPool::CreateWorker.perform_async(pool.id)
+      end
+    end
+
+    after_transition scheduled: :ready do |pool, _|
+      pool.run_after_commit do
+        ::ObjectPool::ScheduleJoinWorker.perform_async(pool.id)
+      end
+    end
+  end
+
+  def create_object_pool
+    object_pool.create
+  end
+
+  # The members of the pool should have fetched the missing objects to their own
+  # objects directory. If the caller fails to do so, data loss might occur
+  def delete_object_pool
+    object_pool.delete
+  end
+
+  def link_repository(repository)
+    object_pool.link(repository.raw)
+  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)
+    object_pool.unlink_repository(repository.raw)
+  end
+
+  def object_pool
+    @object_pool ||= Gitlab::Git::ObjectPool.new(
+      shard.name,
+      disk_path + '.git',
+      source_project.repository.raw)
+  end
+
   private
 
   def correct_disk_path
diff --git a/app/models/project.rb b/app/models/project.rb
index 091f84bf2400cd..d571f337fac27b 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1584,6 +1584,7 @@ def after_import
     import_state.remove_jid
     update_project_counter_caches
     after_create_default_branch
+    join_pool_repository
     refresh_markdown_cache!
   end
 
@@ -1980,8 +1981,48 @@ def max_attachment_size
     Gitlab::CurrentSettings.max_attachment_size.megabytes.to_i
   end
 
+  def object_pool_params
+    return {} unless !forked? && git_objects_poolable?
+
+    {
+      repository_storage: repository_storage,
+      pool_repository:    pool_repository || create_new_pool_repository
+    }
+  end
+
+  # Git objects are only poolable when the project is or has:
+  # - Hashed storage -> The object pool will have a remote to its members, using relative paths.
+  #                     If the repository path changes we would have to update the remote.
+  # - Public         -> User will be able to fetch Git objects that might not exist
+  #                     in their own repository.
+  # - Repository     -> Else the disk path will be empty, and there's nothing to pool
+  def git_objects_poolable?
+    hashed_storage?(:repository) &&
+      public? &&
+      repository_exists? &&
+      Gitlab::CurrentSettings.hashed_storage_enabled &&
+      Feature.enabled?(:object_pools, self)
+  end
+
   private
 
+  def create_new_pool_repository
+    pool = begin
+             create_or_find_pool_repository!(shard: Shard.by_name(repository_storage), source_project: self)
+           rescue ActiveRecord::RecordNotUnique
+             retry
+           end
+
+    pool.schedule
+    pool
+  end
+
+  def join_pool_repository
+    return unless pool_repository
+
+    ObjectPool::JoinWorker.perform_async(pool_repository.id, self.id)
+  end
+
   def use_hashed_storage
     if self.new_record? && Gitlab::CurrentSettings.hashed_storage_enabled
       self.storage_version = LATEST_STORAGE_VERSION
diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb
index 8dc0e044875239..91091c4393dc1d 100644
--- a/app/services/projects/fork_service.rb
+++ b/app/services/projects/fork_service.rb
@@ -54,6 +54,8 @@ def fork_new_project
         new_params[:avatar] = @project.avatar
       end
 
+      new_params.merge!(@project.object_pool_params)
+
       new_project = CreateService.new(current_user, new_params).execute
       return new_project unless new_project.persisted?
 
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 672c77539af0ab..dfce00a10a1b57 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -85,6 +85,10 @@
 - todos_destroyer:todos_destroyer_project_private
 - todos_destroyer:todos_destroyer_private_features
 
+- object_pool:object_pool_create
+- object_pool:object_pool_schedule_join
+- object_pool:object_pool_join
+
 - default
 - mailers # ActionMailer::DeliveryJob.queue_name
 
diff --git a/app/workers/concerns/object_pool_queue.rb b/app/workers/concerns/object_pool_queue.rb
new file mode 100644
index 00000000000000..5b648df9c72e41
--- /dev/null
+++ b/app/workers/concerns/object_pool_queue.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+##
+# Concern for setting Sidekiq settings for the various ObjectPool queues
+#
+module ObjectPoolQueue
+  extend ActiveSupport::Concern
+
+  included do
+    queue_namespace :object_pool
+  end
+end
diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb
index 2d381c6fd6cd9a..d3628b23189081 100644
--- a/app/workers/git_garbage_collect_worker.rb
+++ b/app/workers/git_garbage_collect_worker.rb
@@ -28,6 +28,8 @@ def perform(project_id, task = :gc, lease_key = nil, lease_uuid = nil)
     # Refresh the branch cache in case garbage collection caused a ref lookup to fail
     flush_ref_caches(project) if task == :gc
 
+    project.repository.expire_statistics_caches
+
     # In case pack files are deleted, release libgit2 cache and open file
     # descriptors ASAP instead of waiting for Ruby garbage collection
     project.cleanup
diff --git a/app/workers/object_pool/create_worker.rb b/app/workers/object_pool/create_worker.rb
new file mode 100644
index 00000000000000..1815c4471dd6dc
--- /dev/null
+++ b/app/workers/object_pool/create_worker.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+module ObjectPool
+  class CreateWorker
+    include ApplicationWorker
+    include ObjectPoolQueue
+    include ExclusiveLeaseGuard
+
+    attr_reader :pool
+
+    def perform(pool_id)
+      @pool = PoolRepository.find_by_id(pool_id)
+      return unless pool
+
+      try_obtain_lease do
+        perform_pool_creation
+      end
+    end
+
+    private
+
+    def perform_pool_creation
+      return unless pool.failed? || pool.scheduled?
+
+      # If this is a retry and the previous execution failed, deletion will
+      # bring the pool back to a pristine state
+      pool.delete_object_pool if pool.failed?
+
+      pool.create_object_pool
+      pool.mark_ready
+    rescue => e
+      pool.mark_failed
+      raise e
+    end
+
+    def lease_key
+      "object_pool:create:#{pool.id}"
+    end
+
+    private
+
+    def lease_timeout
+      1.hour
+    end
+  end
+end
diff --git a/app/workers/object_pool/join_worker.rb b/app/workers/object_pool/join_worker.rb
new file mode 100644
index 00000000000000..07676011b2a234
--- /dev/null
+++ b/app/workers/object_pool/join_worker.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module ObjectPool
+  class JoinWorker
+    include ApplicationWorker
+    include ObjectPoolQueue
+
+    def perform(pool_id, project_id)
+      pool = PoolRepository.find_by_id(pool_id)
+      return unless pool&.joinable?
+
+      project = Project.find_by_id(project_id)
+      return unless project
+
+      pool.link_repository(project.repository)
+
+      Projects::HousekeepingService.new(project).execute
+    end
+  end
+end
diff --git a/app/workers/object_pool/schedule_join_worker.rb b/app/workers/object_pool/schedule_join_worker.rb
new file mode 100644
index 00000000000000..647a8b724351fb
--- /dev/null
+++ b/app/workers/object_pool/schedule_join_worker.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module ObjectPool
+  class ScheduleJoinWorker
+    include ApplicationWorker
+    include ObjectPoolQueue
+
+    def perform(pool_id)
+      pool = PoolRepository.find_by_id(pool_id)
+      return unless pool&.joinable?
+
+      pool.member_projects.find_each do |project|
+        next if project.forked? && !project.import_finished?
+
+        ObjectPool::JoinWorker.perform_async(pool.id, project.id)
+      end
+    end
+  end
+end
diff --git a/changelogs/unreleased/zj-pool-repository-creation.yml b/changelogs/unreleased/zj-pool-repository-creation.yml
new file mode 100644
index 00000000000000..a24b96e49243c8
--- /dev/null
+++ b/changelogs/unreleased/zj-pool-repository-creation.yml
@@ -0,0 +1,5 @@
+---
+title: Allow public forks to be deduplicated
+merge_request: 23508
+author:
+type: added
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index 5ae6ad915457b6..fb1ffcb4b26991 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -81,6 +81,7 @@
   - [delete_diff_files, 1]
   - [detect_repository_languages, 1]
   - [auto_devops, 2]
+  - [object_pool, 1]
   - [repository_cleanup, 1]
   - [delete_stored_files, 1]
 
diff --git a/db/migrate/20181128123704_add_state_to_pool_repository.rb b/db/migrate/20181128123704_add_state_to_pool_repository.rb
new file mode 100644
index 00000000000000..714232ede56400
--- /dev/null
+++ b/db/migrate/20181128123704_add_state_to_pool_repository.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class AddStateToPoolRepository < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  # Given the table is empty, and the non concurrent methods are chosen so
+  # the transactions don't have to be disabled
+  # rubocop: disable Migration/AddConcurrentForeignKey, Migration/AddIndex
+  def change
+    add_column(:pool_repositories, :state, :string, null: true)
+
+    add_column :pool_repositories, :source_project_id, :integer
+    add_index :pool_repositories, :source_project_id, unique: true
+    add_foreign_key :pool_repositories, :projects, column: :source_project_id, on_delete: :nullify
+  end
+  # rubocop: enable Migration/AddConcurrentForeignKey, Migration/AddIndex
+end
diff --git a/db/schema.rb b/db/schema.rb
index 7a73d05f632f82..39b1cb656d6e99 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -2069,8 +2069,11 @@
   create_table "pool_repositories", id: :bigserial, force: :cascade do |t|
     t.integer "shard_id", null: false
     t.string "disk_path"
+    t.string "state"
+    t.integer "source_project_id"
     t.index ["disk_path"], name: "index_pool_repositories_on_disk_path", unique: true, using: :btree
     t.index ["shard_id"], name: "index_pool_repositories_on_shard_id", using: :btree
+    t.index ["source_project_id"], name: "index_pool_repositories_on_source_project_id", unique: true, using: :btree
   end
 
   create_table "programming_languages", force: :cascade do |t|
@@ -3303,6 +3306,7 @@
   add_foreign_key "path_locks", "projects", name: "fk_5265c98f24", on_delete: :cascade
   add_foreign_key "path_locks", "users"
   add_foreign_key "personal_access_tokens", "users"
+  add_foreign_key "pool_repositories", "projects", column: "source_project_id", on_delete: :nullify
   add_foreign_key "pool_repositories", "shards", on_delete: :restrict
   add_foreign_key "project_authorizations", "projects", on_delete: :cascade
   add_foreign_key "project_authorizations", "users", on_delete: :cascade
diff --git a/doc/administration/geo/replication/index.md b/doc/administration/geo/replication/index.md
index 4f1a2702b6a3e4..671ecd90c1581d 100644
--- a/doc/administration/geo/replication/index.md
+++ b/doc/administration/geo/replication/index.md
@@ -231,6 +231,7 @@ This list of limitations only reflects the latest version of GitLab. If you are
 - The installation takes multiple manual steps that together can take about an hour depending on circumstances. We are working on improving this experience. See [gitlab-org/omnibus-gitlab#2978](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2978) for details.
 - Real-time updates of issues/merge requests (for example, via long polling) doesn't work on the **secondary** node.
 - [Selective synchronization](configuration.md#selective-synchronization) applies only to files and repositories. Other datasets are replicated to the **secondary** node in full, making it inappropriate for use as an access control mechanism.
+- Object pools for forked project deduplication work only on the **primary** node, and are duplicated on the **secondary** node.
 
 ### Limitations on replication
 
diff --git a/doc/administration/repository_storage_types.md b/doc/administration/repository_storage_types.md
index 7c68db7b621193..300bf4d5558303 100644
--- a/doc/administration/repository_storage_types.md
+++ b/doc/administration/repository_storage_types.md
@@ -93,6 +93,23 @@ need to be performed on these nodes as well. Database changes will propagate wit
 You must make sure the migration event was already processed or otherwise it may migrate
 the files back to Hashed state again.
 
+#### Hashed object pools
+
+For deduplication of public forks and their parent repository, objects are pooled
+in an object pool. These object pools are a third repository where shared objects
+are stored.
+
+```ruby
+# object pool paths
+"@pools/#{hash[0..1]}/#{hash[2..3]}/#{hash}.git"
+```
+
+The object pool feature is behind the `object_pools` feature flag, and can be
+enabled for individual projects by executing
+`Feature.enable(:object_pools, Project.find(<id>))`. Note that the project has to
+be on hashed storage, should not be a fork itself, and hashed storage should be
+enabled for all new projects.
+
 ##### Attachments
 
 To rollback single Attachment migration, rename `aa/bb/abcdef1234567890...` folder back to `namespace/project`.
diff --git a/lib/gitlab/git/object_pool.rb b/lib/gitlab/git/object_pool.rb
new file mode 100644
index 00000000000000..558699a6318375
--- /dev/null
+++ b/lib/gitlab/git/object_pool.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Git
+    class ObjectPool
+      # GL_REPOSITORY has to be passed for Gitlab::Git::Repositories, but not
+      # used for ObjectPools.
+      GL_REPOSITORY = ""
+
+      delegate :exists?, :size, to: :repository
+      delegate :delete, to: :object_pool_service
+
+      attr_reader :storage, :relative_path, :source_repository
+
+      def initialize(storage, relative_path, source_repository)
+        @storage = storage
+        @relative_path = relative_path
+        @source_repository = source_repository
+      end
+
+      def create
+        object_pool_service.create(source_repository)
+      end
+
+      def link(to_link_repo)
+        remote_name = to_link_repo.object_pool_remote_name
+        repository.set_config(
+          "remote.#{remote_name}.url" => relative_path_to(to_link_repo.relative_path),
+          "remote.#{remote_name}.tagOpt" => "--no-tags",
+          "remote.#{remote_name}.fetch" => "+refs/*:refs/remotes/#{remote_name}/*"
+        )
+
+        object_pool_service.link_repository(to_link_repo)
+      end
+
+      def gitaly_object_pool
+        Gitaly::ObjectPool.new(repository: to_gitaly_repository)
+      end
+
+      def to_gitaly_repository
+        Gitlab::GitalyClient::Util.repository(storage, relative_path, GL_REPOSITORY)
+      end
+
+      # Allows for reusing other RPCs by 'tricking' Gitaly to think its a repository
+      def repository
+        @repository ||= Gitlab::Git::Repository.new(storage, relative_path, GL_REPOSITORY)
+      end
+
+      private
+
+      def object_pool_service
+        @object_pool_service ||= Gitlab::GitalyClient::ObjectPoolService.new(self)
+      end
+
+      def relative_path_to(pool_member_path)
+        pool_path = Pathname.new("#{relative_path}#{File::SEPARATOR}")
+
+        Pathname.new(pool_member_path).relative_path_from(pool_path).to_s
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 291a326dd2dde9..b8c6d146f90abc 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -69,6 +69,13 @@ def create_hooks(repo_path, global_hooks_path)
 
       attr_reader :storage, :gl_repository, :relative_path
 
+      # This remote name has to be stable for all types of repositories that
+      # can join an object pool. If it's structure ever changes, a migration
+      # has to be performed on the object pools to update the remote names.
+      # Else the pool can't be updated anymore and is left in an inconsistent
+      # state.
+      alias_method :object_pool_remote_name, :gl_repository
+
       # This initializer method is only used on the client side (gitlab-ce).
       # Gitaly-ruby uses a different initializer.
       def initialize(storage, relative_path, gl_repository)
diff --git a/lib/gitlab/gitaly_client/object_pool_service.rb b/lib/gitlab/gitaly_client/object_pool_service.rb
new file mode 100644
index 00000000000000..272ce73ad643f3
--- /dev/null
+++ b/lib/gitlab/gitaly_client/object_pool_service.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module GitalyClient
+    class ObjectPoolService
+      attr_reader :object_pool, :storage
+
+      def initialize(object_pool)
+        @object_pool = object_pool.gitaly_object_pool
+        @storage = object_pool.storage
+      end
+
+      def create(repository)
+        request = Gitaly::CreateObjectPoolRequest.new(
+          object_pool: object_pool,
+          origin: repository.gitaly_repository)
+
+        GitalyClient.call(storage, :object_pool_service, :create_object_pool, request)
+      end
+
+      def delete
+        request = Gitaly::DeleteObjectPoolRequest.new(object_pool: object_pool)
+
+        GitalyClient.call(storage, :object_pool_service, :delete_object_pool, request)
+      end
+
+      def link_repository(repository)
+        request = Gitaly::LinkRepositoryToObjectPoolRequest.new(
+          object_pool: object_pool,
+          repository: repository.gitaly_repository
+        )
+
+        GitalyClient.call(storage, :object_pool_service, :link_repository_to_object_pool,
+                          request, timeout: GitalyClient.fast_timeout)
+      end
+
+      def unlink_repository(repository)
+        request = Gitaly::UnlinkRepositoryFromObjectPoolRequest.new(repository: repository.gitaly_repository)
+
+        GitalyClient.call(storage, :object_pool_service, :unlink_repository_from_object_pool,
+                          request, timeout: GitalyClient.fast_timeout)
+      end
+    end
+  end
+end
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index a5a42a2561f556..63feb99829f7b2 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -302,7 +302,7 @@
         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 2ed0844ed47e03..265a4643f46070 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 00000000000000..363c2aa67afff1
--- /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 00000000000000..149b7ec5bb065c
--- /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 541e78507e5804..3d3878b8c391b2 100644
--- a/spec/models/pool_repository_spec.rb
+++ b/spec/models/pool_repository_spec.rb
@@ -5,6 +5,7 @@
 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 @@
     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 3847628bff4b0c..b8104abc36fe5e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -4396,6 +4396,44 @@ def domain_variable
     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 a3d24ae312aed8..26e8d829345f5e 100644
--- a/spec/services/projects/fork_service_spec.rb
+++ b/spec/services/projects/fork_service_spec.rb
@@ -2,7 +2,8 @@
 
 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 @@
     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 00000000000000..06416489472386
--- /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 00000000000000..906bc22c8d208f
--- /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
-- 
GitLab


From a303bde3b140a38faee7f39d3139457bed5921ad Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <git@zjvandeweg.nl>
Date: Fri, 7 Dec 2018 17:26:59 +0100
Subject: [PATCH 2/2] Guard against data loss when snapshotting

As discussed in:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/23508#note_123296899

Snapshotting doesn't use git, and will miss the required shared data.
---
 app/workers/object_pool/create_worker.rb | 2 --
 ee/app/services/geo/base_sync_service.rb | 4 ++++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/workers/object_pool/create_worker.rb b/app/workers/object_pool/create_worker.rb
index 1815c4471dd6dc..135b99886dc6f1 100644
--- a/app/workers/object_pool/create_worker.rb
+++ b/app/workers/object_pool/create_worker.rb
@@ -37,8 +37,6 @@ def lease_key
       "object_pool:create:#{pool.id}"
     end
 
-    private
-
     def lease_timeout
       1.hour
     end
diff --git a/ee/app/services/geo/base_sync_service.rb b/ee/app/services/geo/base_sync_service.rb
index c442bc02d3dc5b..befbb3171c8c93 100644
--- a/ee/app/services/geo/base_sync_service.rb
+++ b/ee/app/services/geo/base_sync_service.rb
@@ -118,6 +118,10 @@ def remote_url
     # will be enqueued by the log cursor, which should resolve any problems
     # it is possible to fix.
     def fetch_snapshot
+      # Snapshots will miss the data that are shared in object pools, and snapshotting should
+      # be avoided to guard against data loss.
+      return if project.pool_repository
+
       log_info("Attempting to fetch repository via snapshot")
 
       temp_repo.create_from_snapshot(
-- 
GitLab