Commit 418e8068 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Improve Sidekiq jobs that use DB load balancing

Removing this FF changes the way we handle Sidekiq jobs that read from
DB replicas. The job will no longer be scheduled 1 second in the future
because this adds considerable overhead to Sidekiq and Redis.

The Sidekiq job would just sleep for at most 1 second if the replica is
not up to date.

Changelog: performance
parent a634d02e
......@@ -13,7 +13,6 @@ module ApplicationWorker
include Gitlab::SidekiqVersioning::Worker
LOGGING_EXTRA_KEY = 'extra'
DEFAULT_DELAY_INTERVAL = 1
SAFE_PUSH_BULK_LIMIT = 1000
included do
......@@ -92,18 +91,6 @@ module ApplicationWorker
validate_worker_attributes!
end
def perform_async(*args)
return super if Gitlab::Database::LoadBalancing.primary_only?
# Worker execution for workers with data_consistency set to :delayed or :sticky
# will be delayed to give replication enough time to complete
if utilizes_load_balancing_capabilities? && Feature.disabled?(:skip_scheduling_workers_for_replicas, default_enabled: :yaml)
perform_in(delay_interval, *args)
else
super
end
end
def set_queue
queue_name = ::Gitlab::SidekiqConfig::WorkerRouter.global.route(self)
sidekiq_options queue: queue_name # rubocop:disable Cop/SidekiqOptionsQueue
......@@ -194,12 +181,6 @@ module ApplicationWorker
end
end
protected
def delay_interval
DEFAULT_DELAY_INTERVAL.seconds
end
private
def do_push_bulk(args_list)
......
---
name: skip_scheduling_workers_for_replicas
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74532
rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1380
milestone: '14.5'
type: development
group: group::project management
default_enabled: false
......@@ -247,45 +247,6 @@ RSpec.describe ApplicationWorker do
end
end
describe '.perform_async' do
using RSpec::Parameterized::TableSyntax
where(:primary_only?, :skip_scheduling_ff, :data_consistency, :schedules_job?) do
true | false | :sticky | false
true | false | :delayed | false
true | false | :always | false
true | true | :sticky | false
true | true | :delayed | false
true | true | :always | false
false | false | :sticky | true
false | false | :delayed | true
false | false | :always | false
false | true | :sticky | false
false | true | :delayed | false
false | true | :always | false
end
before do
stub_const(worker.name, worker)
worker.data_consistency(data_consistency)
allow(Gitlab::Database::LoadBalancing).to receive(:primary_only?).and_return(primary_only?)
stub_feature_flags(skip_scheduling_workers_for_replicas: skip_scheduling_ff)
end
with_them do
it 'schedules or enqueues the job correctly' do
if schedules_job?
expect(worker).to receive(:perform_in).with(described_class::DEFAULT_DELAY_INTERVAL.seconds, 123)
else
expect(worker).not_to receive(:perform_in)
end
worker.perform_async(123)
end
end
end
context 'different kinds of push_bulk' do
shared_context 'set safe limit beyond the number of jobs to be enqueued' do
before do
......
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