From cacdbc35d7514ffbb141b3ba97ed58094347afd3 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 24 Apr 2015 12:09:24 -0400 Subject: [PATCH] Remove the ability to `select_for_updates` from FieldDataCache. The only consumer of that functionality (the XQueue callback) already does retries, so newly introduce integrity errors (due to multiple commiters trying to update a StudentModule) won't break the XQueue processing pipeline. --- lms/djangoapps/courseware/model_data.py | 44 +++++----------------- lms/djangoapps/courseware/module_render.py | 1 - 2 files changed, 9 insertions(+), 36 deletions(-) diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index cdcbd4e32f..7abcbc4450 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -44,18 +44,7 @@ def chunks(items, chunk_size): return (items[i:i + chunk_size] for i in xrange(0, len(items), chunk_size)) -def _query(model_class, select_for_update, **kwargs): - """ - Queries model_class with **kwargs, optionally adding select_for_update if - `select_for_update` is True. - """ - query = model_class.objects - if select_for_update: - query = query.select_for_update() - query = query.filter(**kwargs) - return query - -def _chunked_query(model_class, select_for_update, chunk_field, items, chunk_size=500, **kwargs): +def _chunked_query(model_class, chunk_field, items, chunk_size=500, **kwargs): """ Queries model_class with `chunk_field` set to chunks of size `chunk_size`, and all other parameters from `**kwargs`. @@ -64,7 +53,7 @@ def _chunked_query(model_class, select_for_update, chunk_field, items, chunk_siz that can be put into a single query. """ res = chain.from_iterable( - _query(model_class, select_for_update, **dict([(chunk_field, chunk)] + kwargs.items())) + model_class.objects.filter(**dict([(chunk_field, chunk)] + kwargs.items())) for chunk in chunks(items, chunk_size) ) return res @@ -358,11 +347,10 @@ class UserStateCache(object): """ Cache for Scope.user_state xblock field data. """ - def __init__(self, user, course_id, select_for_update=False): + def __init__(self, user, course_id): self._cache = {} self.course_id = course_id self.user = user - self.select_for_update = select_for_update def cache_fields(self, fields, xblocks, aside_types): # pylint: disable=unused-argument """ @@ -376,7 +364,6 @@ class UserStateCache(object): """ query = _chunked_query( StudentModule, - self.select_for_update, 'module_state_key__in', _all_usage_keys(xblocks, aside_types), course_id=self.course_id, @@ -549,10 +536,9 @@ class UserStateSummaryCache(DjangoOrmFieldCache): """ Cache for Scope.user_state_summary xblock field data. """ - def __init__(self, course_id, select_for_update=False): + def __init__(self, course_id): super(UserStateSummaryCache, self).__init__() self.course_id = course_id - self.select_for_update = select_for_update def _create_object(self, kvs_key): """ @@ -583,7 +569,6 @@ class UserStateSummaryCache(DjangoOrmFieldCache): """ return _chunked_query( XModuleUserStateSummaryField, - self.select_for_update, 'usage_id__in', _all_usage_keys(xblocks, aside_types), field_name__in=set(field.name for field in fields), @@ -612,10 +597,9 @@ class PreferencesCache(DjangoOrmFieldCache): """ Cache for Scope.preferences xblock field data. """ - def __init__(self, user, select_for_update=False): + def __init__(self, user): super(PreferencesCache, self).__init__() self.user = user - self.select_for_update = select_for_update def _create_object(self, kvs_key): """ @@ -647,7 +631,6 @@ class PreferencesCache(DjangoOrmFieldCache): """ return _chunked_query( XModuleStudentPrefsField, - self.select_for_update, 'module_type__in', _all_block_types(xblocks, aside_types), student=self.user.pk, @@ -677,10 +660,9 @@ class UserInfoCache(DjangoOrmFieldCache): """ Cache for Scope.user_info xblock field data """ - def __init__(self, user, select_for_update=False): + def __init__(self, user): super(UserInfoCache, self).__init__() self.user = user - self.select_for_update = select_for_update def _create_object(self, kvs_key): """ @@ -709,9 +691,7 @@ class UserInfoCache(DjangoOrmFieldCache): aside_types (list of str): Asides to load field for (which annotate the supplied xblocks). """ - return _query( - XModuleStudentInfoField, - self.select_for_update, + return XModuleStudentInfoField.objects.filter( student=self.user.pk, field_name__in=set(field.name for field in fields), ) @@ -751,11 +731,9 @@ class FieldDataCache(object): descriptors: A list of XModuleDescriptors. course_id: The id of the current course user: The user for which to cache data - select_for_update: True if rows should be locked until end of transaction + select_for_update: Ignored asides: The list of aside types to load, or None to prefetch no asides. ''' - self.select_for_update = select_for_update - if asides is None: self.asides = [] else: @@ -769,19 +747,15 @@ class FieldDataCache(object): Scope.user_state: UserStateCache( self.user, self.course_id, - self.select_for_update, ), Scope.user_info: UserInfoCache( self.user, - self.select_for_update, ), Scope.preferences: PreferencesCache( self.user, - self.select_for_update, ), Scope.user_state_summary: UserStateSummaryCache( self.course_id, - self.select_for_update, ), } self.add_descriptors_to_cache(descriptors) @@ -849,7 +823,7 @@ class FieldDataCache(object): the supplied descriptor. If depth is None, load all descendent StudentModules descriptor_filter is a function that accepts a descriptor and return wether the StudentModule should be cached - select_for_update: Flag indicating whether the rows should be locked until end of transaction + select_for_update: Ignored """ cache = FieldDataCache([], course_id, user, select_for_update, asides=asides) cache.add_descriptor_descendents(descriptor, depth, descriptor_filter) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 827f33921d..7de942c7ec 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -737,7 +737,6 @@ def load_single_xblock(request, user_id, course_id, usage_key_string): user, modulestore().get_item(usage_key), depth=0, - select_for_update=True ) instance = get_module(user, request, usage_key, field_data_cache, grade_bucket_type='xqueue') if instance is None: