From b609a96902e6be8c1be4fd423db2c9d2dbe018ca Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 27 Mar 2013 22:51:52 -0400 Subject: [PATCH] ummm. forgot to commit stuff --- cms/envs/common.py | 1 + cms/one_time_startup.py | 4 +- .../lib/xmodule/xmodule/modulestore/mongo.py | 105 ++++++++++-------- .../xmodule/modulestore/tests/test_mongo.py | 55 --------- lms/envs/common.py | 1 + lms/one_time_startup.py | 4 +- 6 files changed, 64 insertions(+), 106 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index a83f61d8f9..12fa09947a 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -113,6 +113,7 @@ TEMPLATE_LOADERS = ( MIDDLEWARE_CLASSES = ( 'contentserver.middleware.StaticContentServer', + 'request_cache.middleware.RequestCache', 'django.middleware.cache.UpdateCacheMiddleware', 'django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', diff --git a/cms/one_time_startup.py b/cms/one_time_startup.py index 38a2fef847..6e88fed439 100644 --- a/cms/one_time_startup.py +++ b/cms/one_time_startup.py @@ -1,13 +1,15 @@ from dogapi import dog_http_api, dog_stats_api from django.conf import settings from xmodule.modulestore.django import modulestore +from request_cache.middleware import RequestCache from django.core.cache import get_cache, InvalidCacheBackendError cache = get_cache('mongo_metadata_inheritance') for store_name in settings.MODULESTORE: store = modulestore(store_name) - store.metadata_inheritance_cache = cache + store.metadata_inheritance_cache_subsystem = cache + store.request_cache = RequestCache.get_request_cache() if hasattr(settings, 'DATADOG_API'): dog_http_api.api_key = settings.DATADOG_API diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 38b15ab76e..b93a95c965 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -27,6 +27,9 @@ from .inheritance import own_metadata, INHERITABLE_METADATA, inherit_metadata log = logging.getLogger(__name__) +import threading +_mongo_metadata_request_cache_threadlocal = threading.local() + # TODO (cpennington): This code currently operates under the assumption that # there is only one revision for each item. Once we start versioning inside the CMS, # that assumption will have to change @@ -109,7 +112,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): references to metadata_inheritance_tree """ def __init__(self, modulestore, module_data, default_class, resources_fs, - error_tracker, render_template, metadata_cache=None): + error_tracker, render_template, cached_metadata=None): """ modulestore: the module store that can be used to retrieve additional modules @@ -134,7 +137,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem): # cdodge: other Systems have a course_id attribute defined. To keep things consistent, let's # define an attribute here as well, even though it's None self.course_id = None - self.metadata_cache = metadata_cache + self.cached_metadata = cached_metadata + def load_item(self, location): """ @@ -170,8 +174,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem): model_data = DbModel(kvs, class_, None, MongoUsage(self.course_id, location)) module = class_(self, location, model_data) - if self.metadata_cache is not None: - metadata_to_inherit = self.metadata_cache.get(metadata_cache_key(location), {}).get('parent_metadata', {}).get(location.url(), {}) + if self.cached_metadata is not None: + metadata_to_inherit = self.cached_metadata.get(location.url(), {}) inherit_metadata(module, metadata_to_inherit) return module except: @@ -223,7 +227,8 @@ class MongoModuleStore(ModuleStoreBase): def __init__(self, host, db, collection, fs_root, render_template, port=27017, default_class=None, error_tracker=null_error_tracker, - user=None, password=None, **kwargs): + user=None, password=None, request_cache=None, + metadata_inheritance_cache_subsystem=None, **kwargs): ModuleStoreBase.__init__(self) @@ -254,8 +259,10 @@ class MongoModuleStore(ModuleStoreBase): self.error_tracker = error_tracker self.render_template = render_template self.ignore_write_events_on_courses = [] + self.request_cache = request_cache + self.metadata_inheritance_cache_subsystem = metadata_inheritance_cache_subsystem - def get_metadata_inheritance_tree(self, location): + def compute_metadata_inheritance_tree(self, location): ''' TODO (cdodge) This method can be deleted when the 'split module store' work has been completed ''' @@ -323,32 +330,47 @@ class MongoModuleStore(ModuleStoreBase): if root is not None: _compute_inherited_metadata(root) - return {'parent_metadata': metadata_to_inherit, - 'timestamp': datetime.now()} + return metadata_to_inherit - def get_cached_metadata_inheritance_trees(self, locations, force_refresh=False): + def get_cached_metadata_inheritance_tree(self, location, force_refresh=False): ''' TODO (cdodge) This method can be deleted when the 'split module store' work has been completed ''' + key = metadata_cache_key(location) + tree = {} + + if not force_refresh: + # see if we are first in the request cache (if present) + if self.request_cache is not None and key in self.request_cache.data.get('metadata_inheritance', {}): + logging.debug('***** HIT IN REQUEST CACHE') + return self.request_cache.data['metadata_inheritance'][key] - trees = {} - if locations and self.metadata_inheritance_cache is not None and not force_refresh: - trees = self.metadata_inheritance_cache.get_many(list(set([metadata_cache_key(loc) for loc in locations]))) - else: - # This is to help guard against an accident prod runtime without a cache - logging.warning('Running MongoModuleStore without metadata_inheritance_cache. ' - 'This should not happen in production!') + # then look in any caching subsystem (e.g. memcached) + if self.metadata_inheritance_cache_subsystem is not None: + tree = self.metadata_inheritance_cache_subsystem.get(key, {}) + if tree: + logging.debug('***** HIT IN MEMCACHED') + else: + logging.warning('Running MongoModuleStore without a metadata_inheritance_cache_subsystem. This is OK in localdev and testing environment. Not OK in production.') - to_cache = {} - for loc in locations: - cache_key = metadata_cache_key(loc) - if cache_key not in trees: - to_cache[cache_key] = trees[cache_key] = self.get_metadata_inheritance_tree(loc) + if not tree: + # if not in subsystem, or we are on force refresh, then we have to compute + logging.debug('***** COMPUTING METADATA') + tree = self.compute_metadata_inheritance_tree(location) - if to_cache and self.metadata_inheritance_cache is not None: - self.metadata_inheritance_cache.set_many(to_cache) + # now populate a request_cache, if available + if self.request_cache is not None: + # we can't assume the 'metadatat_inheritance' part of the request cache dict has been + # defined + if 'metadata_inheritance' not in self.request_cache.data: + self.request_cache.data['metadata_inheritance'] = {} + self.request_cache.data['metadata_inheritance'][key] = tree - return trees + # now write to caching subsystem (e.g. memcached), if available + if self.metadata_inheritance_cache_subsystem is not None: + self.metadata_inheritance_cache_subsystem.set(key, tree) + + return tree def refresh_cached_metadata_inheritance_tree(self, location): """ @@ -357,15 +379,7 @@ class MongoModuleStore(ModuleStoreBase): """ pseudo_course_id = '/'.join([location.org, location.course]) if pseudo_course_id not in self.ignore_write_events_on_courses: - self.get_cached_metadata_inheritance_trees([location], force_refresh=True) - - def clear_cached_metadata_inheritance_tree(self, location): - """ - Delete the cached metadata inheritance tree for the org/course combination - for location - """ - if self.metadata_inheritance_cache is not None: - self.metadata_inheritance_cache.delete(metadata_cache_key(location)) + self.get_cached_metadata_inheritance_tree(location, force_refresh=True) def _clean_item_data(self, item): """ @@ -411,18 +425,7 @@ class MongoModuleStore(ModuleStoreBase): return data - def _cache_metadata_inheritance(self, items, depth, force_refresh=False): - """ - Retrieves all course metadata inheritance trees needed to load items - """ - - locations = [ - Location(item['location']) for item in items - if not (item['location']['category'] == 'course' and depth == 0) - ] - return self.get_cached_metadata_inheritance_trees(locations, force_refresh=force_refresh) - - def _load_item(self, item, data_cache, metadata_cache): + def _load_item(self, item, data_cache, apply_cached_metadata=True): """ Load an XModuleDescriptor from item, using the children stored in data_cache """ @@ -434,6 +437,10 @@ class MongoModuleStore(ModuleStoreBase): resource_fs = OSFS(root) + cached_metadata = {} + if apply_cached_metadata: + cached_metadata = self.get_cached_metadata_inheritance_tree(Location(item['location'])) + # TODO (cdodge): When the 'split module store' work has been completed, we should remove # the 'metadata_inheritance_tree' parameter system = CachingDescriptorSystem( @@ -443,7 +450,7 @@ class MongoModuleStore(ModuleStoreBase): resource_fs, self.error_tracker, self.render_template, - metadata_cache, + cached_metadata, ) return system.load_item(item['location']) @@ -453,11 +460,11 @@ class MongoModuleStore(ModuleStoreBase): to specified depth """ data_cache = self._cache_children(items, depth) - inheritance_cache = self._cache_metadata_inheritance(items, depth) # if we are loading a course object, if we're not prefetching children (depth != 0) then don't - # bother with the metadata inheritence - return [self._load_item(item, data_cache, inheritance_cache) for item in items] + # bother with the metadata inheritance + return [self._load_item(item, data_cache, + apply_cached_metadata=(item['location']['category']!='course' or depth !=0)) for item in items] def get_courses(self): ''' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 3e29c07ea4..061d70d09f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -103,58 +103,3 @@ class TestMongoModuleStore(object): def test_path_to_location(self): '''Make sure that path_to_location works''' check_path_to_location(self.store) - - def test_metadata_inheritance_query_count(self): - ''' - When retrieving items from mongo, we should only query the cache a number of times - equal to the number of courses being retrieved from. - - We should also not query - ''' - self.store.metadata_inheritance_cache = Mock() - get_many = self.store.metadata_inheritance_cache.get_many - set_many = self.store.metadata_inheritance_cache.set_many - get_many.return_value = {('edX', 'toy'): {}} - - self.store.get_item(Location("i4x://edX/toy/course/2012_Fall"), depth=0) - assert_false(get_many.called) - assert_false(set_many.called) - get_many.reset_mock() - - self.store.get_item(Location("i4x://edX/toy/course/2012_Fall"), depth=3) - get_many.assert_called_with([('edX', 'toy')]) - assert_equals(0, set_many.call_count) - get_many.reset_mock() - - self.store.get_items(Location('i4x', 'edX', None, 'course', None), depth=0) - assert_false(get_many.called) - assert_false(set_many.called) - get_many.reset_mock() - - self.store.get_items(Location('i4x', 'edX', None, 'course', None), depth=3) - assert_equals(1, get_many.call_count) - assert_equals([('edX', 'simple'), ('edX', 'toy')], sorted(get_many.call_args[0][0])) - assert_equals(1, set_many.call_count) - assert_equals([('edX', 'simple')], sorted(set_many.call_args[0][0].keys())) - get_many.reset_mock() - - self.store.get_items(Location('i4x', 'edX', None, None, None), depth=0) - assert_equals(1, get_many.call_count) - assert_equals([('edX', 'simple'), ('edX', 'toy')], sorted(get_many.call_args[0][0])) - assert_equals(1, set_many.call_count) - assert_equals([('edX', 'simple')], sorted(set_many.call_args[0][0].keys())) - get_many.reset_mock() - - def test_metadata_inheritance_query_count_forced_refresh(self): - self.store.metadata_inheritance_cache = Mock() - get_many = self.store.metadata_inheritance_cache.get_many - set_many = self.store.metadata_inheritance_cache.set_many - get_many.return_value = {('edX', 'toy'): {}} - - self.store.get_cached_metadata_inheritance_trees( - [Location("i4x://edX/toy/course/2012_Fall"), Location("i4x://edX/simple/course/2012_Fall")], - True - ) - assert_false(get_many.called) - assert_equals(1, set_many.call_count) - assert_equals([('edX', 'simple'), ('edX', 'toy')], sorted(set_many.call_args[0][0].keys())) diff --git a/lms/envs/common.py b/lms/envs/common.py index cfd6fc34de..8654b5ebf5 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -364,6 +364,7 @@ TEMPLATE_LOADERS = ( MIDDLEWARE_CLASSES = ( 'contentserver.middleware.StaticContentServer', + 'request_cache.middleware.RequestCache', 'django_comment_client.middleware.AjaxExceptionMiddleware', 'django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', diff --git a/lms/one_time_startup.py b/lms/one_time_startup.py index 6b3c45d60f..e1b1f79444 100644 --- a/lms/one_time_startup.py +++ b/lms/one_time_startup.py @@ -2,13 +2,15 @@ import logging from dogapi import dog_http_api, dog_stats_api from django.conf import settings from xmodule.modulestore.django import modulestore +from request_cache.middleware import RequestCache from django.core.cache import get_cache, InvalidCacheBackendError cache = get_cache('mongo_metadata_inheritance') for store_name in settings.MODULESTORE: store = modulestore(store_name) - store.metadata_inheritance_cache = cache + store.metadata_inheritance_cache_subsystem = cache + store.request_cache = RequestCache.get_request_cache() if hasattr(settings, 'DATADOG_API'): dog_http_api.api_key = settings.DATADOG_API