From 408208bc2b75d28235092e9bb3821242fdad08fb Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 11 Dec 2017 23:39:40 +0100 Subject: Use AtomicFixNum to implement CAS isolated cache update. i.e. Using compare and swap we update the expires_at value. The thread that actually is able to update this value will also set the cache holding method_call enabled state --- lib/gitlab/metrics/method_call.rb | 17 +++++++---------- spec/lib/gitlab/metrics/method_call_spec.rb | 13 +++++++++---- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index dc56a10957d..2c1cb789314 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -2,7 +2,8 @@ module Gitlab module Metrics # Class for tracking timing information about method calls class MethodCall - MEASUREMENT_ENABLED_CACHE = Concurrent::AtomicReference.new({ enabled: false, expires_at: Time.now }) + MEASUREMENT_ENABLED_CACHE = Concurrent::AtomicBoolean.new(false) + MEASUREMENT_ENABLED_CACHE_EXPIRES_AT = Concurrent::AtomicFixnum.new(Time.now.to_i) MUTEX = Mutex.new BASE_LABELS = { module: nil, method: nil }.freeze attr_reader :real_time, :cpu_time, :call_count, :labels @@ -20,18 +21,14 @@ module Gitlab end def call_measurement_enabled? - res = MEASUREMENT_ENABLED_CACHE.update do |cache| - if cache[:expires_at] < Time.now - { - enabled: Feature.get(:prometheus_metrics_method_instrumentation).enabled?, - expires_at: Time.now + 5.minutes - } - else - cache + expires_at = MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.value + if expires_at < Time.now.to_i + if MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.compare_and_set(expires_at, (Time.now + 30.seconds).to_i) + MEASUREMENT_ENABLED_CACHE.value = Feature.get(:prometheus_metrics_method_instrumentation).enabled? end end - res[:enabled] + MEASUREMENT_ENABLED_CACHE.value end # name - The full name of the method (including namespace) such as diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index eca6a2cb54a..5ce62504245 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -21,7 +21,7 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is enabled' do before do allow(Feature.get(:prometheus_metrics_method_instrumentation)).to receive(:enabled?).and_call_original - described_class::MEASUREMENT_ENABLED_CACHE.set({enabled: false, expires_at: Time.now - 1.second}) + described_class::MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.value = Time.now.to_i - 1 Feature.get(:prometheus_metrics_method_instrumentation).enable end @@ -39,12 +39,16 @@ describe Gitlab::Metrics::MethodCall do expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).once end - it 'expires feature check cache after 5 minutes' do + it 'expires feature check cache after 30 seconds' do 10.times do method_call.measure { 'foo' } end - Timecop.travel(Time.now + 5.minutes) do + Timecop.travel(Time.now + 30.seconds) do + method_call.measure { 'foo' } + end + + Timecop.travel(Time.now + 31.seconds) do method_call.measure { 'foo' } end @@ -62,7 +66,8 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is disabled' do before do - described_class::MEASUREMENT_ENABLED_CACHE.set({enabled: false, expires_at: Time.now}) + described_class::MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.value = Time.now.to_i - 1 + Feature.get(:prometheus_metrics_method_instrumentation).disable end -- cgit v1.2.1