Skip to content

Atomic internal ids for all models

What does this MR do?

This change adopts AtomicInternalId for models MergeRequest, Milestone and Deployment.

A Milestone can belong to a Project or Namespace (group). That is, the scope for InternalId is either a project or a group. To reflect that, this change adds a column namespace_id to internal_ids. Depending on the situation, we get internal ids scoped on project or group.

Once this change is merged, I'll setup a corresponding EE MR to finally remove the NonatomicInternalId concern (it's still used on the EE-side for epics).

Migration

Up:

$ spring rake db:migrate                                                                                                                                                                                                                        
Running via Spring preloader in process 27196
== 20180416155103 AddFurtherScopeColumnsToInternalIdTable: migrating ==========
-- change_column(:internal_ids, :project_id, :integer, {:null=>true})
   -> 0.0039s
-- add_column(:internal_ids, :namespace_id, :integer, {:null=>true})
   -> 0.0005s
== 20180416155103 AddFurtherScopeColumnsToInternalIdTable: migrated (0.0045s) =

== 20180417090132 AddIndexConstraintsToInternalIdTable: migrating =============
-- transaction_open?()
   -> 0.0000s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- index_exists?(:internal_ids, [:usage, :namespace_id], {:unique=>true, :where=>"namespace_id IS NOT NULL", :algorithm=>:concurrently})
   -> 0.0020s
-- add_index(:internal_ids, [:usage, :namespace_id], {:unique=>true, :where=>"namespace_id IS NOT NULL", :algorithm=>:concurrently})
   -> 0.0054s
-- index_exists?(:internal_ids, [:usage, :project_id], {:name=>"index_internal_ids_on_usage_and_project_id"})
   -> 0.0021s
-- rename_index(:internal_ids, "index_internal_ids_on_usage_and_project_id", "index_internal_ids_on_usage_and_project_id_old")
   -> 0.0013s
-- transaction_open?()
   -> 0.0000s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- index_exists?(:internal_ids, [:usage, :project_id], {:unique=>true, :where=>"project_id IS NOT NULL", :algorithm=>:concurrently})
   -> 0.0017s
-- add_index(:internal_ids, [:usage, :project_id], {:unique=>true, :where=>"project_id IS NOT NULL", :algorithm=>:concurrently})
   -> 0.0056s
-- transaction_open?()
   -> 0.0000s
-- select_one("SELECT current_setting('server_version_num') AS v")
   -> 0.0003s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- indexes(:internal_ids)
   -> 0.0021s
-- remove_index(:internal_ids, {:algorithm=>:concurrently, :name=>"index_internal_ids_on_usage_and_project_id_old"})
   -> 0.0022s
-- transaction_open?()
   -> 0.0000s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- foreign_keys(:internal_ids)
   -> 0.0028s
-- execute("ALTER TABLE internal_ids\nADD CONSTRAINT fk_162941d509\nFOREIGN KEY (namespace_id)\nREFERENCES namespaces (id)\nON DELETE CASCADE\nNOT VALID;\n")
   -> 0.0020s
-- execute("ALTER TABLE internal_ids VALIDATE CONSTRAINT fk_162941d509;")
   -> 0.0023s
== 20180417090132 AddIndexConstraintsToInternalIdTable: migrated (0.0315s) ====

Down:

$ spring rake db:rollback VERSION=20180417090132                                                                                                                                                                                                 
== 20180417090132 AddIndexConstraintsToInternalIdTable: reverting =============
-- transaction_open?()
   -> 0.0000s
-- select_one("SELECT current_setting('server_version_num') AS v")
   -> 0.0006s
-- execute("SET statement_timeout TO 0")
   -> 0.0003s
-- index_exists?(:internal_ids, [:usage, :namespace_id], {:algorithm=>:concurrently})
   -> 0.0028s
-- remove_index(:internal_ids, {:algorithm=>:concurrently, :column=>[:usage, :namespace_id]})
   -> 0.0032s
-- index_exists?(:internal_ids, [:usage, :project_id], {:name=>"index_internal_ids_on_usage_and_project_id"})
   -> 0.0011s
-- rename_index(:internal_ids, "index_internal_ids_on_usage_and_project_id", "index_internal_ids_on_usage_and_project_id_old")
   -> 0.0017s
-- transaction_open?()
   -> 0.0000s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- index_exists?(:internal_ids, [:usage, :project_id], {:unique=>true, :algorithm=>:concurrently})
   -> 0.0011s
-- add_index(:internal_ids, [:usage, :project_id], {:unique=>true, :algorithm=>:concurrently})
   -> 0.0066s
-- transaction_open?()
   -> 0.0000s
-- select_one("SELECT current_setting('server_version_num') AS v")
   -> 0.0003s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- indexes(:internal_ids)
   -> 0.0014s
-- remove_index(:internal_ids, {:algorithm=>:concurrently, :name=>"index_internal_ids_on_usage_and_project_id_old"})
   -> 0.0022s
-- remove_foreign_key(:internal_ids, {:column=>:namespace_id})
   -> 0.0051s
== 20180417090132 AddIndexConstraintsToInternalIdTable: reverted (0.0277s) ====

$ spring rake db:rollback VERSION=20180416155103                                                                                                                                                                                                
== 20180416155103 AddFurtherScopeColumnsToInternalIdTable: reverting ==========
-- change_column(:internal_ids, :project_id, :integer, {:null=>false})
   -> 0.0042s
-- remove_column(:internal_ids, :namespace_id)
   -> 0.0004s
== 20180416155103 AddFurtherScopeColumnsToInternalIdTable: reverted (0.0048s) =

Are there points in the code the reviewer needs to double check?

The assumption is that a Milestone belongs to either Project xor Namespace, never both.

Why was this MR needed?

NonatomicInternalId is prone to race-conditions

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes #44259 (closed).

Edited by Yorick Peterse

Merge request reports