Merge branch 'sc1-redis-gem-upgrade-3rd-try' into 'master'

Upgrade redis gem - 3rd attempt

See merge request !145203



Merged-by: Dat Tang <dattang@gitlab.com>
Approved-by: Stan Hu <stanhu@gmail.com>
Approved-by: Sam Word <sword@gitlab.com>
Approved-by: Igor Drozdov <idrozdov@gitlab.com>
Approved-by: Dat Tang <dattang@gitlab.com>
Approved-by: charlie ablett <cablett@gitlab.com>
Approved-by: Adam Hegyi <ahegyi@gitlab.com>
Co-authored-by: Sylvester Chin <schin@gitlab.com>

.rubocop_todo/layout/line_length.yml

@@ -3698,7 +3698,6 @@ Layout/LineLength:
     - 'spec/lib/gitlab/import_export/uploads_manager_spec.rb'
     - 'spec/lib/gitlab/import_export/version_checker_spec.rb'
     - 'spec/lib/gitlab/import_sources_spec.rb'
-    - 'spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb'
     - 'spec/lib/gitlab/issuable_metadata_spec.rb'
     - 'spec/lib/gitlab/issues/rebalancing/state_spec.rb'
     - 'spec/lib/gitlab/jira/dvcs_spec.rb'

.rubocop_todo/lint/ambiguous_operator_precedence.yml

@@ -102,7 +102,6 @@ Lint/AmbiguousOperatorPrecedence:
     - 'spec/lib/gitlab/database/batch_count_spec.rb'
     - 'spec/lib/gitlab/database/consistency_checker_spec.rb'
     - 'spec/lib/gitlab/graphql/tracers/metrics_tracer_spec.rb'
-    - 'spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb'
     - 'spec/lib/gitlab/issues/rebalancing/state_spec.rb'
     - 'spec/lib/gitlab/kroki_spec.rb'
     - 'spec/lib/gitlab/memory/instrumentation_spec.rb'

.rubocop_todo/rspec/feature_category.yml

@@ -3585,7 +3585,6 @@ RSpec/FeatureCategory:
     - 'spec/lib/gitlab/instrumentation/rate_limiting_gates_spec.rb'
     - 'spec/lib/gitlab/instrumentation/redis_base_spec.rb'
     - 'spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb'
-    - 'spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb'
     - 'spec/lib/gitlab/instrumentation/redis_spec.rb'
     - 'spec/lib/gitlab/internal_post_receive/response_spec.rb'
     - 'spec/lib/gitlab/issuable/clone/attributes_rewriter_spec.rb'

.rubocop_todo/rspec/named_subject.yml

@@ -2330,7 +2330,6 @@ RSpec/NamedSubject:
     - 'spec/lib/gitlab/rack_attack_spec.rb'
     - 'spec/lib/gitlab/reactive_cache_set_cache_spec.rb'
     - 'spec/lib/gitlab/redis/boolean_spec.rb'
-    - 'spec/lib/gitlab/redis/cross_slot_spec.rb'
     - 'spec/lib/gitlab/redis/db_load_balancing_spec.rb'
     - 'spec/lib/gitlab/redis/multi_store_spec.rb'
     - 'spec/lib/gitlab/redis/queues_spec.rb'

.rubocop_todo/style/inline_disable_annotation.yml

@@ -2514,7 +2514,6 @@ Style/InlineDisableAnnotation:
     - 'lib/gitlab/import_export/project/relation_factory.rb'
     - 'lib/gitlab/import_sources.rb'
     - 'lib/gitlab/instrumentation/redis_cluster_validator.rb'
-    - 'lib/gitlab/instrumentation/redis_interceptor.rb'
     - 'lib/gitlab/internal_events.rb'
     - 'lib/gitlab/issuable/clone/copy_resource_events_service.rb'
     - 'lib/gitlab/issues/rebalancing/state.rb'
@@ -2553,7 +2552,6 @@ Style/InlineDisableAnnotation:
     - 'lib/gitlab/pagination/offset_pagination.rb'
     - 'lib/gitlab/pagination_delegate.rb'
     - 'lib/gitlab/patch/action_cable_subscription_adapter_identifier.rb'
-    - 'lib/gitlab/patch/node_loader.rb'
     - 'lib/gitlab/patch/prependable.rb'
     - 'lib/gitlab/patch/redis_cache_store.rb'
     - 'lib/gitlab/patch/sidekiq_cron_poller.rb'
@@ -2570,7 +2568,6 @@ Style/InlineDisableAnnotation:
     - 'lib/gitlab/rack_attack.rb'
     - 'lib/gitlab/rack_attack/request.rb'
     - 'lib/gitlab/rack_attack/store.rb'
-    - 'lib/gitlab/redis/cross_slot.rb'
     - 'lib/gitlab/redis/hll.rb'
     - 'lib/gitlab/redis/multi_store.rb'
     - 'lib/gitlab/reference_extractor.rb'
@@ -2938,9 +2935,7 @@ Style/InlineDisableAnnotation:
     - 'spec/lib/gitlab/pagination/keyset/order_spec.rb'
     - 'spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb'
     - 'spec/lib/gitlab/patch/database_config_spec.rb'
-    - 'spec/lib/gitlab/patch/node_loader_spec.rb'
     - 'spec/lib/gitlab/quick_actions/dsl_spec.rb'
-    - 'spec/lib/gitlab/redis/cross_slot_spec.rb'
     - 'spec/lib/gitlab/redis/multi_store_spec.rb'
     - 'spec/lib/gitlab/search/abuse_detection_spec.rb'
     - 'spec/lib/gitlab/shard_health_cache_spec.rb'

Gemfile

@@ -288,8 +288,9 @@ gem 'js_regex', '~> 3.8' # rubocop:todo Gemfile/MissingFeatureCategory
 gem 'device_detector' # rubocop:todo Gemfile/MissingFeatureCategory
 
 # Redis
-gem 'redis', '~> 4.8.0' # rubocop:todo Gemfile/MissingFeatureCategory
-gem 'redis-namespace', '~> 1.10.0' # rubocop:todo Gemfile/MissingFeatureCategory
+gem 'redis-namespace', '~> 1.10.0', feature_category: :redis
+gem 'redis', '~> 5.0.0', feature_category: :redis
+gem 'redis-clustering', '~> 5.0.0', feature_category: :redis
 gem 'connection_pool', '~> 2.4' # rubocop:todo Gemfile/MissingFeatureCategory
 
 # Redis session store

Gemfile.checksum

@@ -526,9 +526,11 @@
 {"name":"recaptcha","version":"5.12.3","platform":"ruby","checksum":"37d1894add9e70a54d0c6c7f0ecbeedffbfa7d075acfbd4c509818dfdebdb7ee"},
 {"name":"recursive-open-struct","version":"1.1.3","platform":"ruby","checksum":"a3538a72552fcebcd0ada657bdff313641a4a5fbc482c08cfb9a65acb1c9de5a"},
 {"name":"redcarpet","version":"3.6.0","platform":"ruby","checksum":"8ad1889c0355ff4c47174af14edd06d62f45a326da1da6e8a121d59bdcd2e9e9"},
-{"name":"redis","version":"4.8.0","platform":"ruby","checksum":"2000cf5014669c9dc821704b6d322a35a9a33852a95208911d9175d63b448a44"},
+{"name":"redis","version":"5.0.8","platform":"ruby","checksum":"3b770ea597850b26d6a9718fa184241e53e6c8a7ae0486ee8bfaefd29f26f3d8"},
 {"name":"redis-actionpack","version":"5.4.0","platform":"ruby","checksum":"f10cf649ab05914716d63334d7f709221ecc883b87cf348f90ecfe0c35ea3540"},
 {"name":"redis-client","version":"0.20.0","platform":"ruby","checksum":"239ac38fe4f0b62c8d2d8641989319b736b7dd40eebec868fd874a14686bdc8c"},
+{"name":"redis-cluster-client","version":"0.7.5","platform":"ruby","checksum":"12fd1c9eda17157a5cd2ce46afba13a024c28d24922092299a8daa9f46e4e78a"},
+{"name":"redis-clustering","version":"5.0.8","platform":"ruby","checksum":"8e2f3de3b1a700668eeac59125636e01be6ecd985e635a4d5649c47d71f6e166"},
 {"name":"redis-namespace","version":"1.10.0","platform":"ruby","checksum":"2c1c6ea7c6c5e343e75b9bee3aa4c265e364a5b9966507397467af2bb3758d94"},
 {"name":"redis-rack","version":"3.0.0","platform":"ruby","checksum":"abb50b82ae10ad4d11ca2e4901bfc2b98256cdafbbd95f80c86fc9e001478380"},
 {"name":"redis-store","version":"1.10.0","platform":"ruby","checksum":"f258894f9f7e82834308a3d86242294f0cff2c9db9ae66e5cb4c553a5ec8b09e"},

Gemfile.lock

@@ -1385,13 +1385,19 @@ GEM
       json
     recursive-open-struct (1.1.3)
     redcarpet (3.6.0)
-    redis (4.8.0)
+    redis (5.0.8)
+      redis-client (>= 0.17.0)
     redis-actionpack (5.4.0)
       actionpack (>= 5, < 8)
       redis-rack (>= 2.1.0, < 4)
       redis-store (>= 1.1.0, < 2)
     redis-client (0.20.0)
       connection_pool
+    redis-cluster-client (0.7.5)
+      redis-client (~> 0.12)
+    redis-clustering (5.0.8)
+      redis (= 5.0.8)
+      redis-cluster-client (>= 0.7.0)
     redis-namespace (1.10.0)
       redis (>= 4)
     redis-rack (3.0.0)
@@ -2069,8 +2075,9 @@ DEPENDENCIES
   rbtrace (~> 0.4)
   re2 (= 2.7.0)
   recaptcha (~> 5.12)
-  redis (~> 4.8.0)
+  redis (~> 5.0.0)
   redis-actionpack (~> 5.4.0)
+  redis-clustering (~> 5.0.0)
   redis-namespace (~> 1.10.0)
   request_store (~> 1.5.1)
   responders (~> 3.0)

app/models/active_session.rb

@@ -90,7 +90,7 @@ def self.set(user, request)
       )
 
       Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
-        Gitlab::Redis::CrossSlot::Pipeline.new(redis).pipelined do |pipeline|
+        redis.pipelined do |pipeline|
           pipeline.setex(
             key_name(user.id, session_private_id),
             expiry,

app/workers/concerns/limited_capacity/job_tracker.rb

@@ -21,7 +21,7 @@ def initialize(namespace)
 
     def register(jid, max_jids)
       with_redis do |redis|
-        redis.eval(LUA_REGISTER_SCRIPT, keys: [counter_key], argv: [jid, max_jids])
+        redis.eval(LUA_REGISTER_SCRIPT, keys: [counter_key], argv: [jid.to_s, max_jids.to_i])
       end.present?
     end
 
@@ -59,7 +59,7 @@ def counter_key
     end
 
     def remove_job_keys(redis, keys)
-      redis.srem?(counter_key, keys)
+      redis.srem?(counter_key, keys) if keys.present?
     end
 
     def with_redis(&block)

config/initializers/7_redis.rb

@@ -21,12 +21,6 @@
 # :nocov:
 # rubocop:enable Gitlab/NoCodeCoverageComment
 
-Redis::Client.prepend(Gitlab::Instrumentation::RedisInterceptor)
-Redis::Cluster::NodeLoader.prepend(Gitlab::Patch::NodeLoader)
-Redis::Cluster::SlotLoader.prepend(Gitlab::Patch::SlotLoader)
-Redis::Cluster::CommandLoader.prepend(Gitlab::Patch::CommandLoader)
-Redis::Cluster.prepend(Gitlab::Patch::RedisCluster)
-
 # this only instruments `RedisClient` used in `Sidekiq.redis`
 RedisClient.register(Gitlab::Instrumentation::RedisClientMiddleware)
 RedisClient.prepend(Gitlab::Patch::RedisClient)

config/initializers/action_cable.rb

@@ -25,7 +25,7 @@
 # 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)
-    .merge(instrumentation_class: ::Gitlab::Instrumentation::Redis::ActionCable)
+    .merge(custom: { instrumentation_class: "ActionCable" })
 
   ::Redis.new(args)
 end

config/initializers/peek.rb

@@ -6,7 +6,7 @@
 Peek.singleton_class.prepend ::Gitlab::PerformanceBar::WithTopLevelWarnings
 
 Rails.application.config.peek.adapter = :redis, {
-  client: ::Redis.new(Gitlab::Redis::Cache.params),
+  client: Gitlab::Redis::Cache.redis,
   expires_in: 5.minutes
 }
 

config/initializers/session_store.rb

@@ -29,12 +29,12 @@
                "_gitlab_session"
              end
 
-store = Gitlab::Redis::Sessions.store(namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE)
+::Redis::Store::Factory.prepend(Gitlab::Patch::RedisStoreFactory)
 
 Rails.application.configure do
   config.session_store(
     :redis_store, # Using the cookie_store would enable session replay attacks.
-    redis_store: store,
+    redis_server: Gitlab::Redis::Sessions.params.merge(namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE),
     key: cookie_key,
     secure: Gitlab.config.gitlab.https,
     httponly: true,

config/initializers/sidekiq.rb

@@ -16,7 +16,13 @@ def load_cron_jobs!
 end
 
 # Custom Queues configuration
-queues_config_hash = Gitlab::Redis::Queues.redis_client_params
+#
+# We omit :command_builder since Sidekiq::RedisConnection performs a deep clone using
+# Marshal.load(Marshal.dump(options.slice(*keys))) on the Redis config and Gitlab::Redis::CommandBuilder
+# can't be referred to.
+#
+# We do not need the custom command builder since Sidekiq will handle the typing of Redis arguments.
+queues_config_hash = Gitlab::Redis::Queues.params.except(:command_builder)
 
 enable_json_logs = Gitlab.config.sidekiq.log_format != 'text'
 

doc/administration/redis/troubleshooting.md

@@ -124,7 +124,7 @@ To make sure your configuration is correct:
 1. Run in the console:
 
    ```ruby
-   redis = Redis.new(Gitlab::Redis::SharedState.params)
+   redis = Gitlab::Redis::SharedState.redis
    redis.info
    ```
 

doc/development/redis.md

@@ -81,10 +81,10 @@ Developers are highly encouraged to use [hash-tags](https://redis.io/docs/refere
 where appropriate to facilitate future adoption of Redis Cluster in more Redis types. For example, the Namespace model uses hash-tags
 for its [config cache keys](https://gitlab.com/gitlab-org/gitlab/-/blob/1a12337058f260d38405886d82da5e8bb5d8da0b/app/models/namespace.rb#L786).
 
-To perform multi-key commands, developers may use the [`Gitlab::Redis::CrossSlot::Pipeline`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/redis/cross_slot.rb) wrapper.
+To perform multi-key commands, developers may use the [`.pipelined`](https://github.com/redis-rb/redis-cluster-client#interfaces) method which splits and sends commands to each node and aggregates replies.
 However, this does not work for [transactions](https://redis.io/docs/interact/transactions/) as Redis Cluster does not support cross-slot transactions.
 
-For `Rails.cache`, we handle the `MGET` command found in `read_multi_get` by [patching it](https://gitlab.com/gitlab-org/gitlab/-/blob/c2bad2aac25e2f2778897bd4759506a72b118b15/lib/gitlab/patch/redis_cache_store.rb#L10) to use the `Gitlab::Redis::CrossSlot::Pipeline` wrapper.
+For `Rails.cache`, we handle the `MGET` command found in `read_multi_get` by [patching it](https://gitlab.com/gitlab-org/gitlab/-/blob/c2bad2aac25e2f2778897bd4759506a72b118b15/lib/gitlab/patch/redis_cache_store.rb#L10) to use the `.pipelined` method.
 The minimum size of the pipeline is set to 1000 commands and it can be adjusted by using the `GITLAB_REDIS_CLUSTER_PIPELINE_BATCH_LIMIT` environment variable.
 
 ## Redis in structured logging

ee/app/services/elastic/indexing_control_service.rb

@@ -68,7 +68,7 @@ def resume_processing!
 
         Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
           if queue_size == 0
-            Gitlab::Redis::CrossSlot::Pipeline.new(redis).pipelined do |p|
+            redis.pipelined do |p|
               p.del(redis_score_key)
               p.del(redis_set_key)
             end

lib/gitlab/background_migration/redis/backfill_project_pipeline_status_ttl.rb

@@ -14,7 +14,7 @@ def perform(keys)
           ttl_jitter = 2.hours.to_i
 
           Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
-            Gitlab::Redis::CrossSlot::Pipeline.new(redis).pipelined do |pipeline|
+            redis.pipelined do |pipeline|
               keys.each { |key| pipeline.expire(key, ttl_duration + rand(-ttl_jitter..ttl_jitter)) }
             end
           end
@@ -25,7 +25,7 @@ def scan_match_pattern
         end
 
         def redis
-          @redis ||= ::Redis.new(Gitlab::Redis::Cache.params)
+          @redis ||= Gitlab::Redis::Cache.redis
         end
       end
     end

lib/gitlab/cache/import/caching.rb

@@ -139,7 +139,7 @@ def self.set_includes?(raw_key, value)
           key = cache_key_for(raw_key)
 
           with_redis do |redis|
-            redis.sismember(key, value)
+            redis.sismember(key, value || value.to_s)
           end
         end
 
@@ -162,7 +162,7 @@ def self.values_from_set(raw_key)
         def self.write_multiple(mapping, key_prefix: nil, timeout: TIMEOUT)
           with_redis do |redis|
             Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
-              Gitlab::Redis::CrossSlot::Pipeline.new(redis).pipelined do |pipeline|
+              redis.pipelined do |pipeline|
                 mapping.each do |raw_key, value|
                   key = cache_key_for("#{key_prefix}#{raw_key}")