Merge branch 'refactor_hll_unique_action_counts' into 'master'

Refactor the HLL unique counting between visits and actions

See merge request gitlab-org/gitlab!36583

lib/gitlab/analytics/unique_visits.rb

@@ -25,26 +25,19 @@ module Gitlab
       def track_visit(visitor_id, target_id, time = Time.zone.now)
         target_key = key(target_id, time)
 
-        Gitlab::Redis::SharedState.with do |redis|
-          redis.multi do |multi|
-            multi.pfadd(target_key, visitor_id)
-            multi.expire(target_key, KEY_EXPIRY_LENGTH)
-          end
-        end
+        Gitlab::Redis::HLL.add(key: target_key, value: visitor_id, expiry: KEY_EXPIRY_LENGTH)
       end
 
       def weekly_unique_visits_for_target(target_id, week_of: 7.days.ago)
-        Gitlab::Redis::SharedState.with do |redis|
-          redis.pfcount(key(target_id, week_of))
-        end
+        target_key = key(target_id, week_of)
+
+        Gitlab::Redis::HLL.count(keys: [target_key])
       end
 
       def weekly_unique_visits_for_any_target(week_of: 7.days.ago)
         keys = TARGET_IDS.select { |id| id =~ /_analytics_/ }.map { |target_id| key(target_id, week_of) }
 
-        Gitlab::Redis::SharedState.with do |redis|
-          redis.pfcount(*keys)
-        end
+        Gitlab::Redis::HLL.count(keys: keys)
       end
 
       private

lib/gitlab/redis/hll.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Redis
+    class HLL
+      def self.count(params)
+        self.new.count(params)
+      end
+
+      def self.add(params)
+        self.new.add(params)
+      end
+
+      # NOTE: It is important to make sure the keys are in the same hash slot
+      # https://redis.io/topics/cluster-spec#keys-hash-tags
+      def count(keys:)
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.pfcount(*keys)
+        end
+      end
+
+      def add(key:, value:, expiry:)
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.multi do |multi|
+            multi.pfadd(key, value)
+            multi.expire(key, expiry)
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/usage_data_counters/track_unique_actions.rb

@@ -35,16 +35,15 @@ module Gitlab
 
           transformed_target = transform_target(event_target)
           transformed_action = transform_action(event_action, transformed_target)
+          target_key = key(transformed_action, time)
 
-          add_event(transformed_action, author_id, time)
+          Gitlab::Redis::HLL.add(key: target_key, value: author_id, expiry: KEY_EXPIRY_LENGTH)
         end
 
         def count_unique_events(event_action:, date_from:, date_to:)
           keys = (date_from.to_date..date_to.to_date).map { |date| key(event_action, date) }
 
-          Gitlab::Redis::SharedState.with do |redis|
-            redis.pfcount(*keys)
-          end
+          Gitlab::Redis::HLL.count(keys: keys)
         end
 
         private
@@ -69,17 +68,6 @@ module Gitlab
           year_day = date.strftime('%G-%j')
           "#{year_day}-{#{event_action}}"
         end
-
-        def add_event(event_action, author_id, date)
-          target_key = key(event_action, date)
-
-          Gitlab::Redis::SharedState.with do |redis|
-            redis.multi do |multi|
-              multi.pfadd(target_key, author_id)
-              multi.expire(target_key, KEY_EXPIRY_LENGTH)
-            end
-          end
-        end
       end
     end
   end