Commit 754272e3 authored by Andreas Brandl's avatar Andreas Brandl

Atomic generation of internal ids for issues.

parent a0abb904
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' require 'carrierwave/orm/activerecord'
class Issue < ActiveRecord::Base class Issue < ActiveRecord::Base
include NonatomicInternalId include AtomicInternalId
include Issuable include Issuable
include Noteable include Noteable
include Referable include Referable
......
...@@ -188,6 +188,8 @@ class Project < ActiveRecord::Base ...@@ -188,6 +188,8 @@ class Project < ActiveRecord::Base
has_many :todos has_many :todos
has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent 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 :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true
has_one :project_feature, inverse_of: :project has_one :project_feature, inverse_of: :project
has_one :statistics, class_name: 'ProjectStatistics' 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 @@ ActiveRecord::Schema.define(version: 20180309160427) do ...@@ -866,6 +866,14 @@ ActiveRecord::Schema.define(version: 20180309160427) do
add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree 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| create_table "issue_assignees", id: false, force: :cascade do |t|
t.integer "user_id", null: false t.integer "user_id", null: false
t.integer "issue_id", null: false t.integer "issue_id", null: false
...@@ -2058,6 +2066,7 @@ ActiveRecord::Schema.define(version: 20180309160427) do ...@@ -2058,6 +2066,7 @@ ActiveRecord::Schema.define(version: 20180309160427) do
add_foreign_key "gpg_signatures", "gpg_keys", on_delete: :nullify add_foreign_key "gpg_signatures", "gpg_keys", on_delete: :nullify
add_foreign_key "gpg_signatures", "projects", on_delete: :cascade 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 "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", "issues", name: "fk_b7d881734a", on_delete: :cascade
add_foreign_key "issue_assignees", "users", name: "fk_5e0c8d9154", on_delete: :cascade add_foreign_key "issue_assignees", "users", name: "fk_5e0c8d9154", on_delete: :cascade
add_foreign_key "issue_metrics", "issues", 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