Sort vulnerability identifiers on ingestion to prevent Deadlock errors

It is possible to have multiple threads trying to ingest vulnerability
identifiers for the same project in different orders which can cause
multiple connections to wait for each other to release row locks to
proceed which introduces a deadlock error.

By sorting the identifiers by fingerprint before ingestion, we prevent
the risk of having multiple transactions to wait for each other.

Changelog: fixed
EE: true

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