Skip to content

Commit 22dfd71

Browse files
feat: Implement lock striping for decision retrieval in CMAB service
1 parent 1a8881f commit 22dfd71

File tree

2 files changed

+64
-2
lines changed

2 files changed

+64
-2
lines changed

lib/optimizely/cmab/cmab_service.rb

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,35 @@ class DefaultCmabService
3535
#
3636
# @raise [ArgumentError] If cmab_cache is not an instance of LRUCache.
3737
# @raise [ArgumentError] If cmab_client is not an instance of DefaultCmabClient.
38+
39+
NUM_LOCK_STRIPES = 1000
40+
3841
def initialize(cmab_cache, cmab_client, logger = nil)
3942
@cmab_cache = cmab_cache
4043
@cmab_client = cmab_client
4144
@logger = logger
45+
@locks = Array.new(NUM_LOCK_STRIPES) { Mutex.new }
4246
end
4347

4448
def get_decision(project_config, user_context, rule_id, options)
49+
lock_index = get_lock_index(user_context.user_id, rule_id)
50+
51+
@locks[lock_index].synchronize do
52+
get_decision_impl(project_config, user_context, rule_id, options)
53+
end
54+
end
55+
56+
private
57+
58+
def get_lock_index(user_id, rule_id)
59+
# Create a hash of user_id + rule_id for consistent lock selection
60+
hash_input = "#{user_id}#{rule_id}"
61+
# Use Ruby's built-in hash method and ensure positive result
62+
hash_value = hash_input.hash.abs
63+
hash_value % NUM_LOCK_STRIPES
64+
end
65+
66+
def get_decision_impl(project_config, user_context, rule_id, options)
4567
# Retrieves a decision for a given user and rule, utilizing a cache for efficiency.
4668
#
4769
# 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)
85107
cmab_decision
86108
end
87109

88-
private
89-
90110
def fetch_decision(rule_id, user_id, attributes)
91111
# Fetches a decision for a given rule and user, along with user attributes.
92112
#

spec/cmab/cmab_service_spec.rb

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,4 +230,46 @@
230230
)
231231
end
232232
end
233+
234+
describe 'lock striping behavior' do
235+
describe '#get_lock_index' do
236+
it 'returns consistent lock index for same user/rule combination' do
237+
user_id = 'test_user'
238+
rule_id = 'test_rule'
239+
240+
# Get lock index multiple times
241+
index1 = cmab_service.send(:get_lock_index, user_id, rule_id)
242+
index2 = cmab_service.send(:get_lock_index, user_id, rule_id)
243+
index3 = cmab_service.send(:get_lock_index, user_id, rule_id)
244+
245+
# All should be the same
246+
expect(index1).to eq(index2), 'Same user/rule should always use same lock'
247+
expect(index2).to eq(index3), 'Same user/rule should always use same lock'
248+
end
249+
250+
it 'distributes different user/rule combinations across multiple locks' do
251+
test_cases = [
252+
%w[user1 rule1],
253+
%w[user2 rule1],
254+
%w[user1 rule2],
255+
%w[user3 rule3],
256+
%w[user4 rule4]
257+
]
258+
259+
lock_indices = Set.new
260+
test_cases.each do |user_id, rule_id|
261+
index = cmab_service.send(:get_lock_index, user_id, rule_id)
262+
263+
# Verify index is within expected range
264+
expect(index).to be >= 0, 'Lock index should be non-negative'
265+
expect(index).to be < Optimizely::DefaultCmabService::NUM_LOCK_STRIPES, 'Lock index should be less than NUM_LOCK_STRIPES'
266+
267+
lock_indices.add(index)
268+
end
269+
270+
# We should have multiple different lock indices (though not necessarily all unique due to hash collisions)
271+
expect(lock_indices.size).to be > 1, 'Different user/rule combinations should generally use different locks'
272+
end
273+
end
274+
end
233275
end

0 commit comments

Comments
 (0)