Add latest changes from gitlab-org/gitlab@master

12 files changed, 209 insertions, 57 deletions

diff --git a/lib/gitlab/analytics/unique_visits.rb b/lib/gitlab/analytics/unique_visits.rb
index 3ed76c5c7f5..ba0447990b1 100644
--- a/lib/gitlab/analytics/unique_visits.rb
+++ b/lib/gitlab/analytics/unique_visits.rb
@@ -43,7 +43,9 @@ module Gitlab
         keys = TARGET_IDS.map { |target_id| key(target_id, week_of) }
 
         Gitlab::Redis::SharedState.with do |redis|
-          redis.pfcount(*keys)
+          Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
+            redis.pfcount(*keys)
+          end
         end
       end
 
diff --git a/lib/gitlab/discussions_diff/highlight_cache.rb b/lib/gitlab/discussions_diff/highlight_cache.rb
index 75d5a5df74b..4bec6467c1a 100644
--- a/lib/gitlab/discussions_diff/highlight_cache.rb
+++ b/lib/gitlab/discussions_diff/highlight_cache.rb
@@ -36,7 +36,9 @@ module Gitlab
 
           content =
             Redis::Cache.with do |redis|
-              redis.mget(keys)
+              Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
+                redis.mget(keys)
+              end
             end
 
           content.map! do |lines|
@@ -58,7 +60,11 @@ module Gitlab
 
           keys = raw_keys.map { |id| cache_key_for(id) }
 
-          Redis::Cache.with { |redis| redis.del(keys) }
+          Redis::Cache.with do |redis|
+            Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
+              redis.del(keys)
+            end
+          end
         end
 
         def cache_key_for(raw_key)
diff --git a/lib/gitlab/gitaly_client/cleanup_service.rb b/lib/gitlab/gitaly_client/cleanup_service.rb
index e2293d3121a..8836d08a807 100644
--- a/lib/gitlab/gitaly_client/cleanup_service.rb
+++ b/lib/gitlab/gitaly_client/cleanup_service.rb
@@ -13,15 +13,15 @@ module Gitlab
       end
 
       def apply_bfg_object_map_stream(io, &blk)
-        responses = GitalyClient.call(
+        GitalyClient.streaming_call(
           storage,
           :cleanup_service,
           :apply_bfg_object_map_stream,
           build_object_map_enum(io),
           timeout: GitalyClient.long_timeout
-        )
-
-        responses.each(&blk)
+        ) do |response|
+          response.each(&blk)
+        end
       end
 
       private
diff --git a/lib/gitlab/gl_repository.rb b/lib/gitlab/gl_repository.rb
index abd4e847a50..7346de13626 100644
--- a/lib/gitlab/gl_repository.rb
+++ b/lib/gitlab/gl_repository.rb
@@ -43,10 +43,10 @@ module Gitlab
     end
 
     def self.parse(gl_repository)
-      result = ::Gitlab::GlRepository::Identifier.new(gl_repository)
+      identifier = ::Gitlab::GlRepository::Identifier.parse(gl_repository)
 
-      repo_type = result.repo_type
-      container = result.fetch_container!
+      repo_type = identifier.repo_type
+      container = identifier.container
 
       [container, repo_type.project_for(container), repo_type]
     end
diff --git a/lib/gitlab/gl_repository/identifier.rb b/lib/gitlab/gl_repository/identifier.rb
index dc3e7931696..57350b1edb0 100644
--- a/lib/gitlab/gl_repository/identifier.rb
+++ b/lib/gitlab/gl_repository/identifier.rb
@@ -3,71 +3,83 @@
 module Gitlab
   class GlRepository
     class Identifier
-      attr_reader :gl_repository, :repo_type
+      include Gitlab::Utils::StrongMemoize
 
-      def initialize(gl_repository)
-        @gl_repository = gl_repository
-        @segments = gl_repository.split('-')
+      InvalidIdentifier = Class.new(ArgumentError)
 
-        raise_error if segments.size > 3
+      def self.parse(gl_repository)
+        segments = gl_repository&.split('-')
 
-        @repo_type = find_repo_type
-        @container_id = find_container_id
-        @container_class = find_container_class
-      end
+        # gl_repository can either have 2 or 3 segments:
+        #
+        # TODO: convert all 2-segment format to 3-segment:
+        # https://gitlab.com/gitlab-org/gitlab/-/issues/219192
+        identifier = case segments&.size
+                     when 2
+                       TwoPartIdentifier.new(*segments)
+                     when 3
+                       ThreePartIdentifier.new(*segments)
+                     end
+
+        return identifier if identifier&.valid?
 
-      def fetch_container!
-        container_class.find_by_id(container_id)
+        raise InvalidIdentifier, %Q(Invalid GL Repository "#{gl_repository}")
       end
 
-      private
+      # The older 2-segment format, where the container is implied.
+      # eg. project-1, wiki-1
+      class TwoPartIdentifier < Identifier
+        def initialize(repo_type_name, container_id_str)
+          @container_id_str = container_id_str
+          @repo_type_name = repo_type_name
+        end
 
-      attr_reader :segments, :container_class, :container_id
+        private
 
-      def find_repo_type
-        type_name = three_segments_format? ? segments.last : segments.first
-        type = Gitlab::GlRepository.types[type_name]
+        def container_class
+          repo_type.container_class
+        end
+      end
 
-        raise_error unless type
+      # The newer 3-segment format, where the container is explicit
+      # eg. group-1-wiki, project-1-wiki
+      class ThreePartIdentifier < Identifier
+        def initialize(container_type, container_id_str, repo_type_name)
+          @container_id_str = container_id_str
+          @container_type = container_type
+          @repo_type_name = repo_type_name
+        end
 
-        type
-      end
+        private
 
-      def find_container_class
-        if three_segments_format?
-          case segments[0]
+        def container_class
+          case @container_type
           when 'project'
             Project
           when 'group'
             Group
-          else
-            raise_error
           end
-        else
-          repo_type.container_class
         end
       end
 
-      def find_container_id
-        id = Integer(segments[1], 10, exception: false)
-
-        raise_error unless id
+      def repo_type
+        strong_memoize(:repo_type) { Gitlab::GlRepository.types[repo_type_name] }
+      end
 
-        id
+      def container
+        strong_memoize(:container) { container_class.find_by_id(container_id) }
       end
 
-      # gl_repository can either have 2 or 3 segments:
-      # "wiki-1" is the older 2-segment format, where container is implied.
-      # "group-1-wiki" is the newer 3-segment format, including container information.
-      #
-      # TODO: convert all 2-segment format to 3-segment:
-      # https://gitlab.com/gitlab-org/gitlab/-/issues/219192
-      def three_segments_format?
-        segments.size == 3
+      def valid?
+        repo_type.present? && container_class.present? && container_id&.positive?
       end
 
-      def raise_error
-        raise ArgumentError, "Invalid GL Repository \"#{gl_repository}\""
+      private
+
+      attr_reader :container_id_str, :repo_type_name
+
+      def container_id
+        strong_memoize(:container_id) { Integer(container_id_str, 10, exception: false) }
       end
     end
   end
diff --git a/lib/gitlab/import_export/json/streaming_serializer.rb b/lib/gitlab/import_export/json/streaming_serializer.rb
index 1c2f9b6cc17..05b7679e0ff 100644
--- a/lib/gitlab/import_export/json/streaming_serializer.rb
+++ b/lib/gitlab/import_export/json/streaming_serializer.rb
@@ -69,8 +69,16 @@ module Gitlab
             key_preloads = preloads&.dig(key)
             records = records.preload(key_preloads) if key_preloads
 
-            records.find_each(batch_size: batch_size) do |record|
-              items << Raw.new(record.to_json(options))
+            records.in_batches(of: batch_size) do |batch| # rubocop:disable Cop/InBatches
+              # order each batch by its primary key to ensure
+              # consistent and predictable ordering of each exported relation
+              # as additional `WHERE` clauses can impact the order in which data is being
+              # returned by database when no `ORDER` is specified
+              batch = batch.reorder(batch.klass.primary_key)
+
+              batch.each do |record|
+                items << Raw.new(record.to_json(options))
+              end
             end
           end
 
diff --git a/lib/gitlab/instrumentation/redis.rb b/lib/gitlab/instrumentation/redis.rb
index 82b4701872f..4a85a313fd7 100644
--- a/lib/gitlab/instrumentation/redis.rb
+++ b/lib/gitlab/instrumentation/redis.rb
@@ -5,9 +5,9 @@ module Gitlab
     # Aggregates Redis measurements from different request storage sources.
     class Redis
       ActionCable = Class.new(RedisBase)
-      Cache = Class.new(RedisBase)
+      Cache = Class.new(RedisBase).enable_redis_cluster_validation
       Queues = Class.new(RedisBase)
-      SharedState = Class.new(RedisBase)
+      SharedState = Class.new(RedisBase).enable_redis_cluster_validation
 
       STORAGES = [ActionCable, Cache, Queues, SharedState].freeze
 
diff --git a/lib/gitlab/instrumentation/redis_base.rb b/lib/gitlab/instrumentation/redis_base.rb
index 012543e1645..a96ca1f4dea 100644
--- a/lib/gitlab/instrumentation/redis_base.rb
+++ b/lib/gitlab/instrumentation/redis_base.rb
@@ -71,6 +71,16 @@ module Gitlab
           query_time.round(::Gitlab::InstrumentationHelper::DURATION_PRECISION)
         end
 
+        def redis_cluster_validate!(command)
+          RedisClusterValidator.validate!(command) if @redis_cluster_validation
+        end
+
+        def enable_redis_cluster_validation
+          @redis_cluster_validation = true
+
+          self
+        end
+
         private
 
         def request_count_key
diff --git a/lib/gitlab/instrumentation/redis_cluster_validator.rb b/lib/gitlab/instrumentation/redis_cluster_validator.rb
new file mode 100644
index 00000000000..6800e5667f6
--- /dev/null
+++ b/lib/gitlab/instrumentation/redis_cluster_validator.rb
@@ -0,0 +1,106 @@
+# frozen_string_literal: true
+
+require 'rails'
+require 'redis'
+
+module Gitlab
+  module Instrumentation
+    module RedisClusterValidator
+      # Generate with:
+      #
+      # Gitlab::Redis::Cache
+      #   .with { |redis| redis.call('COMMAND') }
+      #   .select { |command| command[3] != command[4] }
+      #   .map { |command| [command[0].upcase, { first: command[3], last: command[4], step: command[5] }] }
+      #   .sort_by(&:first)
+      #   .to_h
+      #
+      MULTI_KEY_COMMANDS = {
+        "BITOP" => { first: 2, last: -1, step: 1 },
+        "BLPOP" => { first: 1, last: -2, step: 1 },
+        "BRPOP" => { first: 1, last: -2, step: 1 },
+        "BRPOPLPUSH" => { first: 1, last: 2, step: 1 },
+        "BZPOPMAX" => { first: 1, last: -2, step: 1 },
+        "BZPOPMIN" => { first: 1, last: -2, step: 1 },
+        "DEL" => { first: 1, last: -1, step: 1 },
+        "EXISTS" => { first: 1, last: -1, step: 1 },
+        "MGET" => { first: 1, last: -1, step: 1 },
+        "MSET" => { first: 1, last: -1, step: 2 },
+        "MSETNX" => { first: 1, last: -1, step: 2 },
+        "PFCOUNT" => { first: 1, last: -1, step: 1 },
+        "PFMERGE" => { first: 1, last: -1, step: 1 },
+        "RENAME" => { first: 1, last: 2, step: 1 },
+        "RENAMENX" => { first: 1, last: 2, step: 1 },
+        "RPOPLPUSH" => { first: 1, last: 2, step: 1 },
+        "SDIFF" => { first: 1, last: -1, step: 1 },
+        "SDIFFSTORE" => { first: 1, last: -1, step: 1 },
+        "SINTER" => { first: 1, last: -1, step: 1 },
+        "SINTERSTORE" => { first: 1, last: -1, step: 1 },
+        "SMOVE" => { first: 1, last: 2, step: 1 },
+        "SUNION" => { first: 1, last: -1, step: 1 },
+        "SUNIONSTORE" => { first: 1, last: -1, step: 1 },
+        "UNLINK" => { first: 1, last: -1, step: 1 },
+        "WATCH" => { first: 1, last: -1, step: 1 }
+      }.freeze
+
+      CrossSlotError = Class.new(StandardError)
+
+      class << self
+        def validate!(command)
+          return unless Rails.env.development? || Rails.env.test?
+          return if allow_cross_slot_commands?
+
+          command_name = command.first.to_s.upcase
+          argument_positions = MULTI_KEY_COMMANDS[command_name]
+
+          return unless argument_positions
+
+          arguments = command.flatten[argument_positions[:first]..argument_positions[:last]]
+
+          key_slots = arguments.each_slice(argument_positions[:step]).map do |args|
+            key_slot(args.first)
+          end
+
+          unless key_slots.uniq.length == 1
+            raise CrossSlotError.new("Redis command #{command_name} arguments hash to different slots. See https://docs.gitlab.com/ee/development/redis.html#multi-key-commands")
+          end
+        end
+
+        # Keep track of the call stack to allow nested calls to work.
+        def allow_cross_slot_commands
+          Thread.current[:allow_cross_slot_commands] ||= 0
+          Thread.current[:allow_cross_slot_commands] += 1
+
+          yield
+        ensure
+          Thread.current[:allow_cross_slot_commands] -= 1
+        end
+
+        private
+
+        def allow_cross_slot_commands?
+          Thread.current[:allow_cross_slot_commands].to_i > 0
+        end
+
+        def key_slot(key)
+          ::Redis::Cluster::KeySlotConverter.convert(extract_hash_tag(key))
+        end
+
+        # This is almost identical to Redis::Cluster::Command#extract_hash_tag,
+        # except that it returns the original string if no hash tag is found.
+        #
+        def extract_hash_tag(key)
+          s = key.index('{')
+
+          return key unless s
+
+          e = key.index('}', s + 1)
+
+          return key unless e
+
+          key[s + 1..e - 1]
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/instrumentation/redis_interceptor.rb b/lib/gitlab/instrumentation/redis_interceptor.rb
index a36aade59c3..725e9939ad9 100644
--- a/lib/gitlab/instrumentation/redis_interceptor.rb
+++ b/lib/gitlab/instrumentation/redis_interceptor.rb
@@ -7,6 +7,9 @@ module Gitlab
     module RedisInterceptor
       def call(*args, &block)
         start = Time.now
+
+        instrumentation_class.redis_cluster_validate!(args.first)
+
         super(*args, &block)
       ensure
         duration = (Time.now - start)
diff --git a/lib/gitlab/set_cache.rb b/lib/gitlab/set_cache.rb
index e891b805879..6ba9ee26634 100644
--- a/lib/gitlab/set_cache.rb
+++ b/lib/gitlab/set_cache.rb
@@ -20,7 +20,10 @@ module Gitlab
 
       with do |redis|
         keys = keys.map { |key| cache_key(key) }
-        unlink_or_delete(redis, keys)
+
+        Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
+          unlink_or_delete(redis, keys)
+        end
       end
     end
 
diff --git a/lib/tasks/cache.rake b/lib/tasks/cache.rake
index c380eb293b5..6af91d473a6 100644
--- a/lib/tasks/cache.rake
+++ b/lib/tasks/cache.rake
@@ -18,7 +18,9 @@ namespace :cache do
               count: REDIS_CLEAR_BATCH_SIZE
             )
 
-            redis.del(*keys) if keys.any?
+            Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
+              redis.del(*keys) if keys.any?
+            end
 
             break if cursor == REDIS_SCAN_START_STOP
           end