diff --git a/lib/optimizely/cmab/cmab_service.rb b/lib/optimizely/cmab/cmab_service.rb index ceed3066..7bd60c0f 100644 --- a/lib/optimizely/cmab/cmab_service.rb +++ b/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. # diff --git a/spec/cmab/cmab_service_spec.rb b/spec/cmab/cmab_service_spec.rb index de94d39b..7ac38147 100644 --- a/spec/cmab/cmab_service_spec.rb +++ b/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