Skip to content

Commit 70a880d

Browse files
[FSSDK-11177] lock implementation
1 parent f1a33cc commit 70a880d

File tree

2 files changed

+142
-0
lines changed

2 files changed

+142
-0
lines changed

OptimizelySDK.Tests/CmabTests/DefaultCmabServiceTest.cs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,103 @@ public void ConstructorUsesProvidedClientInstance()
489489
Assert.AreSame(mockClient, client);
490490
}
491491

492+
[Test]
493+
public void ConcurrentRequestsForSameUserUseCacheAfterFirstNetworkCall()
494+
{
495+
var experiment = CreateExperiment(TEST_RULE_ID, new List<string> { AGE_ATTRIBUTE_ID });
496+
var attributeMap = new Dictionary<string, AttributeEntity>
497+
{
498+
{ AGE_ATTRIBUTE_ID, new AttributeEntity { Id = AGE_ATTRIBUTE_ID, Key = "age" } },
499+
};
500+
var projectConfig = CreateProjectConfig(TEST_RULE_ID, experiment, attributeMap);
501+
var userContext = CreateUserContext(TEST_USER_ID,
502+
new Dictionary<string, object> { { "age", 25 } });
503+
504+
var clientCallCount = 0;
505+
var clientCallLock = new object();
506+
507+
_mockCmabClient.Setup(c => c.FetchDecision(
508+
TEST_RULE_ID,
509+
TEST_USER_ID,
510+
It.Is<IDictionary<string, object>>(attrs =>
511+
attrs != null && attrs.Count == 1 && attrs.ContainsKey("age") &&
512+
(int)attrs["age"] == 25),
513+
It.IsAny<string>(),
514+
It.IsAny<TimeSpan?>()))
515+
.Returns(() =>
516+
{
517+
lock (clientCallLock)
518+
{
519+
clientCallCount++;
520+
}
521+
System.Threading.Thread.Sleep(100);
522+
523+
return "varConcurrent";
524+
});
525+
526+
var tasks = new System.Threading.Tasks.Task<CmabDecision>[10];
527+
528+
for (int i = 0; i < tasks.Length; i++)
529+
{
530+
tasks[i] = System.Threading.Tasks.Task.Run(() =>
531+
_cmabService.GetDecision(projectConfig, userContext, TEST_RULE_ID));
532+
}
533+
534+
System.Threading.Tasks.Task.WaitAll(tasks);
535+
536+
foreach (var task in tasks)
537+
{
538+
Assert.IsNotNull(task.Result);
539+
Assert.AreEqual("varConcurrent", task.Result.VariationId);
540+
}
541+
542+
Assert.AreEqual(1, clientCallCount,
543+
"Client should only be called once - subsequent requests should use cache");
544+
545+
_mockCmabClient.VerifyAll();
546+
}
547+
548+
[Test]
549+
public void SameUserRuleCombinationUsesConsistentLock()
550+
{
551+
var userId = "test_user";
552+
var ruleId = "test_rule";
553+
554+
var index1 = _cmabService.GetLockIndex(userId, ruleId);
555+
var index2 = _cmabService.GetLockIndex(userId, ruleId);
556+
var index3 = _cmabService.GetLockIndex(userId, ruleId);
557+
558+
Assert.AreEqual(index1, index2, "Same user/rule should always use same lock");
559+
Assert.AreEqual(index2, index3, "Same user/rule should always use same lock");
560+
}
561+
562+
[Test]
563+
public void LockStripingDistribution()
564+
{
565+
var testCases = new[]
566+
{
567+
new { UserId = "user1", RuleId = "rule1" },
568+
new { UserId = "user2", RuleId = "rule1" },
569+
new { UserId = "user1", RuleId = "rule2" },
570+
new { UserId = "user3", RuleId = "rule3" },
571+
new { UserId = "user4", RuleId = "rule4" },
572+
};
573+
574+
var lockIndices = new HashSet<int>();
575+
foreach (var testCase in testCases)
576+
{
577+
var index = _cmabService.GetLockIndex(testCase.UserId, testCase.RuleId);
578+
579+
Assert.GreaterOrEqual(index, 0, "Lock index should be non-negative");
580+
Assert.Less(index, 1000, "Lock index should be less than NUM_LOCK_STRIPES (1000)");
581+
582+
lockIndices.Add(index);
583+
}
584+
585+
Assert.Greater(lockIndices.Count, 1,
586+
"Different user/rule combinations should generally use different locks");
587+
}
588+
492589
private static ICache<CmabCacheEntry> GetInternalCache(DefaultCmabService service)
493590
{
494591
return Reflection.GetFieldValue<ICache<CmabCacheEntry>, DefaultCmabService>(service,
@@ -554,5 +651,9 @@ private static Experiment CreateExperiment(string ruleId, List<string> attribute
554651
Cmab = attributeIds == null ? null : new Entity.Cmab(attributeIds),
555652
};
556653
}
654+
655+
656+
657+
557658
}
558659
}

OptimizelySDK/Cmab/DefaultCmabService.cs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,16 @@ public class CmabCacheEntry
8383
/// </summary>
8484
public class DefaultCmabService : ICmabService
8585
{
86+
/// <summary>
87+
/// Number of lock stripes to use for concurrency control.
88+
/// Using multiple locks reduces contention while ensuring the same user/rule combination always uses the same lock.
89+
/// </summary>
90+
private const int NUM_LOCK_STRIPES = 1000;
91+
8692
private readonly ICache<CmabCacheEntry> _cmabCache;
8793
private readonly ICmabClient _cmabClient;
8894
private readonly ILogger _logger;
95+
private readonly object[] _locks;
8996

9097
/// <summary>
9198
/// Initializes a new instance of the DefaultCmabService class.
@@ -100,12 +107,46 @@ public DefaultCmabService(ICache<CmabCacheEntry> cmabCache,
100107
_cmabCache = cmabCache;
101108
_cmabClient = cmabClient;
102109
_logger = logger;
110+
_locks = Enumerable.Range(0, NUM_LOCK_STRIPES).Select(_ => new object()).ToArray();
111+
}
112+
113+
/// <summary>
114+
/// Calculate the lock index for a given user and rule combination.
115+
/// Uses MurmurHash to ensure consistent lock selection for the same user/rule while distributing different combinations across locks.
116+
/// </summary>
117+
/// <param name="userId">The user ID.</param>
118+
/// <param name="ruleId">The experiment/rule ID.</param>
119+
/// <returns>The lock index in the range [0, NUM_LOCK_STRIPES).</returns>
120+
internal int GetLockIndex(string userId, string ruleId)
121+
{
122+
var hashInput = $"{userId}{ruleId}";
123+
var murmer32 = Murmur.MurmurHash.Create32(0, true);
124+
var data = Encoding.UTF8.GetBytes(hashInput);
125+
var hash = murmer32.ComputeHash(data);
126+
var hashValue = BitConverter.ToUInt32(hash, 0);
127+
return (int)(hashValue % NUM_LOCK_STRIPES);
103128
}
104129

105130
public CmabDecision GetDecision(ProjectConfig projectConfig,
106131
OptimizelyUserContext userContext,
107132
string ruleId,
108133
OptimizelyDecideOption[] options = null)
134+
{
135+
var lockIndex = GetLockIndex(userContext.GetUserId(), ruleId);
136+
lock (_locks[lockIndex])
137+
{
138+
return GetDecisionInternal(projectConfig, userContext, ruleId, options);
139+
}
140+
}
141+
142+
/// <summary>
143+
/// Internal implementation of GetDecision that performs the actual decision logic.
144+
/// This method should only be called while holding the appropriate lock.
145+
/// </summary>
146+
private CmabDecision GetDecisionInternal(ProjectConfig projectConfig,
147+
OptimizelyUserContext userContext,
148+
string ruleId,
149+
OptimizelyDecideOption[] options = null)
109150
{
110151
var optionSet = options ?? new OptimizelyDecideOption[0];
111152
var filteredAttributes = FilterAttributes(projectConfig, userContext, ruleId);

0 commit comments

Comments
 (0)