From 14be0b6c7a752be60c159f3299cb6a198de30206 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Sat, 25 Jan 2014 20:45:20 -0500 Subject: [PATCH 1/4] Cache course location translations --- common/djangoapps/student/roles.py | 2 +- .../xmodule/modulestore/loc_mapper_store.py | 31 ++++++++++++++++--- .../modulestore/tests/test_location_mapper.py | 4 +-- .../modulestore/tests/test_split_migrator.py | 5 ++- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 13b9b38a159b..7020bcd5d3b4 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -239,4 +239,4 @@ class CourseCreatorRole(GroupBasedRole): """ ROLE = "course_creator_group" def __init__(self, *args, **kwargs): - super(CourseCreatorRole, self).__init__(self.ROLE, *args, **kwargs) + super(CourseCreatorRole, self).__init__([self.ROLE], *args, **kwargs) diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py index d05810b4730f..9d5c29e6aed1 100644 --- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py +++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py @@ -256,7 +256,10 @@ def translate_location_to_course_locator(self, old_style_course_id, location, pu :param course_id: old style course id """ - # doesn't use caching as cache requires full location w/o wildcards + cached = self._get_course_locator_from_cache(old_style_course_id, published) + if cached: + return cached + location_id = self._interpret_location_course_id(old_style_course_id, location) if old_style_course_id is None: old_style_course_id = self._generate_location_course_id(location_id) @@ -274,11 +277,13 @@ def translate_location_to_course_locator(self, old_style_course_id, location, pu if 'name' not in item['_id']: entry = item break + published_course_locator = CourseLocator(package_id=entry['course_id'], branch=entry['prod_branch']) + draft_course_locator = CourseLocator(package_id=entry['course_id'], branch=entry['draft_branch']) + self._cache_course_locator(old_style_course_id, published_course_locator, draft_course_locator) if published: - branch = entry['prod_branch'] + return published_course_locator else: - branch = entry['draft_branch'] - return CourseLocator(package_id=entry['course_id'], branch=branch) + return draft_course_locator def _add_to_block_map(self, location, location_id, block_map): '''add the given location to the block_map and persist it''' @@ -392,6 +397,17 @@ def _get_locator_from_cache(self, old_course_id, location, published): return entry[1] return None + def _get_course_locator_from_cache(self, old_course_id, published): + """ + Get the course Locator for this old course id + """ + entry = self.cache.get(old_course_id) + if entry is not None: + if published: + return entry[0].as_course_locator() + else: + return entry[1].as_course_locator() + def _get_location_from_cache(self, locator): """ See if the locator is in the cache. If so, return the mapped location. @@ -405,6 +421,12 @@ def _get_course_location_from_cache(self, locator_package_id): """ return self.cache.get('courseId+{}'.format(locator_package_id)) + def _cache_course_locator(self, old_course_id, published_course_locator, draft_course_locator): + """ + For quick lookup of courses + """ + self.cache.set(old_course_id, (published_course_locator, draft_course_locator)) + def _cache_location_map_entry(self, old_course_id, location, published_usage, draft_usage): """ Cache the mapping from location to the draft and published Locators in entry. @@ -417,4 +439,5 @@ def _cache_location_map_entry(self, old_course_id, location, published_usage, dr setmany[unicode(published_usage)] = location setmany[unicode(draft_usage)] = location setmany['{}+{}'.format(old_course_id, location.url())] = (published_usage, draft_usage) + setmany[old_course_id] = (published_usage, draft_usage) self.cache.set_many(setmany) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py index 73fa287b4cc9..878d3039286d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py @@ -397,11 +397,11 @@ class TrivialCache(object): def __init__(self): self.cache = {} - def get(self, key): + def get(self, key, default=None): """ Mock the .get """ - return self.cache.get(key) + return self.cache.get(key, default) def set_many(self, entries): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py index c444aa32e1a0..98c64dcac5b6 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py @@ -20,6 +20,7 @@ from xmodule.modulestore.mongo.base import MongoModuleStore from xmodule.modulestore.split_migrator import SplitMigrator from xmodule.modulestore.mongo import draft +from xmodule.modulestore.tests import test_location_mapper class TestMigration(unittest.TestCase): @@ -43,10 +44,8 @@ class TestMigration(unittest.TestCase): def setUp(self): super(TestMigration, self).setUp() - noop_cache = mock.Mock(spec=['get', 'set_many']) - noop_cache.configure_mock(**{'get.return_value': None}) # pylint: disable=W0142 - self.loc_mapper = LocMapperStore(noop_cache, **self.db_config) + self.loc_mapper = LocMapperStore(test_location_mapper.TrivialCache(), **self.db_config) self.old_mongo = MongoModuleStore(self.db_config, **self.modulestore_options) self.draft_mongo = DraftModuleStore(self.db_config, **self.modulestore_options) self.split_mongo = SplitMongoModuleStore( From ee5ee92b3734e11aecdd4b1fb4c0243b88bcfd22 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 27 Jan 2014 14:23:11 -0500 Subject: [PATCH 2/4] Create local memory caching entry for location mapping. --- cms/envs/aws.py | 6 ++++++ lms/envs/aws.py | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index a24e25b74e01..ee95e21d1697 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -122,6 +122,12 @@ LOG_DIR = ENV_TOKENS['LOG_DIR'] CACHES = ENV_TOKENS['CACHES'] +# Cache used for location mapping -- called many times with the same key/value +# in a given request. +CACHES['loc_cache'] = { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'edx_location_mem_cache', +} SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN') SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 3e43a35c29fc..6a85255ca4db 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -155,6 +155,12 @@ LOG_DIR = ENV_TOKENS['LOG_DIR'] CACHES = ENV_TOKENS['CACHES'] +# Cache used for location mapping -- called many times with the same key/value +# in a given request. +CACHES['loc_cache'] = { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'edx_location_mem_cache', +} # Email overrides DEFAULT_FROM_EMAIL = ENV_TOKENS.get('DEFAULT_FROM_EMAIL', DEFAULT_FROM_EMAIL) From 38a6bf6824062c17b39e10dafb1cfddf91bf05fa Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Tue, 28 Jan 2014 11:01:46 -0500 Subject: [PATCH 3/4] Add set to mocked cache --- .../xmodule/modulestore/tests/test_location_mapper.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py index 878d3039286d..90da230e760d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py @@ -408,3 +408,9 @@ def set_many(self, entries): mock set_many """ self.cache.update(entries) + + def set(self, key, entry): + """ + mock set + """ + self.cache[key] = entry From 6e6a1d50215175a8ec3380c3a5d244a94cdbcf28 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Tue, 28 Jan 2014 12:30:38 -0500 Subject: [PATCH 4/4] Only create loc_cache CACHE entry if it doesn't exist in ENV_TOKENS config. --- cms/envs/aws.py | 9 +++++---- lms/envs/aws.py | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index ee95e21d1697..a78157645533 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -124,10 +124,11 @@ CACHES = ENV_TOKENS['CACHES'] # Cache used for location mapping -- called many times with the same key/value # in a given request. -CACHES['loc_cache'] = { - 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', - 'LOCATION': 'edx_location_mem_cache', -} +if 'loc_cache' not in CACHES: + CACHES['loc_cache'] = { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'edx_location_mem_cache', + } SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN') SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 6a85255ca4db..0f1873e6881c 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -157,10 +157,11 @@ CACHES = ENV_TOKENS['CACHES'] # Cache used for location mapping -- called many times with the same key/value # in a given request. -CACHES['loc_cache'] = { - 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', - 'LOCATION': 'edx_location_mem_cache', -} +if 'loc_cache' not in CACHES: + CACHES['loc_cache'] = { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'edx_location_mem_cache', + } # Email overrides DEFAULT_FROM_EMAIL = ENV_TOKENS.get('DEFAULT_FROM_EMAIL', DEFAULT_FROM_EMAIL)