diff options
author | Andreas Brandl <abrandl@gitlab.com> | 2018-03-06 22:09:01 +0300 |
---|---|---|
committer | Andreas Brandl <abrandl@gitlab.com> | 2018-03-16 15:35:25 +0300 |
commit | 754272e392c0da088200a1b56156600973f63267 (patch) | |
tree | 21fdb2f633deff884d39d89f7672f230f1d6c143 /app/models | |
parent | a0abb904782970de456dae5539ad5de2afef0e05 (diff) |
Atomic generation of internal ids for issues.
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/concerns/atomic_internal_id.rb | 19 | ||||
-rw-r--r-- | app/models/internal_id.rb | 84 | ||||
-rw-r--r-- | app/models/issue.rb | 2 | ||||
-rw-r--r-- | app/models/project.rb | 2 |
4 files changed, 106 insertions, 1 deletions
diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb new file mode 100644 index 00000000000..3cc9ce7f03f --- /dev/null +++ b/app/models/concerns/atomic_internal_id.rb @@ -0,0 +1,19 @@ +module AtomicInternalId + extend ActiveSupport::Concern + + included do + before_validation(on: :create) do + set_iid + end + + validates :iid, presence: true, numericality: true + end + + def set_iid + self.iid = InternalId.generate_next(self.project, :issues) if iid.blank? + end + + def to_param + iid.to_s + end +end diff --git a/app/models/internal_id.rb b/app/models/internal_id.rb new file mode 100644 index 00000000000..24c7cbf988f --- /dev/null +++ b/app/models/internal_id.rb @@ -0,0 +1,84 @@ +# An InternalId is a strictly monotone sequence of integers +# for a given project and usage (e.g. issues). +# +# For possible usages, see InternalId#usage enum. +class InternalId < ActiveRecord::Base + belongs_to :project + + enum usage: { issues: 0 } + + validates :usage, presence: true + validates :project_id, presence: true + + # Increments #last_value and saves the record + # + # The operation locks the record and gathers + # a `ROW SHARE` lock (in PostgreSQL). As such, + # the increment is atomic and safe to be called + # concurrently. + def increment_and_save! + lock! + self.last_value = (last_value || 0) + 1 + save! + last_value + end + + before_create :calculate_last_value! + + # Calculate #last_value by counting the number of + # existing records for this usage. + def calculate_last_value! + return if last_value + + parent = project # ??|| group + self.last_value = parent.send(usage.to_sym).maximum(:iid) || 0 # rubocop:disable GitlabSecurity/PublicSend + end + + class << self + # Generate next internal id for a given project and usage. + # + # For currently supported usages, see #usage enum. + # + # The method implements a locking scheme that has the following properties: + # 1) Generated sequence of internal ids is unique per (project, usage) + # 2) The method is thread-safe and may be used in concurrent threads/processes. + # 3) The generated sequence is gapless. + # 4) In the absence of a record in the internal_ids table, one will be created + # and last_value will be calculated on the fly. + def generate_next(project, usage) + raise 'project not set - this is required' unless project + + project.transaction do + # Create a record in internal_ids if one does not yet exist + id = (lookup(project, usage) || create_record(project, usage)) + + # This will lock the InternalId record with ROW SHARE + # and increment #last_value + id.increment_and_save! + end + end + + private + + # Retrieve InternalId record for (project, usage) combination, if it exists + def lookup(project, usage) + project.internal_ids.find_by(usage: usages[usage.to_s]) + end + + # Create InternalId record for (project, usage) combination, if it doesn't exist + # + # We blindly insert without any synchronization. If another process + # was faster in doing this, we'll realize once we hit the unique key constraint + # violation. We can safely roll-back the nested transaction and perform + # a lookup instead to retrieve the record. + def create_record(project, usage) + begin + project.transaction(requires_new: true) do + create!(project: project, usage: usages[usage.to_s]) + end + rescue ActiveRecord::RecordNotUnique + lookup(project, usage) + end + end + end +end diff --git a/app/models/issue.rb b/app/models/issue.rb index cca6224d65c..cf547106122 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1,7 +1,7 @@ require 'carrierwave/orm/activerecord' class Issue < ActiveRecord::Base - include NonatomicInternalId + include AtomicInternalId include Issuable include Noteable include Referable diff --git a/app/models/project.rb b/app/models/project.rb index a291ad7eed5..93b0da5cce1 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -188,6 +188,8 @@ class Project < ActiveRecord::Base has_many :todos has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + has_many :internal_ids + has_one :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true has_one :project_feature, inverse_of: :project has_one :statistics, class_name: 'ProjectStatistics' |