Merge branch '329430-upgrade-sidekiq-6' into 'master'

Upgrade to Sidekiq 6

See merge request gitlab-org/gitlab!69655

Gemfile

@@ -195,10 +195,10 @@ gem 'state_machines-activerecord', '~> 0.8.0'
 gem 'acts-as-taggable-on', '~> 7.0'
 
 # Background jobs
-gem 'sidekiq', '~> 5.2.7'
+gem 'sidekiq', '~> 6.2.2'
 gem 'sidekiq-cron', '~> 1.0'
 gem 'redis-namespace', '~> 1.8.1'
-gem 'gitlab-sidekiq-fetcher', '0.5.6', require: 'sidekiq-reliable-fetch'
+gem 'gitlab-sidekiq-fetcher', '0.8.0', require: 'sidekiq-reliable-fetch'
 
 # Cron Parser
 gem 'fugit', '~> 1.2.1'
@@ -229,7 +229,7 @@ gem 'js_regex', '~> 3.7'
 gem 'device_detector'
 
 # Redis
-gem 'redis', '~> 4.1.4'
+gem 'redis', '~> 4.4.0'
 gem 'connection_pool', '~> 2.0'
 
 # Redis session store

Gemfile.lock

@@ -490,8 +490,8 @@ GEM
       addressable (~> 2.7)
       omniauth (~> 1.9)
       openid_connect (~> 1.2)
-    gitlab-sidekiq-fetcher (0.5.6)
-      sidekiq (~> 5)
+    gitlab-sidekiq-fetcher (0.8.0)
+      sidekiq (~> 6.1)
     gitlab-styles (6.3.0)
       rubocop (~> 0.91, >= 0.91.1)
       rubocop-gitlab-security (~> 0.1.1)
@@ -959,8 +959,6 @@ GEM
       httpclient
       json-jwt (>= 1.11.0)
       rack (>= 2.1.0)
-    rack-protection (2.0.5)
-      rack
     rack-proxy (0.6.0)
       rack
     rack-test (1.1.0)
@@ -1017,7 +1015,7 @@ GEM
     recaptcha (4.13.1)
       json
     recursive-open-struct (1.1.3)
-    redis (4.1.4)
+    redis (4.4.0)
     redis-actionpack (5.2.0)
       actionpack (>= 5, < 7)
       redis-rack (>= 2.1.0, < 3)
@@ -1172,11 +1170,10 @@ GEM
     shellany (0.0.1)
     shoulda-matchers (4.0.1)
       activesupport (>= 4.2.0)
-    sidekiq (5.2.9)
-      connection_pool (~> 2.2, >= 2.2.2)
+    sidekiq (6.2.2)
+      connection_pool (>= 2.2.2)
       rack (~> 2.0)
-      rack-protection (>= 1.5.0)
-      redis (>= 3.3.5, < 4.2)
+      redis (>= 4.2.0)
     sidekiq-cron (1.0.4)
       fugit (~> 1.1)
       sidekiq (>= 4.2.1)
@@ -1473,7 +1470,7 @@ DEPENDENCIES
   gitlab-markup (~> 1.7.1)
   gitlab-net-dns (~> 0.9.1)
   gitlab-omniauth-openid-connect (~> 0.8.0)
-  gitlab-sidekiq-fetcher (= 0.5.6)
+  gitlab-sidekiq-fetcher (= 0.8.0)
   gitlab-styles (~> 6.3.0)
   gitlab_chronic_duration (~> 0.10.6.2)
   gitlab_omniauth-ldap (~> 2.1.1)
@@ -1585,7 +1582,7 @@ DEPENDENCIES
   rdoc (~> 6.3.2)
   re2 (~> 1.2.0)
   recaptcha (~> 4.11)
-  redis (~> 4.1.4)
+  redis (~> 4.4.0)
   redis-actionpack (~> 5.2.0)
   redis-namespace (~> 1.8.1)
   request_store (~> 1.5)
@@ -1614,7 +1611,7 @@ DEPENDENCIES
   sentry-raven (~> 3.1)
   settingslogic (~> 2.0.9)
   shoulda-matchers (~> 4.0.1)
-  sidekiq (~> 5.2.7)
+  sidekiq (~> 6.2.2)
   sidekiq-cron (~> 1.0)
   simple_po_parser (~> 1.1.2)
   simplecov (~> 0.18.5)

config/initializers/7_redis.rb

 # frozen_string_literal: true
 
+# We set the instance variable directly to suppress warnings.
+# We cannot switch to the new behavior until we change all existing `redis.exists` calls to `redis.exists?`.
+# Some gems also need to be updated
+# https://gitlab.com/gitlab-org/gitlab/-/issues/340602
+Redis.instance_variable_set(:@exists_returns_integer, false)
+
 Redis::Client.prepend(Gitlab::Instrumentation::RedisInterceptor)
 
 # Make sure we initialize a Redis connection pool before multi-threaded

config/initializers/mailer_retries.rb

 # frozen_string_literal: true
 
-class ActiveJob::QueueAdapters::SidekiqAdapter
-  # With Sidekiq 6, we can do something like:
-  # class ActionMailer::MailDeliveryJob
-  #   sidekiq_options retry: 3
-  # end
-  #
-  # See https://gitlab.com/gitlab-org/gitlab/-/issues/329430
-  raise "Update this monkey patch: #{__FILE__}" unless Sidekiq::VERSION == '5.2.9'
-
-  def enqueue(job) #:nodoc:
-    # Sidekiq::Client does not support symbols as keys
-    job.provider_job_id = Sidekiq::Client.push \
-      "class"   => JobWrapper,
-      "wrapped" => job.class.to_s,
-      "queue"   => job.queue_name,
-      "args"    => [job.serialize],
-      "retry"   => retry_for(job)
-  end
-
-  def enqueue_at(job, timestamp) #:nodoc:
-    job.provider_job_id = Sidekiq::Client.push \
-      "class"   => JobWrapper,
-      "wrapped" => job.class.to_s,
-      "queue"   => job.queue_name,
-      "args"    => [job.serialize],
-      "at"      => timestamp,
-      "retry"   => retry_for(job)
-  end
-
-  private
-
-  def retry_for(job)
-    if job.queue_name == 'mailers'
-      3
-    else
-      true
-    end
-  end
+Rails.application.config.after_initialize do
+  ActionMailer::MailDeliveryJob.sidekiq_options retry: 3
 end

config/initializers/sidekiq.rb

@@ -27,8 +27,13 @@ use_sidekiq_daemon_memory_killer = ENV.fetch("SIDEKIQ_DAEMON_MEMORY_KILLER", 1).
 use_sidekiq_legacy_memory_killer = !use_sidekiq_daemon_memory_killer
 
 Sidekiq.configure_server do |config|
+  config.options[:strict] = false
+  config.options[:queues] = Gitlab::SidekiqConfig.expand_queues(config.options[:queues])
+
+  Sidekiq.logger.info "Listening on queues #{config.options[:queues].uniq.sort}"
+
   if enable_json_logs
-    Sidekiq.logger.formatter = Gitlab::SidekiqLogging::JSONFormatter.new
+    config.log_formatter = Gitlab::SidekiqLogging::JSONFormatter.new
     config.options[:job_logger] = Gitlab::SidekiqLogging::StructuredLogger
 
     # Remove the default-provided handler. The exception is logged inside

config/initializers_before_autoloader/002_sidekiq.rb

@@ -8,10 +8,6 @@
 
 require 'sidekiq/web'
 
-# Disable the Sidekiq Rack session since GitLab already has its own session store.
-# CSRF protection still works (https://github.com/mperham/sidekiq/commit/315504e766c4fd88a29b7772169060afc4c40329).
-Sidekiq::Web.set :sessions, false
-
 if Rails.env.development?
   Sidekiq.default_worker_options[:backtrace] = true
 end

ee/spec/lib/gitlab/mirror_spec.rb

@@ -3,8 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Mirror do
-  before do
-    Sidekiq::Logging.logger = nil
+  around do |example|
+    original_logger = Sidekiq.logger
+    Sidekiq.logger = nil
+
+    example.run
+
+    Sidekiq.logger = original_logger
   end
 
   describe '#configure_cron_job!' do

ee/spec/workers/vulnerability_exports/export_worker_spec.rb

@@ -45,7 +45,7 @@ RSpec.describe VulnerabilityExports::ExportWorker, type: :worker do
     let(:default_job_payload) { { 'class' => described_class.name, 'args' => [vulnerability_export.id] } }
 
     subject(:run_job) do
-      sidekiq_retry_handler.local(worker, job_payload, 'default') do
+      sidekiq_retry_handler.local(worker, job_payload.to_json, 'default') do
         raise 'Foo'
       end
     end

lib/gitlab/sidekiq_logging/structured_logger.rb

@@ -5,7 +5,7 @@ require 'active_record/log_subscriber'
 
 module Gitlab
   module SidekiqLogging
-    class StructuredLogger
+    class StructuredLogger < Sidekiq::JobLogger
       include LogsJobs
 
       def call(job, queue)

lib/gitlab/sidekiq_middleware/client_metrics.rb

@@ -13,7 +13,7 @@ module Gitlab
 
       def call(worker_class, job, queue, _redis_pool)
         # worker_class can either be the string or class of the worker being enqueued.
-        worker_class = worker_class.safe_constantize if worker_class.respond_to?(:safe_constantize)
+        worker_class = worker_class.to_s.safe_constantize
 
         labels = create_labels(worker_class, queue, job)
         labels[:scheduling] = job.key?('at') ? 'delayed' : 'immediate'

lib/gitlab/sidekiq_versioning.rb

@@ -3,25 +3,21 @@
 module Gitlab
   module SidekiqVersioning
     def self.install!
-      Sidekiq::Manager.prepend SidekiqVersioning::Manager
-
       # The Sidekiq client API always adds the queue to the Sidekiq queue
       # list, but mail_room and gitlab-shell do not. This is only necessary
       # for monitoring.
-      begin
-        queues = SidekiqConfig.worker_queues
+      queues = SidekiqConfig.worker_queues
 
-        if queues.any?
-          Sidekiq.redis do |conn|
-            conn.pipelined do
-              queues.each do |queue|
-                conn.sadd('queues', queue)
-              end
+      if queues.any?
+        Sidekiq.redis do |conn|
+          conn.pipelined do
+            queues.each do |queue|
+              conn.sadd('queues', queue)
             end
           end
         end
-      rescue ::Redis::BaseError, SocketError, Errno::ENOENT, Errno::EADDRNOTAVAIL, Errno::EAFNOSUPPORT, Errno::ECONNRESET, Errno::ECONNREFUSED
       end
+    rescue ::Redis::BaseError, SocketError, Errno::ENOENT, Errno::EADDRNOTAVAIL, Errno::EAFNOSUPPORT, Errno::ECONNRESET, Errno::ECONNREFUSED
     end
   end
 end

lib/gitlab/sidekiq_versioning/manager.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module SidekiqVersioning
-    module Manager
-      def initialize(options = {})
-        options[:strict] = false
-        options[:queues] = SidekiqConfig.expand_queues(options[:queues])
-        Sidekiq.logger.info "Listening on queues #{options[:queues].uniq.sort}"
-        super
-      end
-    end
-  end
-end

spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do
+RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_gitlab_redis_queues do
   let(:middleware) { described_class.new }
 
   let(:load_balancer) { Gitlab::Database::LoadBalancing.proxy.load_balancer }
@@ -157,18 +157,15 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do
               process_job(job)
             end.to raise_error(Sidekiq::JobRetry::Skip)
 
-            expect(job['error_class']).to eq('Gitlab::Database::LoadBalancing::SidekiqServerMiddleware::JobReplicaNotUpToDate')
+            job_for_retry = Sidekiq::RetrySet.new.first
+            expect(job_for_retry['error_class']).to eq('Gitlab::Database::LoadBalancing::SidekiqServerMiddleware::JobReplicaNotUpToDate')
           end
 
           include_examples 'load balancing strategy', 'retry'
         end
 
         context 'when job is retried' do
-          before do
-            expect do
-              process_job(job)
-            end.to raise_error(Sidekiq::JobRetry::Skip)
-          end
+          let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'database_replica_location' => '0/D525E3A8', 'retry_count' => 0 } }
 
           context 'and replica still lagging behind' do
             include_examples 'stick to the primary', 'primary'
@@ -199,7 +196,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do
   end
 
   def process_job(job)
-    Sidekiq::JobRetry.new.local(worker_class, job, 'default') do
+    Sidekiq::JobRetry.new.local(worker_class, job.to_json, 'default') do
       worker_class.process_job(job)
     end
   end

spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb

@@ -45,7 +45,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::MemoryKiller do
       expect(subject).to receive(:sleep).with(30).ordered
       expect(Process).to receive(:kill).with('SIGTERM', pid).ordered
 
-      expect(subject).to receive(:sleep).with(10).ordered
+      expect(subject).to receive(:sleep).with(Sidekiq.options[:timeout] + 2).ordered
       expect(Process).to receive(:kill).with('SIGKILL', pid).ordered
 
       expect(Sidekiq.logger)

spec/lib/gitlab/sidekiq_migrate_jobs_spec.rb

@@ -65,7 +65,7 @@ RSpec.describe Gitlab::SidekiqMigrateJobs, :clean_gitlab_redis_queues do
                 expect(item).to include('queue' => 'post_receive', 'args' => [i])
               end
 
-              expect(score).to eq(i.succ.hours.from_now.to_i)
+              expect(score).to be_within(schedule_jitter).of(i.succ.hours.from_now.to_i)
             end
           end
         end
@@ -84,7 +84,7 @@ RSpec.describe Gitlab::SidekiqMigrateJobs, :clean_gitlab_redis_queues do
                 expect(item).to include('queue' => 'another_queue', 'args' => [i])
               end
 
-              expect(score).to eq(i.succ.hours.from_now.to_i)
+              expect(score).to be_within(schedule_jitter).of(i.succ.hours.from_now.to_i)
             end
           end
         end
@@ -98,7 +98,7 @@ RSpec.describe Gitlab::SidekiqMigrateJobs, :clean_gitlab_redis_queues do
 
             set_after.each.with_index do |(item, score), i|
               expect(item).to include('queue' => 'new_queue', 'args' => [i])
-              expect(score).to eq(i.succ.hours.from_now.to_i)
+              expect(score).to be_within(schedule_jitter).of(i.succ.hours.from_now.to_i)
             end
           end
         end
@@ -173,6 +173,7 @@ RSpec.describe Gitlab::SidekiqMigrateJobs, :clean_gitlab_redis_queues do
 
     context 'scheduled jobs' do
       let(:set_name) { 'schedule' }
+      let(:schedule_jitter) { 0 }
 
       def create_jobs(include_post_receive: true)
         AuthorizedProjectsWorker.perform_in(1.hour, 0)
@@ -186,12 +187,14 @@ RSpec.describe Gitlab::SidekiqMigrateJobs, :clean_gitlab_redis_queues do
 
     context 'retried jobs' do
       let(:set_name) { 'retry' }
+      # Account for Sidekiq retry jitter
+      # https://github.com/mperham/sidekiq/blob/3575ccb44c688dd08bfbfd937696260b12c622fb/lib/sidekiq/job_retry.rb#L217
+      let(:schedule_jitter) { 10 }
 
       # Try to mimic as closely as possible what Sidekiq will actually
       # do to retry a job.
       def retry_in(klass, time, args)
-        # In Sidekiq 6, this argument will become a JSON string
-        message = { 'class' => klass, 'args' => [args], 'retry' => true }
+        message = { 'class' => klass.name, 'args' => [args], 'retry' => true }.to_json
 
         allow(klass).to receive(:sidekiq_retry_in_block).and_return(proc { time })
 

spec/lib/gitlab/sidekiq_versioning/manager_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::SidekiqVersioning::Manager do
-  before do
-    Sidekiq::Manager.prepend described_class
-  end
-
-  describe '#initialize' do
-    it 'listens on all expanded queues' do
-      manager = Sidekiq::Manager.new(queues: %w[post_receive repository_fork cronjob unknown])
-
-      queues = manager.options[:queues]
-
-      expect(queues).to include('post_receive')
-      expect(queues).to include('repository_fork')
-      expect(queues).to include('cronjob')
-      expect(queues).to include('cronjob:import_stuck_project_import_jobs')
-      expect(queues).to include('cronjob:jira_import_stuck_jira_import_jobs')
-      expect(queues).to include('cronjob:stuck_merge_jobs')
-      expect(queues).to include('unknown')
-    end
-  end
-end

spec/lib/gitlab/sidekiq_versioning_spec.rb

@@ -29,12 +29,6 @@ RSpec.describe Gitlab::SidekiqVersioning, :redis do
   end
 
   describe '.install!' do
-    it 'prepends SidekiqVersioning::Manager into Sidekiq::Manager' do
-      described_class.install!
-
-      expect(Sidekiq::Manager).to include(Gitlab::SidekiqVersioning::Manager)
-    end
-
     it 'registers all versionless and versioned queues with Redis' do
       described_class.install!