Drop the use of namespace in Sidekiq and Mailroom delivery

Changelog: other

config/initializers/sidekiq.rb

@@ -30,10 +30,6 @@ def enable_semi_reliable_fetch_mode?
 # Custom Queues configuration
 queues_config_hash = Gitlab::Redis::Queues.params
 
-unless Gitlab::Utils.to_boolean(ENV['SIDEKIQ_ENQUEUE_NON_NAMESPACED'])
-  queues_config_hash[:namespace] = Gitlab::Redis::Queues::SIDEKIQ_NAMESPACE
-end
-
 enable_json_logs = Gitlab.config.sidekiq.log_format != 'text'
 
 Sidekiq.configure_server do |config|

config/mail_room.yml

@@ -31,7 +31,6 @@
       :delivery_options:
         :redis_url: <%= config[:redis_url].to_json %>
         :redis_db: <%= config[:redis_db] %>
-        :namespace: <%= Gitlab::Redis::Queues::SIDEKIQ_NAMESPACE %>
         :queue: <%= config[:queue] %>
         :worker: <%= config[:worker] %>
         <% if config[:sentinels] %>

db/post_migrate/20231003045342_migrate_sidekiq_namespaced_jobs.rb

+# frozen_string_literal: true
+
+class MigrateSidekiqNamespacedJobs < Gitlab::Database::Migration[2.1]
+  BATCH_SIZE = 1000
+  SORTED_SET_NAMES = %w[schedule retry dead]
+
+  def up
+    SORTED_SET_NAMES.each do |set_name|
+      sorted_set_migrate("resque:gitlab:#{set_name}", set_name)
+    end
+
+    Sidekiq::Queue.all.each do |queue|
+      name = queue.name
+      sidekiq_queue_migrate("resque:gitlab:queue:#{name}", to: "queue:#{name}")
+    end
+  end
+
+  def down
+    SORTED_SET_NAMES.each do |set_name|
+      sorted_set_migrate(set_name, "resque:gitlab:#{set_name}")
+    end
+
+    Sidekiq::Queue.all.each do |queue|
+      name = queue.name
+      sidekiq_queue_migrate("queue:#{name}", to: "resque:gitlab:queue:#{name}")
+    end
+  end
+
+  private
+
+  def sidekiq_queue_migrate(queue_from, to:)
+    Gitlab::Redis::Queues.with do |conn| # rubocop:disable Cop/RedisQueueUsage
+      conn.rpoplpush(queue_from, to) while conn.llen(queue_from) > 0
+    end
+  end
+
+  def sorted_set_migrate(from, to)
+    cursor = '0'
+
+    loop do
+      result = []
+
+      Gitlab::Redis::Queues.with do |redis| # rubocop:disable Cop/RedisQueueUsage
+        cursor, result = redis.zscan(from, cursor, count: BATCH_SIZE)
+
+        next if result.empty?
+
+        redis.multi do |multi|
+          multi.zadd(to, result.map { |k, v| [v, k] })
+          multi.zrem(from, result.map { |k, _v| k })
+        end
+      end
+
+      sleep(0.01)
+
+      break if cursor == '0'
+    end
+  end
+end

db/schema_migrations/20231003045342

+a3a577992319a628fb7b1e8c492b1cb3ef1994d3e91e2af351c7b75a3900144d
\ No newline at end of file

lib/gitlab/patch/sidekiq_scheduled_enq.rb

@@ -15,10 +15,8 @@ def enqueue_jobs(sorted_sets = Sidekiq::Scheduled::SETS)
           # this portion swaps out Sidekiq.redis for Gitlab::Redis::Queues
           Gitlab::Redis::Queues.with do |conn| # rubocop:disable Cop/RedisQueueUsage
             sorted_sets.each do |sorted_set|
-              # adds namespace if `super` polls with a non-namespaced Sidekiq.redis
-              if Gitlab::Utils.to_boolean(ENV['SIDEKIQ_ENQUEUE_NON_NAMESPACED'])
-                sorted_set = "#{Gitlab::Redis::Queues::SIDEKIQ_NAMESPACE}:#{sorted_set}" # rubocop:disable Cop/RedisQueueUsage
-              end
+              # adds namespace since `super` polls with a non-namespaced Sidekiq.redis
+              sorted_set = "#{Gitlab::Redis::Queues::SIDEKIQ_NAMESPACE}:#{sorted_set}" # rubocop:disable Cop/RedisQueueUsage
 
               while !@done && (job = zpopbyscore(conn, keys: [sorted_set], argv: [Time.now.to_f.to_s])) # rubocop:disable Gitlab/ModuleWithInstanceVariables, Lint/AssignmentInCondition
                 Sidekiq::Client.push(Sidekiq.load_json(job)) # rubocop:disable Cop/SidekiqApiUsage
@@ -28,7 +26,6 @@ def enqueue_jobs(sorted_sets = Sidekiq::Scheduled::SETS)
           end
         end
 
-        # calls original enqueue_jobs which may or may not be namespaced depending on SIDEKIQ_ENQUEUE_NON_NAMESPACED
         super
       end
     end

spec/lib/gitlab/mail_room/mail_room_spec.rb

@@ -245,7 +245,6 @@
             delivery_options: {
               redis_url: "localhost",
               redis_db: 99,
-              namespace: "resque:gitlab",
               queue: "default",
               worker: "EmailReceiverWorker",
               sentinels: [{ host: "localhost", port: 1234 }]
@@ -258,7 +257,6 @@
             delivery_options: {
               redis_url: "localhost",
               redis_db: 99,
-              namespace: "resque:gitlab",
               queue: "default",
               worker: "ServiceDeskEmailReceiverWorker",
               sentinels: [{ host: "localhost", port: 1234 }]

spec/lib/gitlab/patch/sidekiq_scheduled_enq_spec.rb

@@ -10,7 +10,7 @@
       allow(Sidekiq).to receive(:load_json).and_return(payload)
 
       # stub data in both namespaces
-      Sidekiq.redis { |c| c.zadd('schedule', 100, 'dummy') }
+      Gitlab::Redis::Queues.with { |c| c.zadd('resque:gitlab:schedule', 100, 'dummy') }
       Gitlab::Redis::Queues.with { |c| c.zadd('schedule', 100, 'dummy') }
     end
 
@@ -26,7 +26,7 @@
       end
 
       Gitlab::Redis::Queues.with do |conn|
-        expect(conn.zcard('schedule')).to eq(0)
+        expect(conn.zcard('resque:gitlab:schedule')).to eq(0)
       end
     end
 
@@ -45,29 +45,13 @@
         end
 
         Gitlab::Redis::Queues.with do |conn|
-          expect(conn.zcard('schedule')).to eq(1)
+          expect(conn.zcard('resque:gitlab:schedule')).to eq(1)
         end
       end
     end
 
-    context 'when both envvar are enabled' do
-      around do |example|
-        # runs the zadd to ensure it goes into namespaced set
-        Sidekiq.redis { |c| c.zadd('schedule', 100, 'dummy') }
-
-        holder = Sidekiq.redis_pool
-
-        # forcibly replace Sidekiq.redis since this is set in config/initializer/sidekiq.rb
-        Sidekiq.redis = Gitlab::Redis::Queues.pool
-
-        example.run
-
-      ensure
-        Sidekiq.redis = holder
-      end
-
+    context 'when SIDEKIQ_ENABLE_DUAL_NAMESPACE_POLLING is enabled' do
       before do
-        stub_env('SIDEKIQ_ENQUEUE_NON_NAMESPACED', 'true')
         stub_env('SIDEKIQ_ENABLE_DUAL_NAMESPACE_POLLING', 'true')
       end
 
@@ -81,7 +65,7 @@
         end
 
         Gitlab::Redis::Queues.with do |conn|
-          expect(conn.zcard('schedule')).to eq(0)
+          expect(conn.zcard('resque:gitlab:schedule')).to eq(0)
         end
       end
     end

spec/migrations/20231003045342_migrate_sidekiq_namespaced_jobs_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe MigrateSidekiqNamespacedJobs, :migration, feature_category: :scalability do
+  before do
+    q1 = instance_double(Sidekiq::Queue, name: 'q1')
+    q2 = instance_double(Sidekiq::Queue, name: 'q2')
+    allow(Sidekiq::Queue).to receive(:all).and_return([q1, q2])
+
+    Gitlab::Redis::Queues.with do |redis|
+      (1..1000).each do |i|
+        redis.zadd('resque:gitlab:schedule', [i, i])
+        redis.zadd('resque:gitlab:retry', [i, i])
+        redis.zadd('resque:gitlab:dead', [i, i])
+      end
+
+      Sidekiq::Queue.all.each do |queue|
+        (1..1000).each do |i|
+          redis.lpush("resque:gitlab:queue:#{queue.name}", i)
+        end
+      end
+    end
+  end
+
+  after do
+    Gitlab::Redis::Queues.with(&:flushdb)
+  end
+
+  it "does not creates default organization if needed" do
+    reversible_migration do |migration|
+      migration.before -> {
+        Gitlab::Redis::Queues.with do |redis|
+          expect(redis.zcard('resque:gitlab:schedule')).to eq(1000)
+          expect(redis.zcard('resque:gitlab:retry')).to eq(1000)
+          expect(redis.zcard('resque:gitlab:dead')).to eq(1000)
+          expect(redis.zcard('schedule')).to eq(0)
+          expect(redis.zcard('retry')).to eq(0)
+          expect(redis.zcard('dead')).to eq(0)
+
+          Sidekiq::Queue.all.each do |queue|
+            expect(redis.llen("resque:gitlab:queue:#{queue.name}")).to eq(1000)
+            expect(redis.llen("queue:#{queue.name}")).to eq(0)
+          end
+        end
+      }
+
+      migration.after -> {
+        # no namespaced keys
+        Gitlab::Redis::Queues.with do |redis|
+          expect(redis.zcard('resque:gitlab:schedule')).to eq(0)
+          expect(redis.zcard('resque:gitlab:retry')).to eq(0)
+          expect(redis.zcard('resque:gitlab:dead')).to eq(0)
+          expect(redis.zcard('schedule')).to eq(1000)
+          expect(redis.zcard('retry')).to eq(1000)
+          expect(redis.zcard('dead')).to eq(1000)
+
+          Sidekiq::Queue.all.each do |queue|
+            expect(redis.llen("resque:gitlab:queue:#{queue.name}")).to eq(0)
+            expect(redis.llen("queue:#{queue.name}")).to eq(1000)
+          end
+        end
+      }
+    end
+  end
+end

spec/support/redis.rb

@@ -3,10 +3,7 @@
 
 RSpec.configure do |config|
   config.after(:each, :redis) do
-    Sidekiq.redis do |connection|
-      connection.redis.flushdb
-    end
-
+    Sidekiq.redis(&:flushdb)
     redis_queues_metadata_cleanup!
   end