Merge branch '343332_fix_deadlock_errors' into 'master'

Sort vulnerability identifiers on ingestion to prevent Deadlock errors

See merge request gitlab-org/gitlab!79372

ee/app/services/security/ingestion/finding_map_collection.rb

@@ -35,8 +35,14 @@ module Security
         @report_findings_map ||= report_findings.index_by(&:uuid)
       end
 
+      # Sorting will make sure the findings with `overridden_uuid` values will be
+      # processed before the others.
+      # We are also sorting by `uuid` to prevent having deadlock errors while
+      # ingesting the findings.
       def deduplicated_findings
-        @deduplicated_findings ||= findings.deduplicated
+        @deduplicated_findings ||= findings.deduplicated.sort do |a, b|
+          [b.overridden_uuid.to_s, b.uuid] <=> [a.overridden_uuid.to_s, a.uuid]
+        end
       end
     end
   end

ee/app/services/security/ingestion/tasks/ingest_identifiers.rb

@@ -22,10 +22,13 @@ module Security
           end
         end
 
+        # Important Note:
+        #   Sorting identifiers is important to prevent having deadlock
+        #   errors which can happen if other threads try to import the same
+        #   identifiers in different order.
         def attributes
-          report_identifiers.map do |identifier|
-            identifier.to_hash.merge!(project_id: project.id)
-          end
+          report_identifiers.map { |identifier| identifier.to_hash.merge!(project_id: project.id) }
+                            .sort_by { |identifier_data| identifier_data[:fingerprint] }
         end
 
         def report_identifiers

ee/spec/services/security/ingestion/finding_map_collection_spec.rb

@@ -4,25 +4,30 @@ require 'spec_helper'
 
 RSpec.describe Security::Ingestion::FindingMapCollection do
   describe '#each_slice' do
-    let(:security_scan) { create(:security_scan) }
-    let(:security_findings) { create_list(:security_finding, 3, scan: security_scan, deduplicated: true) }
-    let(:report_findings) { [] }
+    let_it_be(:security_scan) { create(:security_scan) }
+    let_it_be(:security_finding_1) { create(:security_finding, overridden_uuid: '18a77231-f01d-40eb-80f0-de2ddb769a2c', uuid: '78a77231-f01d-40eb-80f0-de2ddb769a2c', scan: security_scan, deduplicated: true) }
+    let_it_be(:security_finding_2) { create(:security_finding, uuid: '88a77231-f01d-40eb-80f0-de2ddb769a2c', scan: security_scan, deduplicated: true) }
+    let_it_be(:security_finding_3) { create(:security_finding, overridden_uuid: '28a77231-f01d-40eb-80f0-de2ddb769a2c', uuid: '98a77231-f01d-40eb-80f0-de2ddb769a2c', scan: security_scan, deduplicated: true) }
+
     let(:finding_map_collection) { described_class.new(security_scan) }
     let(:finding_maps) { [] }
+    let(:report_findings) { [] }
     let(:finding_pairs) { finding_maps.map { |finding_map| [finding_map.security_finding, finding_map.report_finding] } }
     let(:test_block) { proc { |slice| finding_maps.concat(slice) } }
     let(:expected_finding_pairs) do
       [
-        [security_findings[0], report_findings[0]],
-        [security_findings[1], report_findings[1]],
-        [security_findings[2], report_findings[2]]
+        [security_finding_3, report_findings[2]],
+        [security_finding_1, report_findings[0]],
+        [security_finding_2, report_findings[1]]
       ]
     end
 
     before do
       create(:security_finding, scan: security_scan, deduplicated: false)
 
-      security_findings.each { |security_finding| report_findings << create(:ci_reports_security_finding, uuid: security_finding.uuid) }
+      report_findings << create(:ci_reports_security_finding, uuid: security_finding_1.overridden_uuid)
+      report_findings << create(:ci_reports_security_finding, uuid: security_finding_2.uuid)
+      report_findings << create(:ci_reports_security_finding, uuid: security_finding_3.overridden_uuid)
 
       allow(security_scan).to receive(:report_findings).and_return(report_findings)
       allow(finding_maps).to receive(:concat).and_call_original
@@ -35,7 +40,7 @@ RSpec.describe Security::Ingestion::FindingMapCollection do
         run_each_slice
 
         expect(finding_maps).to have_received(:concat).exactly(3).times
-        expect(finding_pairs).to match_array(expected_finding_pairs)
+        expect(finding_pairs).to eq(expected_finding_pairs)
       end
     end
   end