Commit cacdbc35 by Calen Pennington

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.
parent e6db0af1
...@@ -44,18 +44,7 @@ def chunks(items, chunk_size): ...@@ -44,18 +44,7 @@ def chunks(items, chunk_size):
return (items[i:i + chunk_size] for i in xrange(0, len(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): def _chunked_query(model_class, chunk_field, items, chunk_size=500, **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):
""" """
Queries model_class with `chunk_field` set to chunks of size `chunk_size`, Queries model_class with `chunk_field` set to chunks of size `chunk_size`,
and all other parameters from `**kwargs`. and all other parameters from `**kwargs`.
...@@ -64,7 +53,7 @@ def _chunked_query(model_class, select_for_update, chunk_field, items, chunk_siz ...@@ -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. that can be put into a single query.
""" """
res = chain.from_iterable( 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) for chunk in chunks(items, chunk_size)
) )
return res return res
...@@ -358,11 +347,10 @@ class UserStateCache(object): ...@@ -358,11 +347,10 @@ class UserStateCache(object):
""" """
Cache for Scope.user_state xblock field data. 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._cache = {}
self.course_id = course_id self.course_id = course_id
self.user = user self.user = user
self.select_for_update = select_for_update
def cache_fields(self, fields, xblocks, aside_types): # pylint: disable=unused-argument def cache_fields(self, fields, xblocks, aside_types): # pylint: disable=unused-argument
""" """
...@@ -376,7 +364,6 @@ class UserStateCache(object): ...@@ -376,7 +364,6 @@ class UserStateCache(object):
""" """
query = _chunked_query( query = _chunked_query(
StudentModule, StudentModule,
self.select_for_update,
'module_state_key__in', 'module_state_key__in',
_all_usage_keys(xblocks, aside_types), _all_usage_keys(xblocks, aside_types),
course_id=self.course_id, course_id=self.course_id,
...@@ -549,10 +536,9 @@ class UserStateSummaryCache(DjangoOrmFieldCache): ...@@ -549,10 +536,9 @@ class UserStateSummaryCache(DjangoOrmFieldCache):
""" """
Cache for Scope.user_state_summary xblock field data. 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__() super(UserStateSummaryCache, self).__init__()
self.course_id = course_id self.course_id = course_id
self.select_for_update = select_for_update
def _create_object(self, kvs_key): def _create_object(self, kvs_key):
""" """
...@@ -583,7 +569,6 @@ class UserStateSummaryCache(DjangoOrmFieldCache): ...@@ -583,7 +569,6 @@ class UserStateSummaryCache(DjangoOrmFieldCache):
""" """
return _chunked_query( return _chunked_query(
XModuleUserStateSummaryField, XModuleUserStateSummaryField,
self.select_for_update,
'usage_id__in', 'usage_id__in',
_all_usage_keys(xblocks, aside_types), _all_usage_keys(xblocks, aside_types),
field_name__in=set(field.name for field in fields), field_name__in=set(field.name for field in fields),
...@@ -612,10 +597,9 @@ class PreferencesCache(DjangoOrmFieldCache): ...@@ -612,10 +597,9 @@ class PreferencesCache(DjangoOrmFieldCache):
""" """
Cache for Scope.preferences xblock field data. Cache for Scope.preferences xblock field data.
""" """
def __init__(self, user, select_for_update=False): def __init__(self, user):
super(PreferencesCache, self).__init__() super(PreferencesCache, self).__init__()
self.user = user self.user = user
self.select_for_update = select_for_update
def _create_object(self, kvs_key): def _create_object(self, kvs_key):
""" """
...@@ -647,7 +631,6 @@ class PreferencesCache(DjangoOrmFieldCache): ...@@ -647,7 +631,6 @@ class PreferencesCache(DjangoOrmFieldCache):
""" """
return _chunked_query( return _chunked_query(
XModuleStudentPrefsField, XModuleStudentPrefsField,
self.select_for_update,
'module_type__in', 'module_type__in',
_all_block_types(xblocks, aside_types), _all_block_types(xblocks, aside_types),
student=self.user.pk, student=self.user.pk,
...@@ -677,10 +660,9 @@ class UserInfoCache(DjangoOrmFieldCache): ...@@ -677,10 +660,9 @@ class UserInfoCache(DjangoOrmFieldCache):
""" """
Cache for Scope.user_info xblock field data Cache for Scope.user_info xblock field data
""" """
def __init__(self, user, select_for_update=False): def __init__(self, user):
super(UserInfoCache, self).__init__() super(UserInfoCache, self).__init__()
self.user = user self.user = user
self.select_for_update = select_for_update
def _create_object(self, kvs_key): def _create_object(self, kvs_key):
""" """
...@@ -709,9 +691,7 @@ class UserInfoCache(DjangoOrmFieldCache): ...@@ -709,9 +691,7 @@ class UserInfoCache(DjangoOrmFieldCache):
aside_types (list of str): Asides to load field for (which annotate the supplied aside_types (list of str): Asides to load field for (which annotate the supplied
xblocks). xblocks).
""" """
return _query( return XModuleStudentInfoField.objects.filter(
XModuleStudentInfoField,
self.select_for_update,
student=self.user.pk, student=self.user.pk,
field_name__in=set(field.name for field in fields), field_name__in=set(field.name for field in fields),
) )
...@@ -751,11 +731,9 @@ class FieldDataCache(object): ...@@ -751,11 +731,9 @@ class FieldDataCache(object):
descriptors: A list of XModuleDescriptors. descriptors: A list of XModuleDescriptors.
course_id: The id of the current course course_id: The id of the current course
user: The user for which to cache data 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. 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: if asides is None:
self.asides = [] self.asides = []
else: else:
...@@ -769,19 +747,15 @@ class FieldDataCache(object): ...@@ -769,19 +747,15 @@ class FieldDataCache(object):
Scope.user_state: UserStateCache( Scope.user_state: UserStateCache(
self.user, self.user,
self.course_id, self.course_id,
self.select_for_update,
), ),
Scope.user_info: UserInfoCache( Scope.user_info: UserInfoCache(
self.user, self.user,
self.select_for_update,
), ),
Scope.preferences: PreferencesCache( Scope.preferences: PreferencesCache(
self.user, self.user,
self.select_for_update,
), ),
Scope.user_state_summary: UserStateSummaryCache( Scope.user_state_summary: UserStateSummaryCache(
self.course_id, self.course_id,
self.select_for_update,
), ),
} }
self.add_descriptors_to_cache(descriptors) self.add_descriptors_to_cache(descriptors)
...@@ -849,7 +823,7 @@ class FieldDataCache(object): ...@@ -849,7 +823,7 @@ class FieldDataCache(object):
the supplied descriptor. If depth is None, load all descendent StudentModules 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 descriptor_filter is a function that accepts a descriptor and return wether the StudentModule
should be cached 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 = FieldDataCache([], course_id, user, select_for_update, asides=asides)
cache.add_descriptor_descendents(descriptor, depth, descriptor_filter) cache.add_descriptor_descendents(descriptor, depth, descriptor_filter)
......
...@@ -737,7 +737,6 @@ def load_single_xblock(request, user_id, course_id, usage_key_string): ...@@ -737,7 +737,6 @@ def load_single_xblock(request, user_id, course_id, usage_key_string):
user, user,
modulestore().get_item(usage_key), modulestore().get_item(usage_key),
depth=0, depth=0,
select_for_update=True
) )
instance = get_module(user, request, usage_key, field_data_cache, grade_bucket_type='xqueue') instance = get_module(user, request, usage_key, field_data_cache, grade_bucket_type='xqueue')
if instance is None: if instance is None:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment