Make Project#all_lfs_objects query more performant

Add a multi-column index to `project_id` and `lfs_object_id` and
use a JOIN.

This also removes the existing `project_id` index since it'll be
redundant.

app/models/project.rb

@@ -1382,7 +1382,10 @@ class Project < ApplicationRecord
   #
   # See https://gitlab.com/gitlab-org/gitlab/issues/122002 for more info.
   def all_lfs_objects
-    LfsObject.where(id: LfsObjectsProject.select(:lfs_object_id).where(project: [self, lfs_storage_project]))
+    LfsObject
+      .distinct
+      .joins(:lfs_objects_projects)
+      .where(lfs_objects_projects: { project_id: [self, lfs_storage_project] })
   end
 
   def personal?

db/migrate/20200123040535_add_multi_column_index_on_lfs_objects_projects.rb

+# frozen_string_literal: true
+
+class AddMultiColumnIndexOnLfsObjectsProjects < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :lfs_objects_projects, [:project_id, :lfs_object_id]
+  end
+
+  def down
+    remove_concurrent_index :lfs_objects_projects, [:project_id, :lfs_object_id]
+  end
+end

db/migrate/20200123045415_remove_project_id_index_on_lfs_objects_projects.rb

+# frozen_string_literal: true
+
+class RemoveProjectIdIndexOnLfsObjectsProjects < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index :lfs_objects_projects, :project_id
+  end
+
+  def down
+    add_concurrent_index :lfs_objects_projects, :project_id
+  end
+end

db/schema.rb

@@ -2347,7 +2347,7 @@ ActiveRecord::Schema.define(version: 2020_02_06_091544) do
     t.datetime "updated_at"
     t.integer "repository_type", limit: 2
     t.index ["lfs_object_id"], name: "index_lfs_objects_projects_on_lfs_object_id"
-    t.index ["project_id"], name: "index_lfs_objects_projects_on_project_id"
+    t.index ["project_id", "lfs_object_id"], name: "index_lfs_objects_projects_on_project_id_and_lfs_object_id"
   end
 
   create_table "licenses", id: :serial, force: :cascade do |t|

spec/models/project_spec.rb

@@ -2694,16 +2694,44 @@ describe Project do
     describe '#all_lfs_objects' do
       let(:lfs_object) { create(:lfs_object) }
 
-      before do
-        project.lfs_objects << lfs_object
+      context 'when LFS object is only associated to the source' do
+        before do
+          project.lfs_objects << lfs_object
+        end
+
+        it 'returns the lfs object for a project' do
+          expect(project.all_lfs_objects).to contain_exactly(lfs_object)
+        end
+
+        it 'returns the lfs object for a fork' do
+          expect(forked_project.all_lfs_objects).to contain_exactly(lfs_object)
+        end
       end
 
-      it 'returns the lfs object for a project' do
-        expect(project.all_lfs_objects).to contain_exactly(lfs_object)
+      context 'when LFS object is only associated to the fork' do
+        before do
+          forked_project.lfs_objects << lfs_object
+        end
+
+        it 'returns nothing' do
+          expect(project.all_lfs_objects).to be_empty
+        end
+
+        it 'returns the lfs object for a fork' do
+          expect(forked_project.all_lfs_objects).to contain_exactly(lfs_object)
+        end
       end
 
-      it 'returns the lfs object for a fork' do
-        expect(forked_project.all_lfs_objects).to contain_exactly(lfs_object)
+      context 'when LFS object is associated to both source and fork' do
+        before do
+          project.lfs_objects << lfs_object
+          forked_project.lfs_objects << lfs_object
+        end
+
+        it 'returns the lfs object for the source and fork' do
+          expect(project.all_lfs_objects).to contain_exactly(lfs_object)
+          expect(forked_project.all_lfs_objects).to contain_exactly(lfs_object)
+        end
       end
     end
   end