[FSSDK-11902] Fix concurrency bug in cmab service

lib/optimizely/cmab/cmab_service.rb

@@ -20,6 +20,7 @@
 require 'digest'
 require 'json'
 require 'securerandom'
+require 'murmurhash3'
 
 module Optimizely
   CmabDecision = Struct.new(:variation_id, :cmab_uuid, keyword_init: true)
@@ -35,13 +36,34 @@ class DefaultCmabService
     #
     # @raise [ArgumentError] If cmab_cache is not an instance of LRUCache.
     # @raise [ArgumentError] If cmab_client is not an instance of DefaultCmabClient.
+
+    NUM_LOCK_STRIPES = 1000
+
     def initialize(cmab_cache, cmab_client, logger = nil)
       @cmab_cache = cmab_cache
       @cmab_client = cmab_client
       @logger = logger
+      @locks = Array.new(NUM_LOCK_STRIPES) { Mutex.new }
     end
 
     def get_decision(project_config, user_context, rule_id, options)
+      lock_index = get_lock_index(user_context.user_id, rule_id)
+
+      @locks[lock_index].synchronize do
+        get_decision_impl(project_config, user_context, rule_id, options)
+      end
+    end
+
+    private
+
+    def get_lock_index(user_id, rule_id)
+      # Create a hash of user_id + rule_id for consistent lock selection
+      hash_input = "#{user_id}#{rule_id}"
+      hash_value = MurmurHash3::V32.str_hash(hash_input, 1) & 0xFFFFFFFF # Convert to unsigned 32-bit equivalent
+      hash_value % NUM_LOCK_STRIPES
+    end
+
+    def get_decision_impl(project_config, user_context, rule_id, options)
       # Retrieves a decision for a given user and rule, utilizing a cache for efficiency.
       #
       # This method filters user attributes, checks for various cache-related options,
@@ -85,8 +107,6 @@ def get_decision(project_config, user_context, rule_id, options)
       cmab_decision
     end
 
-    private
-
     def fetch_decision(rule_id, user_id, attributes)
       # Fetches a decision for a given rule and user, along with user attributes.
       #

spec/cmab/cmab_service_spec.rb

@@ -230,4 +230,46 @@
       )
     end
   end
+
+  describe 'lock striping behavior' do
+    describe '#get_lock_index' do
+      it 'returns consistent lock index for same user/rule combination' do
+        user_id = 'test_user'
+        rule_id = 'test_rule'
+
+        # Get lock index multiple times
+        index1 = cmab_service.send(:get_lock_index, user_id, rule_id)
+        index2 = cmab_service.send(:get_lock_index, user_id, rule_id)
+        index3 = cmab_service.send(:get_lock_index, user_id, rule_id)
+
+        # All should be the same
+        expect(index1).to eq(index2), 'Same user/rule should always use same lock'
+        expect(index2).to eq(index3), 'Same user/rule should always use same lock'
+      end
+
+      it 'distributes different user/rule combinations across multiple locks' do
+        test_cases = [
+          %w[user1 rule1],
+          %w[user2 rule1],
+          %w[user1 rule2],
+          %w[user3 rule3],
+          %w[user4 rule4]
+        ]
+
+        lock_indices = Set.new
+        test_cases.each do |user_id, rule_id|
+          index = cmab_service.send(:get_lock_index, user_id, rule_id)
+
+          # Verify index is within expected range
+          expect(index).to be >= 0, 'Lock index should be non-negative'
+          expect(index).to be < Optimizely::DefaultCmabService::NUM_LOCK_STRIPES, 'Lock index should be less than NUM_LOCK_STRIPES'
+
+          lock_indices.add(index)
+        end
+
+        # We should have multiple different lock indices (though not necessarily all unique due to hash collisions)
+        expect(lock_indices.size).to be > 1, 'Different user/rule combinations should generally use different locks'
+      end
+    end
+  end
 end