Commit 9265e694 authored by Jan Provaznik's avatar Jan Provaznik

Fix performance bar stats worker

* expire stats key in redis after 30 minutes
* fix accessing the correct redis (Cache instead of SharedState)
parent a61bd329
...@@ -7,12 +7,13 @@ class GitlabPerformanceBarStatsWorker ...@@ -7,12 +7,13 @@ class GitlabPerformanceBarStatsWorker
LEASE_TIMEOUT = 600 LEASE_TIMEOUT = 600
WORKER_DELAY = 120 WORKER_DELAY = 120
STATS_KEY = 'performance_bar_stats:pending_request_ids' STATS_KEY = 'performance_bar_stats:pending_request_ids'
STATS_KEY_EXPIRE = 30.minutes.to_i
feature_category :metrics feature_category :metrics
idempotent! idempotent!
def perform(lease_uuid) def perform(lease_uuid)
Gitlab::Redis::SharedState.with do |redis| Gitlab::Redis::Cache.with do |redis|
request_ids = fetch_request_ids(redis, lease_uuid) request_ids = fetch_request_ids(redis, lease_uuid)
stats = Gitlab::PerformanceBar::Stats.new(redis) stats = Gitlab::PerformanceBar::Stats.new(redis)
......
...@@ -15,23 +15,39 @@ module Gitlab ...@@ -15,23 +15,39 @@ module Gitlab
# schedules a job which parses peek profile data and adds them # schedules a job which parses peek profile data and adds them
# to a structured log # to a structured log
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def enqueue_stats_job(request_id) def enqueue_stats_job(request_id)
return unless gather_stats? return unless gather_stats?
@client.sadd(GitlabPerformanceBarStatsWorker::STATS_KEY, request_id) # rubocop:disable Gitlab/ModuleWithInstanceVariables @client.sadd(GitlabPerformanceBarStatsWorker::STATS_KEY, request_id)
return unless uuid = Gitlab::ExclusiveLease.new( return unless uuid = Gitlab::ExclusiveLease.new(
GitlabPerformanceBarStatsWorker::LEASE_KEY, GitlabPerformanceBarStatsWorker::LEASE_KEY,
timeout: GitlabPerformanceBarStatsWorker::LEASE_TIMEOUT timeout: GitlabPerformanceBarStatsWorker::LEASE_TIMEOUT
).try_obtain ).try_obtain
GitlabPerformanceBarStatsWorker.perform_in(GitlabPerformanceBarStatsWorker::WORKER_DELAY, uuid) # stats key should be periodically processed and deleted by
# GitlabPerformanceBarStatsWorker but if it doesn't happen for
# some reason, we set expiration for the stats key to avoid
# keeping millions of request ids which would be already expired
# anyway
# rubocop:disable Gitlab/ModuleWithInstanceVariables
@client.expire(
GitlabPerformanceBarStatsWorker::STATS_KEY,
GitlabPerformanceBarStatsWorker::STATS_KEY_EXPIRE
)
GitlabPerformanceBarStatsWorker.perform_in(
GitlabPerformanceBarStatsWorker::WORKER_DELAY,
uuid
)
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def gather_stats? def gather_stats?
return unless Feature.enabled?(:performance_bar_stats) return unless Feature.enabled?(:performance_bar_stats)
Gitlab.com? || !Rails.env.production? Gitlab.com? || Gitlab.staging? || !Rails.env.production?
end end
end end
end end
......
...@@ -31,6 +31,7 @@ RSpec.describe Gitlab::PerformanceBar::RedisAdapterWhenPeekEnabled do ...@@ -31,6 +31,7 @@ RSpec.describe Gitlab::PerformanceBar::RedisAdapterWhenPeekEnabled do
expect_to_obtain_exclusive_lease(GitlabPerformanceBarStatsWorker::LEASE_KEY, uuid) expect_to_obtain_exclusive_lease(GitlabPerformanceBarStatsWorker::LEASE_KEY, uuid)
expect(GitlabPerformanceBarStatsWorker).to receive(:perform_in).with(GitlabPerformanceBarStatsWorker::WORKER_DELAY, uuid) expect(GitlabPerformanceBarStatsWorker).to receive(:perform_in).with(GitlabPerformanceBarStatsWorker::WORKER_DELAY, uuid)
expect(client).to receive(:sadd).with(GitlabPerformanceBarStatsWorker::STATS_KEY, uuid) expect(client).to receive(:sadd).with(GitlabPerformanceBarStatsWorker::STATS_KEY, uuid)
expect(client).to receive(:expire).with(GitlabPerformanceBarStatsWorker::STATS_KEY, GitlabPerformanceBarStatsWorker::STATS_KEY_EXPIRE)
peek_adapter.new(client).save('foo') peek_adapter.new(client).save('foo')
end end
......
...@@ -12,7 +12,7 @@ RSpec.describe GitlabPerformanceBarStatsWorker do ...@@ -12,7 +12,7 @@ RSpec.describe GitlabPerformanceBarStatsWorker do
let(:uuid) { 1 } let(:uuid) { 1 }
before do before do
expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis) expect(Gitlab::Redis::Cache).to receive(:with).and_yield(redis)
expect_to_cancel_exclusive_lease(GitlabPerformanceBarStatsWorker::LEASE_KEY, uuid) expect_to_cancel_exclusive_lease(GitlabPerformanceBarStatsWorker::LEASE_KEY, uuid)
end end
......
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