Remove double-checked internal id generation.
This was needed for a transition phase only. For details see #45389. Closes #45389.
This commit is contained in:
parent
cfdd80ec10
commit
f2cc1169e8
|
@ -24,12 +24,9 @@ class InternalId < ActiveRecord::Base
|
|||
#
|
||||
# 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.
|
||||
#
|
||||
# If a `maximum_iid` is passed in, this overrides the incremented value if it's
|
||||
# greater than that. This can be used to correct the increment value if necessary.
|
||||
def increment_and_save!(maximum_iid)
|
||||
def increment_and_save!
|
||||
lock!
|
||||
self.last_value = [(last_value || 0) + 1, (maximum_iid || 0) + 1].max
|
||||
self.last_value = (last_value || 0) + 1
|
||||
save!
|
||||
last_value
|
||||
end
|
||||
|
@ -93,16 +90,7 @@ class InternalId < ActiveRecord::Base
|
|||
# and increment its last value
|
||||
#
|
||||
# Note this will acquire a ROW SHARE lock on the InternalId record
|
||||
|
||||
# Note we always calculate the maximum iid present here and
|
||||
# pass it in to correct the InternalId entry if it's last_value is off.
|
||||
#
|
||||
# This can happen in a transition phase where both `AtomicInternalId` and
|
||||
# `NonatomicInternalId` code runs (e.g. during a deploy).
|
||||
#
|
||||
# This is subject to be cleaned up with the 10.8 release:
|
||||
# https://gitlab.com/gitlab-org/gitlab-ce/issues/45389.
|
||||
(lookup || create_record).increment_and_save!(maximum_iid)
|
||||
(lookup || create_record).increment_and_save!
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -128,15 +116,11 @@ class InternalId < ActiveRecord::Base
|
|||
InternalId.create!(
|
||||
**scope,
|
||||
usage: usage_value,
|
||||
last_value: maximum_iid
|
||||
last_value: init.call(subject) || 0
|
||||
)
|
||||
end
|
||||
rescue ActiveRecord::RecordNotUnique
|
||||
lookup
|
||||
end
|
||||
|
||||
def maximum_iid
|
||||
@maximum_iid ||= init.call(subject) || 0
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Remove double-checked internal id generation.
|
||||
merge_request: 19181
|
||||
author:
|
||||
type: performance
|
|
@ -5,7 +5,7 @@ describe InternalId do
|
|||
let(:usage) { :issues }
|
||||
let(:issue) { build(:issue, project: project) }
|
||||
let(:scope) { { project: project } }
|
||||
let(:init) { ->(s) { s.project.issues.maximum(:iid) } }
|
||||
let(:init) { ->(s) { s.project.issues.size } }
|
||||
|
||||
context 'validations' do
|
||||
it { is_expected.to validate_presence_of(:usage) }
|
||||
|
@ -39,29 +39,6 @@ describe InternalId do
|
|||
end
|
||||
end
|
||||
|
||||
context 'with an InternalId record present and existing issues with a higher internal id' do
|
||||
# This can happen if the old NonatomicInternalId is still in use
|
||||
before do
|
||||
issues = Array.new(rand(1..10)).map { create(:issue, project: project) }
|
||||
|
||||
issue = issues.last
|
||||
issue.iid = issues.map { |i| i.iid }.max + 1
|
||||
issue.save
|
||||
end
|
||||
|
||||
let(:maximum_iid) { project.issues.map { |i| i.iid }.max }
|
||||
|
||||
it 'updates last_value to the maximum internal id present' do
|
||||
subject
|
||||
|
||||
expect(described_class.find_by(project: project, usage: described_class.usages[usage.to_s]).last_value).to eq(maximum_iid + 1)
|
||||
end
|
||||
|
||||
it 'returns next internal id correctly' do
|
||||
expect(subject).to eq(maximum_iid + 1)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with concurrent inserts on table' do
|
||||
it 'looks up the record if it was created concurrently' do
|
||||
args = { **scope, usage: described_class.usages[usage.to_s] }
|
||||
|
@ -104,8 +81,7 @@ describe InternalId do
|
|||
|
||||
describe '#increment_and_save!' do
|
||||
let(:id) { create(:internal_id) }
|
||||
let(:maximum_iid) { nil }
|
||||
subject { id.increment_and_save!(maximum_iid) }
|
||||
subject { id.increment_and_save! }
|
||||
|
||||
it 'returns incremented iid' do
|
||||
value = id.last_value
|
||||
|
@ -126,14 +102,5 @@ describe InternalId do
|
|||
expect(subject).to eq(1)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with maximum_iid given' do
|
||||
let(:id) { create(:internal_id, last_value: 1) }
|
||||
let(:maximum_iid) { id.last_value + 10 }
|
||||
|
||||
it 'returns maximum_iid instead' do
|
||||
expect(subject).to eq(12)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue