diff --git a/app/models/active_session.rb b/app/models/active_session.rb index e42f9eeef238dfe4a401f71becb261b75d622db2..9756e1b7dd3ec1a4200d1dce4a0aadcc5d4f03ab 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -84,7 +84,7 @@ def self.set(user, request) ) Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - redis.pipelined do |pipeline| + Gitlab::Redis::CrossSlot::Pipeline.new(redis).pipelined do |pipeline| pipeline.setex( key_name(user.id, session_private_id), expiry, @@ -135,9 +135,15 @@ def self.destroy_sessions(redis, user, session_ids) redis.srem(lookup_key_name(user.id), session_ids) + session_keys = rack_session_keys(session_ids) Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - redis.del(key_names) - redis.del(rack_session_keys(session_ids)) + if Gitlab::Redis::ClusterUtil.cluster?(redis) + Gitlab::Redis::ClusterUtil.batch_unlink(key_names, redis) + Gitlab::Redis::ClusterUtil.batch_unlink(session_keys, redis) + else + redis.del(key_names) + redis.del(session_keys) + end end end @@ -206,7 +212,13 @@ def self.sessions_from_ids(session_ids) session_keys.each_slice(SESSION_BATCH_SIZE).flat_map do |session_keys_batch| Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - redis.mget(session_keys_batch).compact.map do |raw_session| + raw_sessions = if Gitlab::Redis::ClusterUtil.cluster?(redis) + Gitlab::Redis::ClusterUtil.batch_get(session_keys_batch, redis) + else + redis.mget(session_keys_batch) + end + + raw_sessions.compact.map do |raw_session| load_raw_session(raw_session) end end @@ -249,7 +261,13 @@ def dump found = Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do entry_keys = session_ids.map { |session_id| key_name(user_id, session_id) } - session_ids.zip(redis.mget(entry_keys)).to_h + entries = if Gitlab::Redis::ClusterUtil.cluster?(redis) + Gitlab::Redis::ClusterUtil.batch_get(entry_keys, redis) + else + redis.mget(entry_keys) + end + + session_ids.zip(entries).to_h end found.compact! @@ -258,7 +276,13 @@ def dump fallbacks = Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do entry_keys = missing.map { |session_id| key_name_v1(user_id, session_id) } - missing.zip(redis.mget(entry_keys)).to_h + entries = if Gitlab::Redis::ClusterUtil.cluster?(redis) + Gitlab::Redis::ClusterUtil.batch_get(entry_keys, redis) + else + redis.mget(entry_keys) + end + + missing.zip(entries).to_h end fallbacks.merge(found.compact) diff --git a/app/models/ci/build_trace_chunks/redis_base.rb b/app/models/ci/build_trace_chunks/redis_base.rb index 3b7a844d122507a1c25a13c418a16269206e99fd..5f6b5c30a6ae444c8ce4832958c0b6bf4eb06c96 100644 --- a/app/models/ci/build_trace_chunks/redis_base.rb +++ b/app/models/ci/build_trace_chunks/redis_base.rb @@ -71,7 +71,11 @@ def delete_keys(keys) with_redis do |redis| # https://gitlab.com/gitlab-org/gitlab/-/issues/224171 Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - redis.del(keys) + if Gitlab::Redis::ClusterUtil.cluster?(redis) + Gitlab::Redis::ClusterUtil.batch_unlink(keys, redis) + else + redis.del(keys) + end end end end diff --git a/config/initializers/7_redis.rb b/config/initializers/7_redis.rb index 060d0a8a67bd373a154405e7603d9df2ce35024b..25c2c6aa11f0ad306619c1bd685a8a3ec3f2ec3d 100644 --- a/config/initializers/7_redis.rb +++ b/config/initializers/7_redis.rb @@ -27,6 +27,10 @@ Redis::Cluster::CommandLoader.prepend(Gitlab::Patch::CommandLoader) Redis::Cluster.prepend(Gitlab::Patch::RedisCluster) +if Gitlab::Redis::Workhorse.params[:cluster].present? + raise "Do not configure workhorse with a Redis Cluster as pub/sub commands are not cluster-compatible." +end + # Make sure we initialize a Redis connection pool before multi-threaded # execution starts by # 1. Sidekiq diff --git a/config/initializers/action_cable.rb b/config/initializers/action_cable.rb index 0d2073586be4dc7cca8ce287bceb4eb7d8e4ac35..fb52ac6eb8adc7cc568ad41db5077a4c6b674556 100644 --- a/config/initializers/action_cable.rb +++ b/config/initializers/action_cable.rb @@ -11,6 +11,14 @@ ActionCable::SubscriptionAdapter::Base.prepend(Gitlab::Patch::ActionCableSubscriptionAdapterIdentifier) +using_redis_cluster = begin + Rails.application.config_for(:cable)[:cluster].present? +rescue RuntimeError + # config/cable.yml does not exist, but that is not the purpose of this check +end + +raise "Do not configure cable.yml with a Redis Cluster as ActionCable only works with Redis." if using_redis_cluster + # https://github.com/rails/rails/blob/bb5ac1623e8de08c1b7b62b1368758f0d3bb6379/actioncable/lib/action_cable/subscription_adapter/redis.rb#L18 ActionCable::SubscriptionAdapter::Redis.redis_connector = lambda do |config| args = config.except(:adapter, :channel_prefix) diff --git a/config/redis.yml.example b/config/redis.yml.example index 9d884038af78ef24d35408bb7aa497a94d94f299..a391ae36a65fa0d06ca275872506bd8b5604f1ef 100644 --- a/config/redis.yml.example +++ b/config/redis.yml.example @@ -18,6 +18,11 @@ development: queues_metadata: cluster: - redis://localhost:7001 + shared_state: + cluster: + - redis://localhost:7001 + workhorse: + url: redis://localhost:6379 test: chat: @@ -38,3 +43,10 @@ test: queues_metadata: cluster: - redis://localhost:7001 + shared_state: + cluster: + - redis://localhost:7001 + # pubsub and workhorse are not redis-cluster compatible + # even though they fall-back to shared_state + workhorse: + url: redis://localhost:6379 diff --git a/ee/app/services/elastic/process_bookkeeping_service.rb b/ee/app/services/elastic/process_bookkeeping_service.rb index dbec655f51890ba678d98638c6ea60fc90779b30..a74043278052f6c02d5c0e0b13850d3914065d58 100644 --- a/ee/app/services/elastic/process_bookkeeping_service.rb +++ b/ee/app/services/elastic/process_bookkeeping_service.rb @@ -79,7 +79,11 @@ def clear_tracking! Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do keys = SHARDS.map { |m| [redis_set_key(m), redis_score_key(m)] }.flatten - redis.unlink(*keys) + if Gitlab::Redis::ClusterUtil.cluster?(redis) + Gitlab::Redis::ClusterUtil.batch_unlink(keys, redis) + else + redis.unlink(*keys) + end end end end diff --git a/lib/gitlab/issues/rebalancing/state.rb b/lib/gitlab/issues/rebalancing/state.rb index 12cc5f6e5ddf4d7dde5c242914e2831a0e76cd24..c60dac6f571e5e393bfa6438af86c309bb84bf59 100644 --- a/lib/gitlab/issues/rebalancing/state.rb +++ b/lib/gitlab/issues/rebalancing/state.rb @@ -100,7 +100,7 @@ def remove_current_project_id_cache def refresh_keys_expiration with_redis do |redis| Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - redis.pipelined do |pipeline| + Gitlab::Redis::CrossSlot::Pipeline.new(redis).pipelined do |pipeline| pipeline.expire(issue_ids_key, REDIS_EXPIRY_TIME) pipeline.expire(current_index_key, REDIS_EXPIRY_TIME) pipeline.expire(current_project_key, REDIS_EXPIRY_TIME) diff --git a/lib/gitlab/redis/cluster_util.rb b/lib/gitlab/redis/cluster_util.rb index 5f1f39b523761ff08409352469d6146f31730136..9e307940de3d17141e18e985226cf3e50661982e 100644 --- a/lib/gitlab/redis/cluster_util.rb +++ b/lib/gitlab/redis/cluster_util.rb @@ -26,6 +26,15 @@ def batch_unlink(keys, redis) end expired_count end + + # Redis cluster alternative to mget + def batch_get(keys, redis) + keys.each_slice(1000).flat_map do |subset| + Gitlab::Redis::CrossSlot::Pipeline.new(redis).pipelined do |pipeline| + subset.map { |key| pipeline.get(key) } + end + end + end end end end diff --git a/lib/gitlab/sidekiq_status.rb b/lib/gitlab/sidekiq_status.rb index 778d278146d2f366599d0ec5dba2bfa76a32ae65..ae4aca7ff921b98807c1b789083fd1db74fd9266 100644 --- a/lib/gitlab/sidekiq_status.rb +++ b/lib/gitlab/sidekiq_status.rb @@ -94,8 +94,17 @@ def self.job_status(job_ids) keys = job_ids.map { |jid| key_for(jid) } - with_redis { |redis| redis.mget(*keys) } - .map { |result| !result.nil? } + status = with_redis do |redis| + Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do + if Gitlab::Redis::ClusterUtil.cluster?(redis) + Gitlab::Redis::ClusterUtil.batch_get(keys, redis) + else + redis.mget(*keys) + end + end + end + + status.map { |result| !result.nil? } end # Returns the JIDs that are completed diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index 36fe4a010a0d35b4653fea70b01ca3ab4c740fa6..4f644812aa7068500670ddc222507f3a9ac83059 100644 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -21,6 +21,7 @@ sed -i 's|url:.*$|url: redis://redis:6379|g' config/resque.yml if [[ "$USE_REDIS_CLUSTER" != "false" ]] && [[ "$SETUP_DB" != "false" ]]; then cp config/redis.yml.example config/redis.yml sed -i 's|- .*$|- redis://rediscluster:7001|g' config/redis.yml + sed -i 's|url:.*$|url: redis://redis:6379|g' config/redis.yml fi setup_database_yml diff --git a/spec/lib/gitlab/instrumentation_helper_spec.rb b/spec/lib/gitlab/instrumentation_helper_spec.rb index 698c8a37d48b815753f627b3e74d87b05ce141dc..072a9c5f008944e06a1746d8250641f42c5157d3 100644 --- a/spec/lib/gitlab/instrumentation_helper_spec.rb +++ b/spec/lib/gitlab/instrumentation_helper_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cache, :clean_gitlab_redis_cache, :use_null_store_as_repository_cache, feature_category: :scalability do using RSpec::Parameterized::TableSyntax + include RedisHelpers describe '.add_instrumentation_data', :request_store do let(:payload) { {} } @@ -39,11 +40,18 @@ end context 'when Redis calls are made' do + let_it_be(:redis_store_class) { define_helper_redis_store_class } + + before do # init redis connection with `test` env details + redis_store_class.with(&:ping) + RequestStore.clear! + end + it 'adds Redis data and omits Gitaly data' do stub_rails_env('staging') # to avoid raising CrossSlotError - Gitlab::Redis::Sessions.with { |redis| redis.mset('test-cache', 123, 'test-cache2', 123) } + redis_store_class.with { |redis| redis.mset('test-cache', 123, 'test-cache2', 123) } Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - Gitlab::Redis::Sessions.with { |redis| redis.mget('cache-test', 'cache-test-2') } + redis_store_class.with { |redis| redis.mget('cache-test', 'cache-test-2') } end Gitlab::Redis::Queues.with { |redis| redis.set('test-queues', 321) } diff --git a/spec/lib/gitlab/issues/rebalancing/state_spec.rb b/spec/lib/gitlab/issues/rebalancing/state_spec.rb index b457e0bf1dc99a385111e349b785f2a8e9f359b6..a0ea5fec8ec8679eaeafc12ef25cea3cfa8954c3 100644 --- a/spec/lib/gitlab/issues/rebalancing/state_spec.rb +++ b/spec/lib/gitlab/issues/rebalancing/state_spec.rb @@ -231,8 +231,16 @@ def generate_and_cache_issues_ids(count:, position_offset: 0, position_direction def check_existing_keys index = 0 - # spec only, we do not actually scan keys in the code - recently_finished_keys_count = Gitlab::Redis::SharedState.with { |redis| redis.scan(0, match: "#{described_class::RECENTLY_FINISHED_REBALANCE_PREFIX}:*") }.last.count + cursor = '0' + recently_finished_keys_count = 0 + + # loop to scan since it may run against a Redis Cluster + loop do + # spec only, we do not actually scan keys in the code + cursor, items = Gitlab::Redis::SharedState.with { |redis| redis.scan(cursor, match: "#{described_class::RECENTLY_FINISHED_REBALANCE_PREFIX}:*") } + recently_finished_keys_count += items.count + break if cursor == '0' + end index += 1 if rebalance_caching.get_current_index > 0 index += 1 if rebalance_caching.get_current_project_id.present? diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 7a88b133a98d41a56a3879d59f8ccfabfbbc2ca4..538eb51387ef7f6258f75a0337d881ebd3b19d9c 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -1059,7 +1059,7 @@ def stub_redis_runner_contacted_at(value) end def value_in_queues - Gitlab::Redis::SharedState.with do |redis| + Gitlab::Redis::Workhorse.with do |redis| runner_queue_key = runner.send(:runner_queue_key) redis.get(runner_queue_key) end