Skip to content

Allow model version to be created if a previous one was deleted

Eduardo Bonet requested to merge model_registry/fix_model_version_creation into master

What does this MR do and why?

Packages are delete async by being marked as pending_descruction, so it should be possible to have more than one package that shares (project_id, name, version, type) as long as in one of them the status is pending destruction.

Creating a model version also creates a package, so since the index didn't account for pending_destruction it creation of a model version with the same model, project and version of a previously deleted model version would fail.

Example below:

  1. Create and delete a model version

    p = Project.find_by_id(1)
    model =  Ml::CreateModelService.new(p, "gitlab_amazing_model_4").execute.payload
    Ml::CreateModelVersionService.new(model, { version: "1.0.0" }).execute
    Ml::ModelVersions::DeleteService.new(p, model.name, "1.0.0", p.creator).execute
  2. Note that Packages::Package.where(name: "gitlab_amazing_model_4", version: '1.0.0', project_id: 1) will return a single package with status 'pending destruction`

  3. Try to recreate the model version Ml::CreateModelVersionService.new(model, { version: "1.0.0" }).execute, it will fail with

    """
    ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR:  duplicate key value violates unique constraint 
    \"uniq_idx_packages_packages_on_project_id_name_version_ml_model\"
    DETAIL:  Key (project_id, name, version)=(1, gitlab_amazing_model_4, 1.0.0) already exists.
    
    from /Users/eduardobonet/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/activerecord- 
    7.0.8.1/lib/active_record/connection_adapters/postgresql_adapter.rb:768:in `exec_params'
    Caused by PG::UniqueViolation: ERROR:  duplicate key value violates unique constraint 
    "uniq_idx_packages_packages_on_project_id_name_version_ml_model"
    DETAIL:  Key (project_id, name, version)=(1, gitlab_amazing_model_4, 1.0.0) already exists.
    """
  4. Run the migrations, and run the creation again. Creation succeeds

  5. Fetch the packages again (Packages::Package.where(name: "gitlab_amazing_model_4", version: '1.0.0', project_id: 1)), it will now have two packages, one with status 'default' and another with status 'pending desctruction') image

  6. Click the invite members button.

Database

Migrations:

main: == [advisory_lock_connection] object_id: 124260, pg_backend_pid: 58426
main: == 20240425120001 RemoveUniqueIndexForMlModelPackagesOnProjectIdNameVersion: migrating
main: -- transaction_open?(nil)
main:    -> 0.0000s
main: -- view_exists?(:postgres_partitions)
main:    -> 0.0138s
main: -- indexes(:packages_packages)
main:    -> 0.0109s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0002s
main: -- remove_index(:packages_packages, {:algorithm=>:concurrently, :name=>"uniq_idx_packages_packages_on_project_id_name_version_ml_model"})
main:    -> 0.0231s
main: -- execute("RESET statement_timeout")
main:    -> 0.0005s
main: == 20240425120001 RemoveUniqueIndexForMlModelPackagesOnProjectIdNameVersion: migrated (0.0651s)

main: == [advisory_lock_connection] object_id: 124260, pg_backend_pid: 58426
ci: == [advisory_lock_connection] object_id: 124680, pg_backend_pid: 58428
ci: == 20240425120001 RemoveUniqueIndexForMlModelPackagesOnProjectIdNameVersion: migrating
ci: -- transaction_open?(nil)
ci:    -> 0.0000s
ci: -- view_exists?(:postgres_partitions)
ci:    -> 0.0006s
ci: -- indexes(:packages_packages)
ci:    -> 0.0156s
ci: -- execute("SET statement_timeout TO 0")
ci:    -> 0.0002s
ci: -- remove_index(:packages_packages, {:algorithm=>:concurrently, :name=>"uniq_idx_packages_packages_on_project_id_name_version_ml_model"})
ci:    -> 0.0081s
ci: -- execute("RESET statement_timeout")
ci:    -> 0.0003s
ci: == 20240425120001 RemoveUniqueIndexForMlModelPackagesOnProjectIdNameVersion: migrated (0.0449s)

ci: == [advisory_lock_connection] object_id: 124680, pg_backend_pid: 58428
main: == [advisory_lock_connection] object_id: 124860, pg_backend_pid: 58431
main: == 20240425120002 AddUniqueIndexForMlModelPackagesOnProjectIdNameVersion: migrating
main: -- transaction_open?(nil)
main:    -> 0.0000s
main: -- view_exists?(:postgres_partitions)
main:    -> 0.0005s
main: -- index_exists?(:packages_packages, [:project_id, :name, :version], {:name=>"uniq_idx_packages_packages_on_project_id_name_version_ml_model", :unique=>true, :where=>"package_type = 14 AND status <> 4", :algorithm=>:concurrently})
main:    -> 0.0075s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0003s
main: -- add_index(:packages_packages, [:project_id, :name, :version], {:name=>"uniq_idx_packages_packages_on_project_id_name_version_ml_model", :unique=>true, :where=>"package_type = 14 AND status <> 4", :algorithm=>:concurrently})
main:    -> 0.0121s
main: -- execute("RESET statement_timeout")
main:    -> 0.0004s
main: == 20240425120002 AddUniqueIndexForMlModelPackagesOnProjectIdNameVersion: migrated (0.0279s)

main: == [advisory_lock_connection] object_id: 124860, pg_backend_pid: 58431
ci: == [advisory_lock_connection] object_id: 124980, pg_backend_pid: 58433
ci: == 20240425120002 AddUniqueIndexForMlModelPackagesOnProjectIdNameVersion: migrating
ci: -- transaction_open?(nil)
ci:    -> 0.0000s
ci: -- view_exists?(:postgres_partitions)
ci:    -> 0.0006s
ci: -- index_exists?(:packages_packages, [:project_id, :name, :version], {:name=>"uniq_idx_packages_packages_on_project_id_name_version_ml_model", :unique=>true, :where=>"package_type = 14 AND status <> 4", :algorithm=>:concurrently})
ci:    -> 0.0102s
ci: -- execute("SET statement_timeout TO 0")
ci:    -> 0.0003s
ci: -- add_index(:packages_packages, [:project_id, :name, :version], {:name=>"uniq_idx_packages_packages_on_project_id_name_version_ml_model", :unique=>true, :where=>"package_type = 14 AND status <> 4", :algorithm=>:concurrently})
ci:    -> 0.0055s
ci: -- execute("RESET statement_timeout")
ci:    -> 0.0003s
ci: == 20240425120002 AddUniqueIndexForMlModelPackagesOnProjectIdNameVersion: migrated (0.0310s)
Edited by Eduardo Bonet

Merge request reports

Loading