From 6384e29dd0ae4c7dcec60941798628102b825ebb Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 8 Oct 2025 17:39:15 +0600 Subject: [PATCH 1/4] Update: Implement locking mechanism for CMAB service to enhance concurrency --- optimizely/cmab/cmab_service.py | 57 ++++++++++++++++++++------------- tests/test_cmab_service.py | 40 ++++++++++++++++++++++- 2 files changed, 74 insertions(+), 23 deletions(-) diff --git a/optimizely/cmab/cmab_service.py b/optimizely/cmab/cmab_service.py index a7c4b69b..37c86d29 100644 --- a/optimizely/cmab/cmab_service.py +++ b/optimizely/cmab/cmab_service.py @@ -13,6 +13,7 @@ import uuid import json import hashlib +import threading from typing import Optional, List, TypedDict from optimizely.cmab.cmab_client import DefaultCmabClient @@ -22,6 +23,8 @@ from optimizely.decision.optimizely_decide_option import OptimizelyDecideOption from optimizely import logger as _logging +NUM_LOCK_STRIPES = 1000 + class CmabDecision(TypedDict): variation_id: str @@ -52,40 +55,50 @@ def __init__(self, cmab_cache: LRUCache[str, CmabCacheValue], self.cmab_cache = cmab_cache self.cmab_client = cmab_client self.logger = logger + self.locks = [threading.Lock() for _ in range(NUM_LOCK_STRIPES)] + + def _get_lock_index(self, user_id: str, rule_id: str) -> int: + """Calculate the lock index for a given user and rule combination.""" + # Create a hash of user_id + rule_id for consistent lock selection + hash_input = f"{user_id}{rule_id}" + hash_value = int(hashlib.md5(hash_input.encode()).hexdigest(), 16) % NUM_LOCK_STRIPES + return hash_value def get_decision(self, project_config: ProjectConfig, user_context: OptimizelyUserContext, rule_id: str, options: List[str]) -> CmabDecision: - filtered_attributes = self._filter_attributes(project_config, user_context, rule_id) + lock_index = self._get_lock_index(user_context.user_id, rule_id) + with self.locks[lock_index]: + filtered_attributes = self._filter_attributes(project_config, user_context, rule_id) - if OptimizelyDecideOption.IGNORE_CMAB_CACHE in options: - return self._fetch_decision(rule_id, user_context.user_id, filtered_attributes) + if OptimizelyDecideOption.IGNORE_CMAB_CACHE in options: + return self._fetch_decision(rule_id, user_context.user_id, filtered_attributes) - if OptimizelyDecideOption.RESET_CMAB_CACHE in options: - self.cmab_cache.reset() + if OptimizelyDecideOption.RESET_CMAB_CACHE in options: + self.cmab_cache.reset() - cache_key = self._get_cache_key(user_context.user_id, rule_id) + cache_key = self._get_cache_key(user_context.user_id, rule_id) - if OptimizelyDecideOption.INVALIDATE_USER_CMAB_CACHE in options: - self.cmab_cache.remove(cache_key) + if OptimizelyDecideOption.INVALIDATE_USER_CMAB_CACHE in options: + self.cmab_cache.remove(cache_key) - cached_value = self.cmab_cache.lookup(cache_key) + cached_value = self.cmab_cache.lookup(cache_key) - attributes_hash = self._hash_attributes(filtered_attributes) + attributes_hash = self._hash_attributes(filtered_attributes) - if cached_value: - if cached_value['attributes_hash'] == attributes_hash: - return CmabDecision(variation_id=cached_value['variation_id'], cmab_uuid=cached_value['cmab_uuid']) - else: - self.cmab_cache.remove(cache_key) + if cached_value: + if cached_value['attributes_hash'] == attributes_hash: + return CmabDecision(variation_id=cached_value['variation_id'], cmab_uuid=cached_value['cmab_uuid']) + else: + self.cmab_cache.remove(cache_key) - cmab_decision = self._fetch_decision(rule_id, user_context.user_id, filtered_attributes) - self.cmab_cache.save(cache_key, { - 'attributes_hash': attributes_hash, - 'variation_id': cmab_decision['variation_id'], - 'cmab_uuid': cmab_decision['cmab_uuid'], - }) - return cmab_decision + cmab_decision = self._fetch_decision(rule_id, user_context.user_id, filtered_attributes) + self.cmab_cache.save(cache_key, { + 'attributes_hash': attributes_hash, + 'variation_id': cmab_decision['variation_id'], + 'cmab_uuid': cmab_decision['cmab_uuid'], + }) + return cmab_decision def _fetch_decision(self, rule_id: str, user_id: str, attributes: UserAttributes) -> CmabDecision: cmab_uuid = str(uuid.uuid4()) diff --git a/tests/test_cmab_service.py b/tests/test_cmab_service.py index 0b3c593a..5aede695 100644 --- a/tests/test_cmab_service.py +++ b/tests/test_cmab_service.py @@ -12,7 +12,7 @@ # limitations under the License. import unittest from unittest.mock import MagicMock -from optimizely.cmab.cmab_service import DefaultCmabService +from optimizely.cmab.cmab_service import DefaultCmabService, NUM_LOCK_STRIPES from optimizely.optimizely_user_context import OptimizelyUserContext from optimizely.decision.optimizely_decide_option import OptimizelyDecideOption from optimizely.odp.lru_cache import LRUCache @@ -185,3 +185,41 @@ def test_only_cmab_attributes_passed_to_client(self): {"age": 25, "location": "USA"}, decision["cmab_uuid"] ) + + def test_same_user_rule_combination_uses_consistent_lock(self): + """Verifies that the same user/rule combination always uses the same lock index""" + user_id = "test_user" + rule_id = "test_rule" + + # Get lock index multiple times + index1 = self.cmab_service._get_lock_index(user_id, rule_id) + index2 = self.cmab_service._get_lock_index(user_id, rule_id) + index3 = self.cmab_service._get_lock_index(user_id, rule_id) + + # All should be the same + self.assertEqual(index1, index2, "Same user/rule should always use same lock") + self.assertEqual(index2, index3, "Same user/rule should always use same lock") + + def test_lock_striping_distribution(self): + """Verifies that different user/rule combinations use different locks to allow for better concurrency""" + test_cases = [ + ("user1", "rule1"), + ("user2", "rule1"), + ("user1", "rule2"), + ("user3", "rule3"), + ("user4", "rule4"), + ] + + lock_indices = set() + for user_id, rule_id in test_cases: + index = self.cmab_service._get_lock_index(user_id, rule_id) + + # Verify index is within expected range + self.assertGreaterEqual(index, 0, "Lock index should be non-negative") + self.assertLess(index, NUM_LOCK_STRIPES, "Lock index should be less than NUM_LOCK_STRIPES") + + lock_indices.add(index) + + # We should have multiple different lock indices (though not necessarily all unique due to hash collisions) + self.assertGreater(len(lock_indices), 1, + "Different user/rule combinations should generally use different locks") From 3312944e58ed1b47d7f738f114b5bd270251d643 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 8 Oct 2025 23:02:47 +0600 Subject: [PATCH 2/4] Update: Refactor get_decision method --- optimizely/cmab/cmab_service.py | 49 ++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/optimizely/cmab/cmab_service.py b/optimizely/cmab/cmab_service.py index 37c86d29..d91f7d28 100644 --- a/optimizely/cmab/cmab_service.py +++ b/optimizely/cmab/cmab_service.py @@ -69,36 +69,41 @@ def get_decision(self, project_config: ProjectConfig, user_context: OptimizelyUs lock_index = self._get_lock_index(user_context.user_id, rule_id) with self.locks[lock_index]: - filtered_attributes = self._filter_attributes(project_config, user_context, rule_id) + return self._get_decision(project_config, user_context, rule_id, options) - if OptimizelyDecideOption.IGNORE_CMAB_CACHE in options: - return self._fetch_decision(rule_id, user_context.user_id, filtered_attributes) + def _get_decision(self, project_config: ProjectConfig, user_context: OptimizelyUserContext, + rule_id: str, options: List[str]) -> CmabDecision: - if OptimizelyDecideOption.RESET_CMAB_CACHE in options: - self.cmab_cache.reset() + filtered_attributes = self._filter_attributes(project_config, user_context, rule_id) - cache_key = self._get_cache_key(user_context.user_id, rule_id) + if OptimizelyDecideOption.IGNORE_CMAB_CACHE in options: + return self._fetch_decision(rule_id, user_context.user_id, filtered_attributes) - if OptimizelyDecideOption.INVALIDATE_USER_CMAB_CACHE in options: - self.cmab_cache.remove(cache_key) + if OptimizelyDecideOption.RESET_CMAB_CACHE in options: + self.cmab_cache.reset() + + cache_key = self._get_cache_key(user_context.user_id, rule_id) - cached_value = self.cmab_cache.lookup(cache_key) + if OptimizelyDecideOption.INVALIDATE_USER_CMAB_CACHE in options: + self.cmab_cache.remove(cache_key) - attributes_hash = self._hash_attributes(filtered_attributes) + cached_value = self.cmab_cache.lookup(cache_key) - if cached_value: - if cached_value['attributes_hash'] == attributes_hash: - return CmabDecision(variation_id=cached_value['variation_id'], cmab_uuid=cached_value['cmab_uuid']) - else: - self.cmab_cache.remove(cache_key) + attributes_hash = self._hash_attributes(filtered_attributes) - cmab_decision = self._fetch_decision(rule_id, user_context.user_id, filtered_attributes) - self.cmab_cache.save(cache_key, { - 'attributes_hash': attributes_hash, - 'variation_id': cmab_decision['variation_id'], - 'cmab_uuid': cmab_decision['cmab_uuid'], - }) - return cmab_decision + if cached_value: + if cached_value['attributes_hash'] == attributes_hash: + return CmabDecision(variation_id=cached_value['variation_id'], cmab_uuid=cached_value['cmab_uuid']) + else: + self.cmab_cache.remove(cache_key) + + cmab_decision = self._fetch_decision(rule_id, user_context.user_id, filtered_attributes) + self.cmab_cache.save(cache_key, { + 'attributes_hash': attributes_hash, + 'variation_id': cmab_decision['variation_id'], + 'cmab_uuid': cmab_decision['cmab_uuid'], + }) + return cmab_decision def _fetch_decision(self, rule_id: str, user_id: str, attributes: UserAttributes) -> CmabDecision: cmab_uuid = str(uuid.uuid4()) From 3792c52e4c3fe1c57797668b49b2e9be51249945 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 13 Oct 2025 20:02:30 +0600 Subject: [PATCH 3/4] update: type checking fix --- optimizely/bucketer.py | 2 +- optimizely/cmab/cmab_service.py | 6 +++--- optimizely/decision/optimizely_decide_option.py | 2 +- optimizely/decision/optimizely_decision_message.py | 2 +- optimizely/entities.py | 2 +- optimizely/event/event_factory.py | 2 +- optimizely/event/event_processor.py | 2 +- optimizely/event/log_event.py | 2 +- optimizely/event/user_event.py | 2 +- optimizely/event_builder.py | 2 +- optimizely/event_dispatcher.py | 2 +- optimizely/helpers/condition.py | 2 +- optimizely/helpers/enums.py | 2 +- optimizely/helpers/event_tag_utils.py | 2 +- optimizely/logger.py | 2 +- optimizely/notification_center.py | 2 +- optimizely/odp/lru_cache.py | 2 +- optimizely/odp/optimizely_odp_option.py | 2 +- optimizely/project_config.py | 2 +- optimizely/user_profile.py | 2 +- 20 files changed, 22 insertions(+), 22 deletions(-) diff --git a/optimizely/bucketer.py b/optimizely/bucketer.py index 1bd7ff52..a6e8323d 100644 --- a/optimizely/bucketer.py +++ b/optimizely/bucketer.py @@ -22,7 +22,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final if TYPE_CHECKING: diff --git a/optimizely/cmab/cmab_service.py b/optimizely/cmab/cmab_service.py index d91f7d28..82c2c206 100644 --- a/optimizely/cmab/cmab_service.py +++ b/optimizely/cmab/cmab_service.py @@ -22,7 +22,7 @@ from optimizely.project_config import ProjectConfig from optimizely.decision.optimizely_decide_option import OptimizelyDecideOption from optimizely import logger as _logging - +from optimizely.lib import pymmh3 as mmh3 NUM_LOCK_STRIPES = 1000 @@ -61,8 +61,8 @@ def _get_lock_index(self, user_id: str, rule_id: str) -> int: """Calculate the lock index for a given user and rule combination.""" # Create a hash of user_id + rule_id for consistent lock selection hash_input = f"{user_id}{rule_id}" - hash_value = int(hashlib.md5(hash_input.encode()).hexdigest(), 16) % NUM_LOCK_STRIPES - return hash_value + hash_value = mmh3.hash(hash_input, seed=0) & 0xFFFFFFFF # Convert to unsigned + return hash_value % NUM_LOCK_STRIPES def get_decision(self, project_config: ProjectConfig, user_context: OptimizelyUserContext, rule_id: str, options: List[str]) -> CmabDecision: diff --git a/optimizely/decision/optimizely_decide_option.py b/optimizely/decision/optimizely_decide_option.py index 8cffcfec..0443ddef 100644 --- a/optimizely/decision/optimizely_decide_option.py +++ b/optimizely/decision/optimizely_decide_option.py @@ -16,7 +16,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final class OptimizelyDecideOption: diff --git a/optimizely/decision/optimizely_decision_message.py b/optimizely/decision/optimizely_decision_message.py index 20231ea5..c6178322 100644 --- a/optimizely/decision/optimizely_decision_message.py +++ b/optimizely/decision/optimizely_decision_message.py @@ -16,7 +16,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final class OptimizelyDecisionMessage: diff --git a/optimizely/entities.py b/optimizely/entities.py index 7d257656..83488bdf 100644 --- a/optimizely/entities.py +++ b/optimizely/entities.py @@ -17,7 +17,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final if TYPE_CHECKING: diff --git a/optimizely/event/event_factory.py b/optimizely/event/event_factory.py index c872fb17..48b3a5cc 100644 --- a/optimizely/event/event_factory.py +++ b/optimizely/event/event_factory.py @@ -25,7 +25,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final if TYPE_CHECKING: # prevent circular dependenacy by skipping import at runtime diff --git a/optimizely/event/event_processor.py b/optimizely/event/event_processor.py index 05f5e078..4fba29eb 100644 --- a/optimizely/event/event_processor.py +++ b/optimizely/event/event_processor.py @@ -34,7 +34,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final class BaseEventProcessor(ABC): diff --git a/optimizely/event/log_event.py b/optimizely/event/log_event.py index 7c0beeb6..49c344dd 100644 --- a/optimizely/event/log_event.py +++ b/optimizely/event/log_event.py @@ -20,7 +20,7 @@ if version_info < (3, 8): from typing_extensions import Literal else: - from typing import Literal # type: ignore + from typing import Literal class LogEvent(event_builder.Event): diff --git a/optimizely/event/user_event.py b/optimizely/event/user_event.py index e257647c..c1152161 100644 --- a/optimizely/event/user_event.py +++ b/optimizely/event/user_event.py @@ -22,7 +22,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final if TYPE_CHECKING: diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 90678830..e9c9fd44 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -25,7 +25,7 @@ if version_info < (3, 8): from typing_extensions import Final, Literal else: - from typing import Final, Literal # type: ignore + from typing import Final, Literal if TYPE_CHECKING: # prevent circular dependenacy by skipping import at runtime diff --git a/optimizely/event_dispatcher.py b/optimizely/event_dispatcher.py index 767fbb7d..55209dc8 100644 --- a/optimizely/event_dispatcher.py +++ b/optimizely/event_dispatcher.py @@ -26,7 +26,7 @@ if version_info < (3, 8): from typing_extensions import Protocol else: - from typing import Protocol # type: ignore + from typing import Protocol class CustomEventDispatcher(Protocol): diff --git a/optimizely/helpers/condition.py b/optimizely/helpers/condition.py index 58000a90..40338b40 100644 --- a/optimizely/helpers/condition.py +++ b/optimizely/helpers/condition.py @@ -32,7 +32,7 @@ if version_info < (3, 8): from typing_extensions import Literal, Final else: - from typing import Literal, Final # type: ignore + from typing import Literal, Final class ConditionOperatorTypes: diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index e3acafef..4630491c 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -17,7 +17,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final class CommonAudienceEvaluationLogs: diff --git a/optimizely/helpers/event_tag_utils.py b/optimizely/helpers/event_tag_utils.py index cb577950..ad90cc13 100644 --- a/optimizely/helpers/event_tag_utils.py +++ b/optimizely/helpers/event_tag_utils.py @@ -21,7 +21,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final if TYPE_CHECKING: diff --git a/optimizely/logger.py b/optimizely/logger.py index 33d3660c..42f879de 100644 --- a/optimizely/logger.py +++ b/optimizely/logger.py @@ -20,7 +20,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final _DEFAULT_LOG_FORMAT: Final = '%(levelname)-8s %(asctime)s %(filename)s:%(lineno)s:%(message)s' diff --git a/optimizely/notification_center.py b/optimizely/notification_center.py index 322a5862..3d0b0cba 100644 --- a/optimizely/notification_center.py +++ b/optimizely/notification_center.py @@ -20,7 +20,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final NOTIFICATION_TYPES: Final = tuple( diff --git a/optimizely/odp/lru_cache.py b/optimizely/odp/lru_cache.py index 073973e6..64337dad 100644 --- a/optimizely/odp/lru_cache.py +++ b/optimizely/odp/lru_cache.py @@ -22,7 +22,7 @@ if version_info < (3, 8): from typing_extensions import Protocol else: - from typing import Protocol # type: ignore + from typing import Protocol # generic type definitions for LRUCache parameters K = TypeVar('K', bound=Hashable, contravariant=True) diff --git a/optimizely/odp/optimizely_odp_option.py b/optimizely/odp/optimizely_odp_option.py index ce6eaf00..94e1e90e 100644 --- a/optimizely/odp/optimizely_odp_option.py +++ b/optimizely/odp/optimizely_odp_option.py @@ -16,7 +16,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final class OptimizelyOdpOption: diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 446d1e2f..89c9b48b 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -24,7 +24,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final if TYPE_CHECKING: # prevent circular dependenacy by skipping import at runtime diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index f5ded013..e3a56195 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -19,7 +19,7 @@ if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final, TYPE_CHECKING # type: ignore + from typing import Final, TYPE_CHECKING if TYPE_CHECKING: # prevent circular dependenacy by skipping import at runtime From 859c44fb6daaaddfffe977ae849194b2a91b3989 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 13 Oct 2025 23:05:24 +0600 Subject: [PATCH 4/4] Update: Remove Python 3.8 from testing matrix in CI workflow --- .github/workflows/python.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 0699f84c..6baa2c09 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -65,10 +65,8 @@ jobs: fail-fast: false matrix: python-version: - - "pypy-3.8" - "pypy-3.9" - "pypy-3.10" - - "3.8" - "3.9" - "3.10" - "3.11" @@ -93,10 +91,8 @@ jobs: fail-fast: false matrix: python-version: - - "pypy-3.8" - "pypy-3.9" - "pypy-3.10" - - "3.8" - "3.9" - "3.10" - "3.11"