Read about our upcoming Code of Conduct on this issue

Commit 4d314cdf authored by Andreas Brandl's avatar Andreas Brandl
Browse files

Atomic generation of internal ids for issues.

parent af3fd07a1efd
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
# 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
require 'carrierwave/orm/activerecord'
class Issue < ActiveRecord::Base
include NonatomicInternalId
include AtomicInternalId
include Issuable
include Noteable
include Referable
......
......@@ -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'
......
class CreateInternalIdsTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
create_table :internal_ids do |t|
t.references :project
t.integer :usage, null: false
t.integer :last_value, null: false
end
unless index_exists?(:internal_ids, [:usage, :project_id])
add_index :internal_ids, [:usage, :project_id], unique: true
end
unless foreign_key_exists?(:internal_ids, :project_id)
add_concurrent_foreign_key :internal_ids, :projects, column: :project_id, on_delete: :cascade
end
end
def down
drop_table :internal_ids
end
private
def foreign_key_exists?(table, column)
foreign_keys(table).any? do |key|
key.options[:column] == column.to_s
end
end
end
......@@ -866,6 +866,14 @@
add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree
create_table "internal_ids", force: :cascade do |t|
t.integer "project_id"
t.integer "usage", null: false
t.integer "last_value", null: false
end
add_index "internal_ids", ["usage", "project_id"], name: "index_internal_ids_on_usage_and_project_id", unique: true, using: :btree
create_table "issue_assignees", id: false, force: :cascade do |t|
t.integer "user_id", null: false
t.integer "issue_id", null: false
......@@ -2058,6 +2066,7 @@
add_foreign_key "gpg_signatures", "gpg_keys", on_delete: :nullify
add_foreign_key "gpg_signatures", "projects", on_delete: :cascade
add_foreign_key "group_custom_attributes", "namespaces", column: "group_id", on_delete: :cascade
add_foreign_key "internal_ids", "projects", name: "fk_f7d46b66c6", on_delete: :cascade
add_foreign_key "issue_assignees", "issues", name: "fk_b7d881734a", on_delete: :cascade
add_foreign_key "issue_assignees", "users", name: "fk_5e0c8d9154", on_delete: :cascade
add_foreign_key "issue_metrics", "issues", on_delete: :cascade
......
FactoryBot.define do
factory :internal_id do
project
usage { InternalId.usages.keys.first }
end
end
require 'spec_helper'
describe InternalId do
let(:project) { create(:project) }
let(:usage) { :issues }
context 'validations' do
it { is_expected.to validate_presence_of(:usage) }
it { is_expected.to validate_presence_of(:project_id) }
end
describe '.generate_next' do
context 'in the absence of a record' do
subject { described_class.generate_next(project, usage) }
it 'creates a record if not yet present' do
expect { subject }.to change { described_class.count }.from(0).to(1)
end
it 'stores record attributes' do
subject
described_class.first.tap do |record|
expect(record.project).to eq(project)
expect(record.usage).to eq(usage.to_s) # TODO
end
end
context 'with existing issues' do
before do
rand(10).times { create(:issue, project: project) }
end
it 'calculates last_value values automatically' do
expect(subject).to eq(project.issues.size + 1)
end
end
end
it 'generates a strictly monotone, gapless sequence' do
seq = (0..rand(1000)).map do
described_class.generate_next(project, usage)
end
normalized = seq.map { |i| i - seq.min }
expect(normalized).to eq((0..seq.size - 1).to_a)
end
end
describe '#increment_and_save!' do
let(:id) { create(:internal_id) }
subject { id.increment_and_save! }
it 'returns incremented iid' do
value = id.last_value
expect(subject).to eq(value + 1)
end
it 'saves the record' do
subject
expect(id.changed?).to be_falsey
end
context 'with last_value=nil' do
let(:id) { build(:internal_id, last_value: nil) }
it 'returns 1' do
expect(subject).to eq(1)
end
end
end
describe '#calculate_last_value! (for issues)' do
subject do
build(:internal_id, project: project, usage: :issues)
end
context 'with existing issues' do
before do
rand(10).times { create(:issue, project: project) }
end
it 'counts related issues and saves' do
expect { subject.calculate_last_value! }.to change { subject.last_value }.from(nil).to(project.issues.size)
end
end
end
end
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment