Skip to content
Snippets Groups Projects

Add lease to update project statistics row and log concurrent updates

Merged Albert requested to merge 373595-lock-on-project-statistics-update into master
4 files
+ 63
39
Compare changes
  • Side-by-side
  • Inline
Files
4
@@ -47,7 +47,7 @@ module CounterAttribute
WORKER_DELAY = 10.minutes
WORKER_LOCK_TTL = 10.minutes
DATABASE_LOCK_RETRY_INTERVAL = 0.5
DATABASE_LEASE_LOCK_RETRY_INTERVAL = 0.5
class_methods do
def counter_attribute(attribute)
@@ -96,7 +96,7 @@ def flush_increments_to_database!(attribute)
next if increment_value == 0
transaction do
update_counters_with_lock({ attribute => increment_value })
update_counters_with_lease({ attribute => increment_value })
redis_state { |redis| redis.del(flushed_key) }
new_db_value = reset.read_attribute(attribute)
end
@@ -131,0+131,0 @@
end
end
def update_counters_with_lock(increments)
with_lock_for_update(log_fields: increments.merge({ caller: __method__ })) do
def update_counters_with_lease(increments)
with_lease_for_update(log_fields: increments.merge({ caller: __method__ })) do
self.class.update_counters(id, increments)
end
end
def reset_counter!(attribute)
with_lock_for_update(log_fields: { caller: __method__ }) do
with_lease_for_update(log_fields: { caller: __method__ }) do
update!(attribute => 0)
clear_counter!(attribute)
end
clear_counter!(attribute)
end
def clear_counter!(attribute)
@@ -176,0+177,0 @@
self.class.counter_attribute_enabled?(attribute)
end
def with_lock_for_update(log_fields: {})
# with_lease_for_update uses a lease to monitor access
# to the project statistics row. This is needed to detect
# concurrent attempts to increment columns, which could result in a
# race condition.
#
# As the purpose is to detect and warn concurrent attempts,
# it falls back to direct update on the row if it fails to obtain the lease.
#
# It does not guarantee that there will not be any concurrent updates.
def with_lease_for_update(log_fields: {})
return yield unless Feature.enabled?(:counter_attribute_lock_for_update, project)
Retriable.retriable(
tries: 3,
base_interval: DATABASE_LOCK_RETRY_INTERVAL,
on: ActiveRecord::LockWaitTimeout,
base_interval: DATABASE_LEASE_LOCK_RETRY_INTERVAL,
on: Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError,
on_retry: retry_logger(log_fields)
) do
with_lock('FOR UPDATE NOWAIT') do
in_lock(database_lock_key) do
yield
end
end
rescue Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError
# fallback to direct update on columns
yield
end
private
def database_lock_key
"project:{#{project_id}}:#{self.class}:#{id}"
end
def steal_increments(increment_key, flushed_key)
redis_state do |redis|
redis.eval(LUA_STEAL_INCREMENT_SCRIPT, keys: [increment_key, flushed_key])
Loading