Replace advisory lock with unique index when uploading maven packages

Context

To fix Database race condition when uploading maven pa... (#424238 - closed), we introduced Use advisory lock to prevent concurrent inserts... (!172576 - merged).

The advisory lock is a temp fix in favour of a permeant one using a database unique index (docs reference). The unique index was introduced in Replace tmp index with unique index for Maven p... (!190992 - merged).

In this MR, we replace the usage of the advisory lock when creating new maven packages with the unique index to prevent duplicates.

We introduced the unique index in 18.1, and since 18.2 is a required stop, it should be safe to start using the index in the application code.

What does this MR do and why?

  • Remove Packages::Maven::Package#prevent_concurrent_inserts.
  • Update the rescue block in Packages::Maven::CreatePackageService to take into consideration the ActiveRecord::RecordNotUnique error that could be raised thanks to the unique index in case of a race condition.
  • Add the needed specs.

References

Screenshots or screen recordings

N/A

How to set up and validate locally

We can simulate a race condition by calling the method below in rails console. It will create only one package, despite that the script will try to create many (you should see many Packages::Maven::Package Create statements in the console).

def simulate_mvn_race_condition
  user = User.first
  project = Project.first
  params = {
    path: 'foo/bar/mypkg/1.9.5',
    file_name: 'my-app-1.9.5.jar'
  }

  wait_for_it = true
  threads = Array.new(10) do
    Thread.new do
      true while wait_for_it

      Packages::Maven::FindOrCreatePackageService.new(project, user, params).execute
    end
  end
  wait_for_it = false
  threads.map(&:join).map(&:value)
end

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #543708 (closed)

Edited by Moaz Khalifa

Merge request reports

Loading
Loading