DEV: Upgrade Sidekiq to v7.3.9

Gemfile.lock

@@ -428,6 +428,8 @@ GEM
       psych (>= 4.0.0)
     redcarpet (3.6.1)
     redis (4.8.1)
+    redis-client (0.23.2)
+      connection_pool
     redis-namespace (1.11.0)
       redis (>= 4)
     regexp_parser (2.10.0)
@@ -558,10 +560,12 @@ GEM
       websocket (~> 1.0)
     shoulda-matchers (6.4.0)
       activesupport (>= 5.2.0)
-    sidekiq (6.5.12)
+    sidekiq (7.3.9)
-      connection_pool (>= 2.2.5, < 3)
+      base64
-      rack (~> 2.0)
+      connection_pool (>= 2.3.0)
-      redis (>= 4.5.0, < 5)
+      logger
+      rack (>= 2.2.4)
+      redis-client (>= 0.22.2)
     simplecov (0.22.0)
       docile (~> 1.1)
       simplecov-html (~> 0.11)
@@ -999,6 +1003,7 @@ CHECKSUMS
   rdoc (6.12.0) sha256=7d6f706e070bffa5d18a448f24076cbfb34923a99c1eab842aa18e6ca69f56e0
   redcarpet (3.6.1) sha256=d444910e6aa55480c6bcdc0cdb057626e8a32c054c29e793fa642ba2f155f445
   redis (4.8.1) sha256=387ee086694fffc9632aaeb1efe4a7b1627ca783bf373320346a8a20cd93333a
+  redis-client (0.23.2) sha256=e33bab6682c8155cfef95e6dd296936bb9c2981a89fb578ace27a076fa2836fa
   redis-namespace (1.11.0) sha256=e91a1aa2b2d888b6dea1d4ab8d39e1ae6fac3426161feb9d91dd5cca598a2239
   regexp_parser (2.10.0) sha256=cb6f0ddde88772cd64bff1dbbf68df66d376043fe2e66a9ef77fcb1b0c548c61
   reline (0.6.0) sha256=57620375dcbe56ec09bac7192bfb7460c716bbf0054dc94345ecaa5438e539d2
@@ -1048,7 +1053,7 @@ CHECKSUMS
   selenium-devtools (0.133.0) sha256=aba5a5225ac38d235bbc6ebb4e0b8209e973ac7af4226e4304a1417573f73f64
   selenium-webdriver (4.29.1) sha256=0a7fe53cc4d2c515adbb89e115c6e786c64e9b98f85939d21071c6e32883a146
   shoulda-matchers (6.4.0) sha256=9055bb7f4bb342125fb860809798855c630e05ef5e75837b3168b8e6ee1608b0
-  sidekiq (6.5.12) sha256=b4f93b2204c42220d0b526a7b8e0c49b5f9da82c1ce1a05d2baf1e8f744c197f
+  sidekiq (7.3.9) sha256=1108712e1def89002b28e3545d5ae15d4a57ffd4d2c25d97bb1360988826b5a7
   simplecov (0.22.0) sha256=fe2622c7834ff23b98066bb0a854284b2729a569ac659f82621fc22ef36213a5
   simplecov-html (0.13.1) sha256=5dab0b7ee612e60e9887ad57693832fdf4695b4c0c859eaea5f95c18791ef10b
   simplecov_json_formatter (0.1.4) sha256=529418fbe8de1713ac2b2d612aa3daa56d316975d307244399fa4838c601b428

app/jobs/base.rb

@@ -475,7 +475,7 @@ module Jobs
     Sidekiq::ScheduledSet.new.select do |scheduled_job|
       if scheduled_job.klass.to_s == job_class
         matched = true
-        job_params = scheduled_job.item["args"][0].with_indifferent_access
+        job_params = scheduled_job.args[0].with_indifferent_access
         opts.each do |key, value|
           if job_params[key] != value
             matched = false

config/initializers/100-sidekiq.rb

@@ -66,16 +66,17 @@ else
   #
   # Instead, this patch adds a dedicated logger instance and patches
   # the #add method to forward messages to Rails.logger.
-  Sidekiq.logger = Logger.new(nil)
+  Sidekiq.default_configuration.logger = Logger.new(nil)
   Sidekiq
+    .default_configuration
     .logger
     .define_singleton_method(:add) do |severity, message = nil, progname = nil, &blk|
       Rails.logger.add(severity, message, progname, &blk)
     end
 end
 
-Sidekiq.error_handlers.clear
+Sidekiq.default_configuration.error_handlers.clear
-Sidekiq.error_handlers << SidekiqLogsterReporter.new
+Sidekiq.default_configuration.error_handlers << SidekiqLogsterReporter.new
 
 Sidekiq.strict_args!
 

db/post_migrate/20250227142351_migrate_sidekiq_jobs.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
-# Delete this migration instead of promoting it (along with the
+# TODO: Remove this after the Discourse 3.5 release
-# `SidekiqMigration` class)
 class MigrateSidekiqJobs < ActiveRecord::Migration[7.2]
   def up
     SidekiqMigration.call

lib/backup_restore/system_interface.rb

@@ -115,7 +115,7 @@ module BackupRestore
 
     def sidekiq_has_running_jobs?
       Sidekiq::Workers.new.each do |_, _, work|
-        args = work&.dig("payload", "args")&.first
+        args = work.job.args&.first
         current_site_id = args["current_site_id"] if args.present?
 
         return true if current_site_id.blank? || current_site_id == @current_db

lib/discourse.rb

@@ -210,7 +210,7 @@ module Discourse
     return if ex.class == Jobs::HandledExceptionWrapper
 
     context ||= {}
-    parent_logger ||= Sidekiq
+    parent_logger ||= Sidekiq.default_configuration
 
     job = context[:job]
 
@@ -930,10 +930,7 @@ module Discourse
     Rails.cache.reconnect
     Discourse.cache.reconnect
     Logster.store.redis.reconnect
-    # shuts down all connections in the pool
+    Sidekiq.redis_pool.reload(&:close)
-    Sidekiq.redis_pool.shutdown { |conn| conn.disconnect! }
-    # re-establish
-    Sidekiq.redis = sidekiq_redis_config
 
     # in case v8 was initialized we want to make sure it is nil
     PrettyText.reset_context
@@ -1048,9 +1045,11 @@ module Discourse
   SIDEKIQ_NAMESPACE = "sidekiq"
 
   def self.sidekiq_redis_config(old: false)
-    redis_config = GlobalSetting.redis_config.dup
+    GlobalSetting
-    return redis_config.merge(namespace: SIDEKIQ_NAMESPACE) if old
+      .redis_config
-    redis_config.merge(db: redis_config[:db].to_i + 1)
+      .dup
+      .except(:connector, :replica_host, :replica_port)
+      .tap { |config| config.merge!(db: config[:db].to_i + 1) unless old }
   end
 
   def self.static_doc_topic_ids

lib/freedom_patches/sidekiq.rb

@@ -1,14 +1,17 @@
 # frozen_string_literal: true
 
-# TODO (2025-03-03): delete this once the migration is propagated everywhere
+# TODO: Remove this after the Discourse 3.5 release
-# (in about 6 months or so)
 module Sidekiq
-  def self.redis_pool
-    @redis ||= RedisConnection.create
-    Thread.current[:sidekiq_via_pool] || @redis
-  end
-
   def self.old_pool
-    @old_pool ||= RedisConnection.create(Discourse.sidekiq_redis_config(old: true))
+    @old_pool ||=
+      begin
+        ConnectionPool.new do
+          Redis::Namespace.new(
+            Discourse::SIDEKIQ_NAMESPACE,
+            redis:
+              Sidekiq::RedisClientAdapter.new(Discourse.sidekiq_redis_config(old: true)).new_client,
+          )
+        end
+      end
   end
 end

lib/sidekiq_logster_reporter.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class SidekiqLogsterReporter
-  def call(ex, context = {})
+  def call(ex, context = {}, _config = nil)
     return if Jobs::HandledExceptionWrapper === ex
     Discourse.reset_active_record_cache_if_needed(ex)
 

lib/sidekiq_long_running_job_logger.rb

@@ -28,19 +28,18 @@ class SidekiqLongRunningJobLogger
               Sidekiq::Workers.new.each do |process_id, thread_id, work|
                 next unless process_id.start_with?(hostname)
 
-                if Time.at(work["run_at"]).to_i >=
+                if Time.at(work.run_at).to_i >= (Time.now - (60 * @stuck_sidekiq_job_minutes)).to_i
-                     (Time.now - (60 * @stuck_sidekiq_job_minutes)).to_i
                   next
                 end
 
-                jid = work.dig("payload", "jid")
+                jid = work.job.jid
                 current_long_running_jobs << jid
 
                 next if @seen_long_running_jobs&.include?(jid)
 
                 if thread = Thread.list.find { |t| t["sidekiq_tid"] == thread_id }
                   Rails.logger.warn(<<~MSG)
-                Sidekiq job `#{work.dig("payload", "class")}` has been running for more than #{@stuck_sidekiq_job_minutes} minutes
+                Sidekiq job `#{work.job.klass}` has been running for more than #{@stuck_sidekiq_job_minutes} minutes
                 #{thread.backtrace.join("\n")}
                 MSG
                 end

lib/sidekiq_migration.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
-# TODO (2025-03-03): delete this once the migration is propagated everywhere
+# TODO: Remove this after the Discourse 3.5 release
-# (in about 6 months or so)
 class SidekiqMigration
   delegate :old_pool, to: :Sidekiq
 

spec/lib/backup_restore/system_interface_spec.rb

@@ -105,11 +105,7 @@ RSpec.describe BackupRestore::SystemInterface do
     end
 
     context "with Sidekiq workers" do
-      after { flush_sidekiq_redis_namespace }
+      after { Sidekiq.redis(&:flushdb) }
-
-      def flush_sidekiq_redis_namespace
-        Sidekiq.redis { |redis| redis.scan_each { |key| redis.del(key) } }
-      end
 
       def create_workers(site_id: nil, all_sites: false)
         payload =
@@ -132,8 +128,8 @@ RSpec.describe BackupRestore::SystemInterface do
           key = "#{hostname}:#{pid}"
           process = { pid: pid, hostname: hostname }
 
-          conn.sadd?("processes", key)
+          conn.sadd("processes", key)
-          conn.hmset(key, "info", Sidekiq.dump_json(process))
+          conn.hset(key, "info", Sidekiq.dump_json(process))
 
           data =
             Sidekiq.dump_json(
@@ -141,7 +137,7 @@ RSpec.describe BackupRestore::SystemInterface do
               run_at: Time.now.to_i,
               payload: Sidekiq.dump_json(payload),
             )
-          conn.hmset("#{key}:work", "444", data)
+          conn.hset("#{key}:work", "444", data)
         end
       end
 

spec/lib/discourse_spec.rb

@@ -362,7 +362,7 @@ RSpec.describe Discourse do
     class TempSidekiqLogger
       attr_accessor :exception, :context
 
-      def call(ex, ctx)
+      def call(ex, ctx, _config)
         self.exception = ex
         self.context = ctx
       end
@@ -370,9 +370,9 @@ RSpec.describe Discourse do
 
     let!(:logger) { TempSidekiqLogger.new }
 
-    before { Sidekiq.error_handlers << logger }
+    before { Sidekiq.default_configuration.error_handlers << logger }
 
-    after { Sidekiq.error_handlers.delete(logger) }
+    after { Sidekiq.default_configuration.error_handlers.delete(logger) }
 
     describe "#job_exception_stats" do
       class FakeTestError < StandardError

spec/lib/sidekiq_long_running_job_logger_spec.rb

@@ -20,24 +20,26 @@ RSpec.describe SidekiqLongRunningJobLogger do
           [
             "#{hostname}:1234",
             "some_sidekiq_id",
-            {
+            Sidekiq::Work.new(
-              "run_at" => (Time.now - (60 * (stuck_sidekiq_job_minutes + 1))).to_i,
+              "#{hostname}:1234",
-              "payload" => {
+              "some_sidekiq_id",
-                "jid" => "job_1",
+              {
-                "class" => "TestWorker",
+                "run_at" => (Time.now - (60 * (stuck_sidekiq_job_minutes + 1))).to_i,
+                "payload" => Sidekiq.dump_json({ "jid" => "job_1", "class" => "TestWorker" }),
               },
-            },
+            ),
           ],
           [
             "#{hostname}:1234",
             "some_other_sidekiq_id",
-            {
+            Sidekiq::Work.new(
-              "run_at" => Time.now.to_i,
+              "#{hostname}:1234",
-              "payload" => {
+              "some_other_sidekiq_id",
-                "jid" => "job_2",
+              {
-                "class" => "AnotherWorker",
+                "run_at" => Time.now.to_i,
+                "payload" => Sidekiq.dump_json({ "jid" => "job_2", "class" => "AnotherWorker" }),
               },
-            },
+            ),
           ],
         ],
       )

spec/rails_helper.rb

@@ -288,7 +288,7 @@ RSpec.configure do |config|
       raise "There are pending migrations, run RAILS_ENV=test bin/rake db:migrate"
     end
 
-    Sidekiq.error_handlers.clear
+    Sidekiq.default_configuration.error_handlers.clear
 
     # Ugly, but needed until we have a user creator
     User.skip_callback(:create, :after, :ensure_in_trust_level_group)