Skip to content

Sort vulnerability identifiers on ingestion to prevent Deadlock errors

Mehmet Emin INAC requested to merge 343332_fix_deadlock_errors into master

What does this MR do and why?

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.

Related to #343332 (closed).

Related Sentry errors;

Here is a script to reproduce the deadlock case 
begin
  require 'bundler/inline'
rescue LoadError => e
  $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(false) do
  source 'https://rubygems.org'
  gem 'activerecord', '6.0.0'
  gem 'sqlite3'
  gem 'pg'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'
require 'sqlite3'
require 'pg'

Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)

# ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: 'dont use in memory database', pool: 6)
ActiveRecord::Base.establish_connection(adapter: 'postgresql', host: 'localhost', database: '...', username: '...', pool: 6)

ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :authors, force: true do |t|
    t.string :name
    t.string :uuid

    t.index :uuid, unique: true
  end
end

class Author < ActiveRecord::Base
  validates :name, presence: true
end

def create_authors
  5.times.map { |i| Author.create(name: "Author #{i}", uuid: SecureRandom.uuid) }
end

class BugTest < Minitest::Test
  def test_reproduce_deadlock
    authors = create_authors

    5.times.map do
      Thread.new do
        ActiveRecord::Base.transaction do
          new_authors = authors.shuffle.map(&:dup).as_json(except: :id)

          Author.upsert_all(new_authors, unique_by: :uuid)
        end
      end
    end.each(&:join)
  end
end

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Mehmet Emin INAC

Merge request reports