From 53dc9e83c34b2a0ee2651046de031566f5b925d2 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 7 Dec 2017 19:49:44 +0100 Subject: Cache feature check for 5 minutes for MethodCall instrumentation toggle --- spec/lib/gitlab/metrics/method_call_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index 5341addf911..df4c5167497 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -23,6 +23,17 @@ describe Gitlab::Metrics::MethodCall do Feature.get(:prometheus_metrics_method_instrumentation).enable end + it 'feature check is cached for 5 minutes' do + allow(Feature.get(:prometheus_metrics_method_instrumentation)).to receive(:enabled?).and_call_original + allow(Rails.cache).to receive(:fetch).and_call_original + + method_call.measure { 'foo' } + method_call.measure { 'foo' } + + expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).twice + expect(Rails.cache).to have_received(:fetch).with(:prometheus_metrics_method_instrumentation_enabled, expires_in: 5.minutes).twice + end + it 'observes the performance of the supplied block' do expect(described_class.call_duration_histogram) .to receive(:observe) -- cgit v1.2.1 From b503e6ff423699f7556fb948051e9c180a7b89f0 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 8 Dec 2017 19:36:30 +0100 Subject: Implement simple in memory cache that expires after 5 minutes --- spec/lib/gitlab/metrics/method_call_spec.rb | 30 +++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index df4c5167497..f514fdd70ed 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -20,18 +20,35 @@ 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.call_measurement_enabled_cache_expire Feature.get(:prometheus_metrics_method_instrumentation).enable end - it 'feature check is cached for 5 minutes' do - allow(Feature.get(:prometheus_metrics_method_instrumentation)).to receive(:enabled?).and_call_original - allow(Rails.cache).to receive(:fetch).and_call_original + around do |example| + Timecop.freeze do + example.run + end + end - method_call.measure { 'foo' } - method_call.measure { 'foo' } + it 'caches subsequent invocations of feature check' do + 10.times do + method_call.measure { 'foo' } + end + + expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).once + end + + it 'expires feature check cache after 5 minutes' do + 10.times do + method_call.measure { 'foo' } + end + + Timecop.travel(Time.now + 5.minutes) do + method_call.measure { 'foo' } + end expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).twice - expect(Rails.cache).to have_received(:fetch).with(:prometheus_metrics_method_instrumentation_enabled, expires_in: 5.minutes).twice end it 'observes the performance of the supplied block' do @@ -45,6 +62,7 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is disabled' do before do + described_class.call_measurement_enabled_cache_expire Feature.get(:prometheus_metrics_method_instrumentation).disable end -- cgit v1.2.1 From 5904b033dba553636ae2a06cbf1469d8f19df040 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 11 Dec 2017 22:24:07 +0100 Subject: Implemente measurement enabled cache using AtomicReference --- spec/lib/gitlab/metrics/method_call_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index f514fdd70ed..eca6a2cb54a 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.call_measurement_enabled_cache_expire + described_class::MEASUREMENT_ENABLED_CACHE.set({enabled: false, expires_at: Time.now - 1.second}) Feature.get(:prometheus_metrics_method_instrumentation).enable end @@ -62,7 +62,7 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is disabled' do before do - described_class.call_measurement_enabled_cache_expire + described_class::MEASUREMENT_ENABLED_CACHE.set({enabled: false, expires_at: Time.now}) Feature.get(:prometheus_metrics_method_instrumentation).disable end -- cgit v1.2.1 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 --- spec/lib/gitlab/metrics/method_call_spec.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'spec/lib') 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 From fd0a5168542f3a69815c99dc21f39587064f872d Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 12 Dec 2017 00:23:56 +0100 Subject: use class variables instead of CONSTANTs --- spec/lib/gitlab/metrics/method_call_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index 5ce62504245..f98d7e48b17 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_EXPIRES_AT.value = Time.now.to_i - 1 + described_class.measurement_enabled_cache_expires_at.value = Time.now.to_i - 1 Feature.get(:prometheus_metrics_method_instrumentation).enable end @@ -66,7 +66,7 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is disabled' do before do - described_class::MEASUREMENT_ENABLED_CACHE_EXPIRES_AT.value = Time.now.to_i - 1 + 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 From da19ce625ba8a0db9921fd85e331e70b14a5625b Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 12 Dec 2017 18:52:08 +0100 Subject: Expire feature flag cache after 1minute --- spec/lib/gitlab/metrics/method_call_spec.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index f98d7e48b17..c11da42ed29 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -40,15 +40,13 @@ describe Gitlab::Metrics::MethodCall do end it 'expires feature check cache after 30 seconds' do - 10.times do - method_call.measure { 'foo' } - end + method_call.measure { 'foo' } - Timecop.travel(Time.now + 30.seconds) do + Timecop.travel(1.minute.from_now) do method_call.measure { 'foo' } end - Timecop.travel(Time.now + 31.seconds) do + Timecop.travel(1.minute.from_now + 1.second) do method_call.measure { 'foo' } end -- cgit v1.2.1 From db9e5bf75e8546649ddd65a5696ff4edb87ded20 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 13 Dec 2017 18:43:08 +0100 Subject: fix test case description --- spec/lib/gitlab/metrics/method_call_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index c11da42ed29..78767d06462 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -39,7 +39,7 @@ describe Gitlab::Metrics::MethodCall do expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).once end - it 'expires feature check cache after 30 seconds' do + it 'expires feature check cache after 1 minute' do method_call.measure { 'foo' } Timecop.travel(1.minute.from_now) do -- cgit v1.2.1