Commit c5d75b4c authored by Sean McGivern's avatar Sean McGivern

Remove log_implicit_sidekiq_status_calls feature flag

We now see no messages logged on GitLab.com production, so we can remove
this flag.
parent fc3ced13
...@@ -15,7 +15,7 @@ module ImportState ...@@ -15,7 +15,7 @@ module ImportState
def refresh_jid_expiration def refresh_jid_expiration
return unless jid return unless jid
Gitlab::SidekiqStatus.set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION, value: 2) Gitlab::SidekiqStatus.set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION)
end end
def self.jid_by(project_id:, status:) def self.jid_by(project_id:, status:)
......
---
name: log_implicit_sidekiq_status_calls
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74815
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343964
milestone: '14.6'
type: development
group: group::scalability
default_enabled: false
...@@ -13,7 +13,7 @@ module Gitlab ...@@ -13,7 +13,7 @@ module Gitlab
def self.set_jid(import_state) def self.set_jid(import_state)
jid = generate_jid(import_state) jid = generate_jid(import_state)
Gitlab::SidekiqStatus.set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION, value: 2) Gitlab::SidekiqStatus.set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION)
import_state.update_column(:jid, jid) import_state.update_column(:jid, jid)
end end
......
...@@ -29,16 +29,13 @@ module Gitlab ...@@ -29,16 +29,13 @@ module Gitlab
# for most jobs. # for most jobs.
DEFAULT_EXPIRATION = 30.minutes.to_i DEFAULT_EXPIRATION = 30.minutes.to_i
DEFAULT_VALUE = 1
DEFAULT_VALUE_MESSAGE = 'Keys using the default value for SidekiqStatus detected'
# Starts tracking of the given job. # Starts tracking of the given job.
# #
# jid - The Sidekiq job ID # jid - The Sidekiq job ID
# expire - The expiration time of the Redis key. # expire - The expiration time of the Redis key.
def self.set(jid, expire = DEFAULT_EXPIRATION, value: DEFAULT_VALUE) def self.set(jid, expire = DEFAULT_EXPIRATION)
Sidekiq.redis do |redis| Sidekiq.redis do |redis|
redis.set(key_for(jid), value, ex: expire) redis.set(key_for(jid), 1, ex: expire)
end end
end end
...@@ -94,17 +91,10 @@ module Gitlab ...@@ -94,17 +91,10 @@ module Gitlab
return [] if job_ids.empty? return [] if job_ids.empty?
keys = job_ids.map { |jid| key_for(jid) } keys = job_ids.map { |jid| key_for(jid) }
results = Sidekiq.redis { |redis| redis.mget(*keys) }
if Feature.enabled?(:log_implicit_sidekiq_status_calls, default_enabled: :yaml)
to_log = keys.zip(results).select do |_key, result|
result == DEFAULT_VALUE.to_s
end.map(&:first)
Sidekiq.logger.info(message: DEFAULT_VALUE_MESSAGE, keys: to_log) if to_log.any?
end
results.map { |result| !result.nil? } Sidekiq
.redis { |redis| redis.mget(*keys) }
.map { |result| !result.nil? }
end end
# Returns the JIDs that are completed # Returns the JIDs that are completed
......
...@@ -8,7 +8,7 @@ RSpec.describe Gitlab::Import::SetAsyncJid do ...@@ -8,7 +8,7 @@ RSpec.describe Gitlab::Import::SetAsyncJid do
it 'sets the JID in Redis' do it 'sets the JID in Redis' do
expect(Gitlab::SidekiqStatus) expect(Gitlab::SidekiqStatus)
.to receive(:set) .to receive(:set)
.with("async-import/project-import-state/#{project.id}", Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION, value: 2) .with("async-import/project-import-state/#{project.id}", Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION)
.and_call_original .and_call_original
described_class.set_jid(project.import_state) described_class.set_jid(project.import_state)
......
...@@ -5,8 +5,8 @@ require 'fast_spec_helper' ...@@ -5,8 +5,8 @@ require 'fast_spec_helper'
RSpec.describe Gitlab::SidekiqStatus::ClientMiddleware do RSpec.describe Gitlab::SidekiqStatus::ClientMiddleware do
describe '#call' do describe '#call' do
context 'when the job has status_expiration set' do context 'when the job has status_expiration set' do
it 'tracks the job in Redis with a value of 2' do it 'tracks the job in Redis' do
expect(Gitlab::SidekiqStatus).to receive(:set).with('123', 1.hour.to_i, value: 2) expect(Gitlab::SidekiqStatus).to receive(:set).with('123', 1.hour.to_i)
described_class.new described_class.new
.call('Foo', { 'jid' => '123', 'status_expiration' => 1.hour.to_i }, double(:queue), double(:pool)) { nil } .call('Foo', { 'jid' => '123', 'status_expiration' => 1.hour.to_i }, double(:queue), double(:pool)) { nil }
...@@ -14,8 +14,8 @@ RSpec.describe Gitlab::SidekiqStatus::ClientMiddleware do ...@@ -14,8 +14,8 @@ RSpec.describe Gitlab::SidekiqStatus::ClientMiddleware do
end end
context 'when the job does not have status_expiration set' do context 'when the job does not have status_expiration set' do
it 'tracks the job in Redis with a value of 1' do it 'tracks the job in Redis' do
expect(Gitlab::SidekiqStatus).to receive(:set).with('123', Gitlab::SidekiqStatus::DEFAULT_EXPIRATION, value: 1) expect(Gitlab::SidekiqStatus).to receive(:set).with('123', Gitlab::SidekiqStatus::DEFAULT_EXPIRATION)
described_class.new described_class.new
.call('Foo', { 'jid' => '123' }, double(:queue), double(:pool)) { nil } .call('Foo', { 'jid' => '123' }, double(:queue), double(:pool)) { nil }
......
...@@ -12,7 +12,7 @@ RSpec.describe Gitlab::SidekiqStatus, :clean_gitlab_redis_queues, :clean_gitlab_ ...@@ -12,7 +12,7 @@ RSpec.describe Gitlab::SidekiqStatus, :clean_gitlab_redis_queues, :clean_gitlab_
Sidekiq.redis do |redis| Sidekiq.redis do |redis|
expect(redis.exists(key)).to eq(true) expect(redis.exists(key)).to eq(true)
expect(redis.ttl(key) > 0).to eq(true) expect(redis.ttl(key) > 0).to eq(true)
expect(redis.get(key)).to eq(described_class::DEFAULT_VALUE.to_s) expect(redis.get(key)).to eq('1')
end end
end end
...@@ -24,19 +24,7 @@ RSpec.describe Gitlab::SidekiqStatus, :clean_gitlab_redis_queues, :clean_gitlab_ ...@@ -24,19 +24,7 @@ RSpec.describe Gitlab::SidekiqStatus, :clean_gitlab_redis_queues, :clean_gitlab_
Sidekiq.redis do |redis| Sidekiq.redis do |redis|
expect(redis.exists(key)).to eq(true) expect(redis.exists(key)).to eq(true)
expect(redis.ttl(key) > described_class::DEFAULT_EXPIRATION).to eq(true) expect(redis.ttl(key) > described_class::DEFAULT_EXPIRATION).to eq(true)
expect(redis.get(key)).to eq(described_class::DEFAULT_VALUE.to_s) expect(redis.get(key)).to eq('1')
end
end
it 'allows overriding the default value' do
described_class.set('123', value: 2)
key = described_class.key_for('123')
Sidekiq.redis do |redis|
expect(redis.exists(key)).to eq(true)
expect(redis.ttl(key) > 0).to eq(true)
expect(redis.get(key)).to eq('2')
end end
end end
end end
...@@ -138,33 +126,5 @@ RSpec.describe Gitlab::SidekiqStatus, :clean_gitlab_redis_queues, :clean_gitlab_ ...@@ -138,33 +126,5 @@ RSpec.describe Gitlab::SidekiqStatus, :clean_gitlab_redis_queues, :clean_gitlab_
it 'handles an empty array' do it 'handles an empty array' do
expect(described_class.job_status([])).to eq([]) expect(described_class.job_status([])).to eq([])
end end
context 'when log_implicit_sidekiq_status_calls is enabled' do
it 'logs keys that contained the default value' do
described_class.set('123', value: 2)
described_class.set('456')
described_class.set('012')
expect(Sidekiq.logger).to receive(:info).with(message: described_class::DEFAULT_VALUE_MESSAGE,
keys: [described_class.key_for('456'), described_class.key_for('012')])
expect(described_class.job_status(%w(123 456 789 012))).to eq([true, true, false, true])
end
end
context 'when log_implicit_sidekiq_status_calls is disabled' do
before do
stub_feature_flags(log_implicit_sidekiq_status_calls: false)
end
it 'does not perform any logging' do
described_class.set('123', value: 2)
described_class.set('456')
expect(Sidekiq.logger).not_to receive(:info)
expect(described_class.job_status(%w(123 456 789))).to eq([true, true, false])
end
end
end end
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