From 619d0648448154c4f53e00dae4d15e5ec67d0187 Mon Sep 17 00:00:00 2001
From: Sylvester Chin <schin@gitlab.com>
Date: Tue, 24 Oct 2023 08:32:02 +0000
Subject: [PATCH] Drop the use of namespace in Sidekiq and Mailroom delivery
Changelog: other
---
config/initializers/sidekiq.rb | 4 --
config/mail_room.yml | 1 -
...3045342_migrate_sidekiq_namespaced_jobs.rb | 59 ++++++++++++++++
db/schema_migrations/20231003045342 | 1 +
lib/gitlab/patch/sidekiq_scheduled_enq.rb | 7 +-
spec/lib/gitlab/mail_room/mail_room_spec.rb | 2 -
.../patch/sidekiq_scheduled_enq_spec.rb | 26 ++-----
...42_migrate_sidekiq_namespaced_jobs_spec.rb | 67 +++++++++++++++++++
spec/support/redis.rb | 5 +-
9 files changed, 135 insertions(+), 37 deletions(-)
create mode 100644 db/post_migrate/20231003045342_migrate_sidekiq_namespaced_jobs.rb
create mode 100644 db/schema_migrations/20231003045342
create mode 100644 spec/migrations/20231003045342_migrate_sidekiq_namespaced_jobs_spec.rb
diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb
index 57850e4e35c48..9b6a9b179359e 100644
--- a/config/initializers/sidekiq.rb
+++ b/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|
diff --git a/config/mail_room.yml b/config/mail_room.yml
index b453ed8ce35c7..355df13fd6123 100644
--- a/config/mail_room.yml
+++ b/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] %>
diff --git a/db/post_migrate/20231003045342_migrate_sidekiq_namespaced_jobs.rb b/db/post_migrate/20231003045342_migrate_sidekiq_namespaced_jobs.rb
new file mode 100644
index 0000000000000..7d4d6876848be
--- /dev/null
+++ b/db/post_migrate/20231003045342_migrate_sidekiq_namespaced_jobs.rb
@@ -0,0 +1,59 @@
+# 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
diff --git a/db/schema_migrations/20231003045342 b/db/schema_migrations/20231003045342
new file mode 100644
index 0000000000000..cf16a592ca62c
--- /dev/null
+++ b/db/schema_migrations/20231003045342
@@ -0,0 +1 @@
+a3a577992319a628fb7b1e8c492b1cb3ef1994d3e91e2af351c7b75a3900144d
\ No newline at end of file
diff --git a/lib/gitlab/patch/sidekiq_scheduled_enq.rb b/lib/gitlab/patch/sidekiq_scheduled_enq.rb
index de0e8465f97a3..b5a40c199234d 100644
--- a/lib/gitlab/patch/sidekiq_scheduled_enq.rb
+++ b/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
diff --git a/spec/lib/gitlab/mail_room/mail_room_spec.rb b/spec/lib/gitlab/mail_room/mail_room_spec.rb
index 7259b5e2484cc..d3ddf034cd32a 100644
--- a/spec/lib/gitlab/mail_room/mail_room_spec.rb
+++ b/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 }]
diff --git a/spec/lib/gitlab/patch/sidekiq_scheduled_enq_spec.rb b/spec/lib/gitlab/patch/sidekiq_scheduled_enq_spec.rb
index f57257cd1c019..cd3718f5dccc3 100644
--- a/spec/lib/gitlab/patch/sidekiq_scheduled_enq_spec.rb
+++ b/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
diff --git a/spec/migrations/20231003045342_migrate_sidekiq_namespaced_jobs_spec.rb b/spec/migrations/20231003045342_migrate_sidekiq_namespaced_jobs_spec.rb
new file mode 100644
index 0000000000000..9e170ff33a4ab
--- /dev/null
+++ b/spec/migrations/20231003045342_migrate_sidekiq_namespaced_jobs_spec.rb
@@ -0,0 +1,67 @@
+# 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
diff --git a/spec/support/redis.rb b/spec/support/redis.rb
index ee6f00e4b63da..9cf5c44de98ad 100644
--- a/spec/support/redis.rb
+++ b/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
--
GitLab