Merge branch 'optimize-link-merge-request-worker' into 'master'

Optimize searching cherry-picked merge requests for linking deployments

See merge request gitlab-org/gitlab!58568

app/models/merge_request.rb

@@ -288,9 +288,6 @@ class MergeRequest < ApplicationRecord
       ]
     )
   end
-  scope :by_cherry_pick_sha, -> (sha) do
-    joins(:notes).where(notes: { commit_id: sha })
-  end
   scope :join_project, -> { joins(:target_project) }
   scope :join_metrics, -> (target_project_id = nil) do
     # Do not join the relation twice

app/models/note.rb

@@ -215,6 +215,10 @@ class Note < ApplicationRecord
     def simple_sorts
       super.except('name_asc', 'name_desc')
     end
+
+    def cherry_picked_merge_requests(shas)
+      where(noteable_type: 'MergeRequest', commit_id: shas).select(:noteable_id)
+    end
   end
 
   # rubocop: disable CodeReuse/ServiceClass

app/services/deployments/link_merge_requests_service.rb

@@ -66,8 +66,15 @@ module Deployments
 
         deployment.link_merge_requests(merge_requests)
 
-        picked_merge_requests =
-          project.merge_requests.by_cherry_pick_sha(slice)
+        # The cherry picked commits are tracked via `notes.commit_id`
+        # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/22209
+        #
+        # NOTE: cross-joining `merge_requests` table and `notes` table could
+        # result in very poor performance because PG planner often uses an
+        # inappropriate index.
+        # See https://gitlab.com/gitlab-org/gitlab/-/issues/321032.
+        mr_ids = project.notes.cherry_picked_merge_requests(slice)
+        picked_merge_requests = project.merge_requests.id_in(mr_ids)
 
         deployment.link_merge_requests(picked_merge_requests)
       end

changelogs/unreleased/optimize-link-merge-request-worker.yml

+---
+title: Optimize searching cherry-picked merge requests for linking deployments
+merge_request: 58568
+author:
+type: performance

spec/models/merge_request_spec.rb

@@ -443,16 +443,6 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
-  describe '.by_cherry_pick_sha' do
-    it 'returns merge requests that match the given merge commit' do
-      note = create(:track_mr_picking_note, commit_id: '456abc')
-
-      create(:track_mr_picking_note, project: create(:project), commit_id: '456def')
-
-      expect(described_class.by_cherry_pick_sha('456abc')).to eq([note.noteable])
-    end
-  end
-
   describe '.in_projects' do
     it 'returns the merge requests for a set of projects' do
       expect(described_class.in_projects(Project.all)).to eq([subject])

spec/models/note_spec.rb

@@ -863,6 +863,16 @@ RSpec.describe Note do
     end
   end
 
+  describe '.cherry_picked_merge_requests' do
+    it 'returns merge requests that match the given merge commit' do
+      note = create(:track_mr_picking_note, commit_id: '456abc')
+
+      create(:track_mr_picking_note, project: create(:project), commit_id: '456def')
+
+      expect(MergeRequest.id_in(described_class.cherry_picked_merge_requests('456abc'))).to eq([note.noteable])
+    end
+  end
+
   describe '#for_project_snippet?' do
     it 'returns true for a project snippet note' do
       expect(build(:note_on_project_snippet).for_project_snippet?).to be true