From 75eee443ff6cdbb2869006f66e841ce3bc613d84 Mon Sep 17 00:00:00 2001 From: kimth Date: Tue, 14 Aug 2012 18:13:53 -0400 Subject: [PATCH] Xqueue callback acquires lock on StudentModule to avoid race condition --- lms/djangoapps/courseware/models.py | 20 +++++++++++++------- lms/djangoapps/courseware/module_render.py | 6 ++---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index aa9f946733..5fae05c177 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -67,7 +67,7 @@ class StudentModuleCache(object): """ A cache of StudentModules for a specific student """ - def __init__(self, user, descriptors): + def __init__(self, user, descriptors, acquire_lock=False): ''' Find any StudentModule objects that are needed by any descriptor in descriptors. Avoids making multiple queries to the database. @@ -86,17 +86,23 @@ class StudentModuleCache(object): self.cache = [] chunk_size = 500 for id_chunk in [module_ids[i:i + chunk_size] for i in xrange(0, len(module_ids), chunk_size)]: - self.cache.extend(StudentModule.objects.filter( - student=user, - module_state_key__in=id_chunk) - ) + if acquire_lock: + self.cache.extend(StudentModule.objects.select_for_update().filter( + student=user, + module_state_key__in=id_chunk) + ) + else: + self.cache.extend(StudentModule.objects.filter( + student=user, + module_state_key__in=id_chunk) + ) else: self.cache = [] @classmethod - def cache_for_descriptor_descendents(cls, user, descriptor, depth=None, descriptor_filter=lambda descriptor: True): + def cache_for_descriptor_descendents(cls, user, descriptor, depth=None, descriptor_filter=lambda descriptor: True, acquire_lock=False): """ descriptor: An XModuleDescriptor depth is the number of levels of descendent modules to load StudentModules for, in addition to @@ -122,7 +128,7 @@ class StudentModuleCache(object): descriptors = get_child_descriptors(descriptor, depth, descriptor_filter) - return StudentModuleCache(user, descriptors) + return StudentModuleCache(user, descriptors, acquire_lock) def _get_module_state_keys(self, descriptors): ''' diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 5184cd9866..000d1ca830 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -160,7 +160,6 @@ def get_module(user, request, location, student_module_cache, position=None): instance_state = instance_module.state if instance_module is not None else None shared_state = shared_module.state if shared_module is not None else None - # TODO (vshnayder): fix hardcoded urls (use reverse) # Setup system context for module instance ajax_url = reverse('modx_dispatch', @@ -169,8 +168,6 @@ def get_module(user, request, location, student_module_cache, position=None): dispatch=''), ) - # ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/' - # Fully qualified callback URL for external queueing system xqueue_callback_url = request.build_absolute_uri('/')[:-1] # Trailing slash provided by reverse xqueue_callback_url += reverse('xqueue_callback', @@ -304,7 +301,8 @@ def xqueue_callback(request, course_id, userid, id, dispatch): # Retrieve target StudentModule user = User.objects.get(id=userid) - student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, modulestore().get_item(id)) + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + user, modulestore().get_item(id), depth=0, acquire_lock=True) instance = get_module(user, request, id, student_module_cache) instance_module = get_instance_module(user, instance, student_module_cache)