Merge pull request #15089 from edx/neem/redo_bulk_grades

Redo Bulk-reads and Request caching in Course Grade Reports
This commit is contained in:
Nimisha Asthagiri
2017-05-10 18:12:19 -04:00
committed by GitHub
22 changed files with 496 additions and 138 deletions

View File

@@ -14,7 +14,8 @@ from django.utils.translation import ugettext as _
from courseware import courses
from eventtracking import tracker
from request_cache.middleware import RequestCache, request_cached
import request_cache
from request_cache.middleware import request_cached
from student.models import get_user_by_username_or_email
from .models import (
@@ -146,8 +147,45 @@ def get_cohorted_commentables(course_key):
return ans
COHORT_CACHE_NAMESPACE = u"cohorts.get_cohort"
def _cohort_cache_key(user_id, course_key):
"""
Returns the cache key for the given user_id and course_key.
"""
return u"{}.{}".format(user_id, course_key)
def bulk_cache_cohorts(course_key, users):
"""
Pre-fetches and caches the cohort assignments for the
given users, for later fast retrieval by get_cohort.
"""
# before populating the cache with another bulk set of data,
# remove previously cached entries to keep memory usage low.
request_cache.clear_cache(COHORT_CACHE_NAMESPACE)
cache = request_cache.get_cache(COHORT_CACHE_NAMESPACE)
if is_course_cohorted(course_key):
cohorts_by_user = {
membership.user: membership
for membership in
CohortMembership.objects.filter(user__in=users, course_id=course_key).select_related('user__id')
}
for user, membership in cohorts_by_user.iteritems():
cache[_cohort_cache_key(user.id, course_key)] = membership.course_user_group
uncohorted_users = filter(lambda u: u not in cohorts_by_user, users)
else:
uncohorted_users = users
for user in uncohorted_users:
cache[_cohort_cache_key(user.id, course_key)] = None
def get_cohort(user, course_key, assign=True, use_cached=False):
"""Returns the user's cohort for the specified course.
"""
Returns the user's cohort for the specified course.
The cohort for the user is cached for the duration of a request. Pass
use_cached=True to use the cached value instead of fetching from the
@@ -166,19 +204,19 @@ def get_cohort(user, course_key, assign=True, use_cached=False):
Raises:
ValueError if the CourseKey doesn't exist.
"""
request_cache = RequestCache.get_request_cache()
cache_key = u"cohorts.get_cohort.{}.{}".format(user.id, course_key)
cache = request_cache.get_cache(COHORT_CACHE_NAMESPACE)
cache_key = _cohort_cache_key(user.id, course_key)
if use_cached and cache_key in request_cache.data:
return request_cache.data[cache_key]
if use_cached and cache_key in cache:
return cache[cache_key]
request_cache.data.pop(cache_key, None)
cache.pop(cache_key, None)
# First check whether the course is cohorted (users shouldn't be in a cohort
# in non-cohorted courses, but settings can change after course starts)
course_cohort_settings = get_course_cohort_settings(course_key)
if not course_cohort_settings.is_cohorted:
return request_cache.data.setdefault(cache_key, None)
return cache.setdefault(cache_key, None)
# If course is cohorted, check if the user already has a cohort.
try:
@@ -186,7 +224,7 @@ def get_cohort(user, course_key, assign=True, use_cached=False):
course_id=course_key,
user_id=user.id,
)
return request_cache.data.setdefault(cache_key, membership.course_user_group)
return cache.setdefault(cache_key, membership.course_user_group)
except CohortMembership.DoesNotExist:
# Didn't find the group. If we do not want to assign, return here.
if not assign:
@@ -201,7 +239,7 @@ def get_cohort(user, course_key, assign=True, use_cached=False):
user=user,
course_user_group=get_random_cohort(course_key)
)
return request_cache.data.setdefault(cache_key, membership.course_user_group)
return cache.setdefault(cache_key, membership.course_user_group)
except IntegrityError as integrity_error:
# An IntegrityError is raised when multiple workers attempt to
# create the same row in one of the cohort model entries:
@@ -419,21 +457,21 @@ def get_group_info_for_cohort(cohort, use_cached=False):
use_cached=True to use the cached value instead of fetching from the
database.
"""
request_cache = RequestCache.get_request_cache()
cache_key = u"cohorts.get_group_info_for_cohort.{}".format(cohort.id)
cache = request_cache.get_cache(u"cohorts.get_group_info_for_cohort")
cache_key = unicode(cohort.id)
if use_cached and cache_key in request_cache.data:
return request_cache.data[cache_key]
if use_cached and cache_key in cache:
return cache[cache_key]
request_cache.data.pop(cache_key, None)
cache.pop(cache_key, None)
try:
partition_group = CourseUserGroupPartitionGroup.objects.get(course_user_group=cohort)
return request_cache.data.setdefault(cache_key, (partition_group.group_id, partition_group.partition_id))
return cache.setdefault(cache_key, (partition_group.group_id, partition_group.partition_id))
except CourseUserGroupPartitionGroup.DoesNotExist:
pass
return request_cache.data.setdefault(cache_key, (None, None))
return cache.setdefault(cache_key, (None, None))
def set_assignment_type(user_group, assignment_type):

View File

@@ -22,6 +22,7 @@ from model_utils.models import TimeStampedModel
import pytz
from simple_history.models import HistoricalRecords
from openedx.core.djangoapps.xmodule_django.models import CourseKeyField
from request_cache.middleware import ns_request_cached, RequestCache
CREDIT_PROVIDER_ID_REGEX = r"[a-z,A-Z,0-9,\-]+"
@@ -290,6 +291,8 @@ class CreditRequirement(TimeStampedModel):
criteria = JSONField()
active = models.BooleanField(default=True)
CACHE_NAMESPACE = u"credit.CreditRequirement.cache."
class Meta(object):
unique_together = ('namespace', 'name', 'course')
ordering = ["order"]
@@ -331,6 +334,7 @@ class CreditRequirement(TimeStampedModel):
return credit_requirement, created
@classmethod
@ns_request_cached(CACHE_NAMESPACE)
def get_course_requirements(cls, course_key, namespace=None, name=None):
"""
Get credit requirements of a given course.
@@ -392,6 +396,13 @@ class CreditRequirement(TimeStampedModel):
return None
@receiver(models.signals.post_save, sender=CreditRequirement)
@receiver(models.signals.post_delete, sender=CreditRequirement)
def invalidate_credit_requirement_cache(sender, **kwargs): # pylint: disable=unused-argument
"""Invalidate the cache of credit requirements. """
RequestCache.clear_request_cache(name=CreditRequirement.CACHE_NAMESPACE)
class CreditRequirementStatus(TimeStampedModel):
"""
This model represents the status of each requirement.

View File

@@ -664,7 +664,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
self.assertFalse(api.is_user_eligible_for_credit(user.username, self.course_key))
# Satisfy the other requirement
with self.assertNumQueries(25):
with self.assertNumQueries(24):
api.set_credit_requirement_status(
user,
self.course_key,
@@ -718,7 +718,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
# Delete the eligibility entries and satisfy the user's eligibility
# requirement again to trigger eligibility notification
CreditEligibility.objects.all().delete()
with self.assertNumQueries(17):
with self.assertNumQueries(16):
api.set_credit_requirement_status(
user,
self.course_key,

View File

@@ -7,6 +7,8 @@ Stores global metadata using the UserPreference model, and per-course metadata u
UserCourseTag model.
"""
from collections import defaultdict
from request_cache import get_cache
from ..models import UserCourseTag
# Scopes
@@ -15,6 +17,42 @@ from ..models import UserCourseTag
COURSE_SCOPE = 'course'
class BulkCourseTags(object):
CACHE_NAMESPACE = u'user_api.course_tag.api'
@classmethod
def prefetch(cls, course_id, users):
"""
Prefetches the value of the course tags for the specified users
for the specified course_id.
Args:
users: iterator of User objects
course_id: course identifier (CourseKey)
Returns:
course_tags: a dict of dicts,
where the primary key is the user's id
and the secondary key is the course tag's key
"""
course_tags = defaultdict(dict)
for tag in UserCourseTag.objects.filter(user__in=users, course_id=course_id).select_related('user__id'):
course_tags[tag.user.id][tag.key] = tag.value
get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_id)] = course_tags
@classmethod
def get_course_tag(cls, user_id, course_id, key):
return get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_id)][user_id][key]
@classmethod
def is_prefetched(cls, course_id):
return cls._cache_key(course_id) in get_cache(cls.CACHE_NAMESPACE)
@classmethod
def _cache_key(cls, course_id):
return u'course_tag.{}'.format(course_id)
def get_course_tag(user, course_id, key):
"""
Gets the value of the user's course tag for the specified key in the specified
@@ -28,6 +66,11 @@ def get_course_tag(user, course_id, key):
Returns:
string value, or None if there is no value saved
"""
if BulkCourseTags.is_prefetched(course_id):
try:
return BulkCourseTags.get_course_tag(user.id, course_id, key)
except KeyError:
return None
try:
record = UserCourseTag.objects.get(
user=user,

View File

@@ -70,7 +70,7 @@ class RandomUserPartitionScheme(object):
exc_info=True
)
if group is None and assign:
if group is None and assign and not course_tag_api.BulkCourseTags.is_prefetched(course_key):
if not user_partition.groups:
raise UserPartitionError('Cannot assign user to an empty user partition')

View File

@@ -26,6 +26,11 @@ class MemoryCourseTagAPI(object):
"""Gets the value of ``key``"""
self._tags[course_id][key] = value
class BulkCourseTags(object):
@classmethod
def is_prefetched(self, course_id):
return False
class TestRandomUserPartitionScheme(PartitionTestCase):
"""

View File

@@ -5,17 +5,17 @@ from django.db import models
from django.utils.translation import ugettext_lazy
from django.dispatch import receiver
from django.db.models.signals import post_save, pre_save
import logging
from openedx.core.djangoapps.xmodule_django.models import CourseKeyField
from student.models import CourseEnrollment
from lms.djangoapps.courseware.courses import get_course_by_id
from openedx.core.djangoapps.xmodule_django.models import CourseKeyField
from openedx.core.djangoapps.verified_track_content.tasks import sync_cohort_with_mode
from openedx.core.djangoapps.course_groups.cohorts import (
get_course_cohorts, CourseCohort, is_course_cohorted, get_random_cohort
)
from request_cache.middleware import ns_request_cached, RequestCache
from student.models import CourseEnrollment
import logging
log = logging.getLogger(__name__)
@@ -97,6 +97,8 @@ class VerifiedTrackCohortedCourse(models.Model):
enabled = models.BooleanField()
CACHE_NAMESPACE = u"verified_track_content.VerifiedTrackCohortedCourse.cache."
def __unicode__(self):
return u"Course: {}, enabled: {}".format(unicode(self.course_key), self.enabled)
@@ -119,6 +121,7 @@ class VerifiedTrackCohortedCourse(models.Model):
return None
@classmethod
@ns_request_cached(CACHE_NAMESPACE)
def is_verified_track_cohort_enabled(cls, course_key):
"""
Checks whether or not verified track cohort is enabled for the given course.
@@ -134,3 +137,10 @@ class VerifiedTrackCohortedCourse(models.Model):
return cls.objects.get(course_key=course_key).enabled
except cls.DoesNotExist:
return False
@receiver(models.signals.post_save, sender=VerifiedTrackCohortedCourse)
@receiver(models.signals.post_delete, sender=VerifiedTrackCohortedCourse)
def invalidate_verified_track_cache(sender, **kwargs): # pylint: disable=unused-argument
"""Invalidate the cache of VerifiedTrackCohortedCourse. """
RequestCache.clear_request_cache(name=VerifiedTrackCohortedCourse.CACHE_NAMESPACE)