Commit 507d9e31 authored by Kamil Trzciński's avatar Kamil Trzciński

Track if IIDs are assigned within transaction

parent a3f68244
...@@ -47,18 +47,10 @@ class InternalId < ApplicationRecord ...@@ -47,18 +47,10 @@ class InternalId < ApplicationRecord
def update_and_save(&block) def update_and_save(&block)
lock! lock!
yield yield
update_and_save_counter.increment(usage: usage, changed: last_value_changed?)
save! save!
last_value last_value
end end
# Instrumentation to track for-update locks
def update_and_save_counter
strong_memoize(:update_and_save_counter) do
Gitlab::Metrics.counter(:gitlab_internal_id_for_update_lock, 'Number of ROW SHARE (FOR UPDATE) locks on individual records from internal_ids')
end
end
class << self class << self
def track_greatest(subject, scope, usage, new_value, init) def track_greatest(subject, scope, usage, new_value, init)
InternalIdGenerator.new(subject, scope, usage, init) InternalIdGenerator.new(subject, scope, usage, init)
...@@ -88,6 +80,8 @@ class InternalId < ApplicationRecord ...@@ -88,6 +80,8 @@ class InternalId < ApplicationRecord
end end
class InternalIdGenerator class InternalIdGenerator
extend Gitlab::Utils::StrongMemoize
# Generate next internal id for a given scope and usage. # Generate next internal id for a given scope and usage.
# #
# For currently supported usages, see #usage enum. # For currently supported usages, see #usage enum.
...@@ -123,6 +117,8 @@ class InternalId < ApplicationRecord ...@@ -123,6 +117,8 @@ class InternalId < ApplicationRecord
# init: Block that gets called to initialize InternalId record if not present # init: Block that gets called to initialize InternalId record if not present
# Make sure to not throw exceptions in the absence of records (if this is expected). # Make sure to not throw exceptions in the absence of records (if this is expected).
def generate def generate
self.class.internal_id_transactions_increment(operation: :generate, usage: usage)
subject.transaction do subject.transaction do
# Create a record in internal_ids if one does not yet exist # Create a record in internal_ids if one does not yet exist
# and increment its last value # and increment its last value
...@@ -138,6 +134,8 @@ class InternalId < ApplicationRecord ...@@ -138,6 +134,8 @@ class InternalId < ApplicationRecord
def reset(value) def reset(value)
return false unless value return false unless value
self.class.internal_id_transactions_increment(operation: :reset, usage: usage)
updated = updated =
InternalId InternalId
.where(**scope, usage: usage_value) .where(**scope, usage: usage_value)
...@@ -152,6 +150,8 @@ class InternalId < ApplicationRecord ...@@ -152,6 +150,8 @@ class InternalId < ApplicationRecord
# #
# Note this will acquire a ROW SHARE lock on the InternalId record # Note this will acquire a ROW SHARE lock on the InternalId record
def track_greatest(new_value) def track_greatest(new_value)
self.class.internal_id_transactions_increment(operation: :track_greatest, usage: usage)
subject.transaction do subject.transaction do
record.track_greatest_and_save!(new_value) record.track_greatest_and_save!(new_value)
end end
...@@ -162,6 +162,8 @@ class InternalId < ApplicationRecord ...@@ -162,6 +162,8 @@ class InternalId < ApplicationRecord
end end
def with_lock(&block) def with_lock(&block)
self.class.internal_id_transactions_increment(operation: :with_lock, usage: usage)
record.with_lock(&block) record.with_lock(&block)
end end
...@@ -197,5 +199,22 @@ class InternalId < ApplicationRecord ...@@ -197,5 +199,22 @@ class InternalId < ApplicationRecord
rescue ActiveRecord::RecordNotUnique rescue ActiveRecord::RecordNotUnique
lookup lookup
end end
def self.internal_id_transactions_increment(operation:, usage:)
self.internal_id_transactions_total.increment(
operation: operation,
usage: usage.to_s,
in_transaction: ActiveRecord::Base.connection.transaction_open?.to_s
)
end
def self.internal_id_transactions_total
strong_memoize(:internal_id_transactions_total) do
name = :gitlab_internal_id_transactions_total
comment = 'Counts all the internal ids happening within transaction'
Gitlab::Metrics.counter(name, comment)
end
end
end end
end end
...@@ -97,6 +97,25 @@ RSpec.describe InternalId do ...@@ -97,6 +97,25 @@ RSpec.describe InternalId do
expect(subject).to eq(1) expect(subject).to eq(1)
end end
end end
context 'when executed outside of transaction' do
it 'increments counter with in_transaction: "false"' do
expect(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
.with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original
subject
end
end
context 'when executed within transaction' do
it 'increments counter with in_transaction: "true"' do
expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
.with(operation: :generate, usage: 'issues', in_transaction: 'true').and_call_original
InternalId.transaction { subject }
end
end
end end
describe '.reset' do describe '.reset' do
...@@ -134,6 +153,29 @@ RSpec.describe InternalId do ...@@ -134,6 +153,29 @@ RSpec.describe InternalId do
described_class.generate_next(issue, scope, usage, init) described_class.generate_next(issue, scope, usage, init)
end end
end end
context 'when executed outside of transaction' do
let(:value) { 2 }
it 'increments counter with in_transaction: "false"' do
expect(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
.with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original
subject
end
end
context 'when executed within transaction' do
let(:value) { 2 }
it 'increments counter with in_transaction: "true"' do
expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
.with(operation: :reset, usage: 'issues', in_transaction: 'true').and_call_original
InternalId.transaction { subject }
end
end
end end
describe '.track_greatest' do describe '.track_greatest' do
...@@ -183,6 +225,25 @@ RSpec.describe InternalId do ...@@ -183,6 +225,25 @@ RSpec.describe InternalId do
expect(subject).to eq(value) expect(subject).to eq(value)
end end
end end
context 'when executed outside of transaction' do
it 'increments counter with in_transaction: "false"' do
expect(ActiveRecord::Base.connection).to receive(:transaction_open?) { false }
expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
.with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original
subject
end
end
context 'when executed within transaction' do
it 'increments counter with in_transaction: "true"' do
expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment)
.with(operation: :track_greatest, usage: 'issues', in_transaction: 'true').and_call_original
InternalId.transaction { subject }
end
end
end end
describe '#increment_and_save!' do describe '#increment_and_save!' do
......
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