Merge pull request #19231 from edx/aed/performance
Prefetch/cache subsection grades for gradebook.
This commit is contained in:
@@ -25,6 +25,7 @@ from lms.djangoapps.grades.course_data import CourseData
|
||||
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
|
||||
from lms.djangoapps.grades.events import SUBSECTION_GRADE_CALCULATED, subsection_grade_calculated
|
||||
from lms.djangoapps.grades.models import (
|
||||
PersistentCourseGrade,
|
||||
PersistentSubsectionGrade,
|
||||
PersistentSubsectionGradeOverride,
|
||||
PersistentSubsectionGradeOverrideHistory
|
||||
@@ -38,6 +39,7 @@ from openedx.core.djangoapps.course_groups import cohorts
|
||||
from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser
|
||||
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin
|
||||
from student.models import CourseEnrollment
|
||||
from student.roles import BulkRoleCache
|
||||
from track.event_transaction_utils import (
|
||||
create_new_event_transaction_id,
|
||||
get_event_transaction_id,
|
||||
@@ -111,6 +113,35 @@ def verify_writable_gradebook_enabled(view_func):
|
||||
return wrapped_function
|
||||
|
||||
|
||||
@contextmanager
|
||||
def bulk_course_grade_context(course_key, users):
|
||||
"""
|
||||
Prefetches grades for the given users in the given course
|
||||
within a context, storing in a RequestCache and deleting
|
||||
on context exit.
|
||||
"""
|
||||
PersistentCourseGrade.prefetch(course_key, users)
|
||||
yield
|
||||
PersistentCourseGrade.clear_prefetched_data(course_key)
|
||||
|
||||
|
||||
@contextmanager
|
||||
def bulk_gradebook_view_context(course_key, users):
|
||||
"""
|
||||
Prefetches all course and subsection grades in the given course for the given
|
||||
list of users, also, fetch all the score relavant data,
|
||||
storing the result in a RequestCache and deleting grades on context exit.
|
||||
"""
|
||||
PersistentSubsectionGrade.prefetch(course_key, users)
|
||||
PersistentCourseGrade.prefetch(course_key, users)
|
||||
CourseEnrollment.bulk_fetch_enrollment_states(users, course_key)
|
||||
cohorts.bulk_cache_cohorts(course_key, users)
|
||||
BulkRoleCache.prefetch(users)
|
||||
yield
|
||||
PersistentSubsectionGrade.clear_prefetched_data(course_key)
|
||||
PersistentCourseGrade.clear_prefetched_data(course_key)
|
||||
|
||||
|
||||
class CourseEnrollmentPagination(CursorPagination):
|
||||
"""
|
||||
Paginates over CourseEnrollment objects.
|
||||
@@ -227,7 +258,7 @@ class GradeViewMixin(DeveloperErrorViewMixin):
|
||||
course_grade = CourseGradeFactory().read(grade_user, course_key=course_key)
|
||||
return Response([self._serialize_user_grade(grade_user, course_key, course_grade)])
|
||||
|
||||
def _iter_user_grades(self, course_key, course_enrollment_filter=None, related_models=None):
|
||||
def _paginate_users(self, course_key, course_enrollment_filter=None, related_models=None):
|
||||
"""
|
||||
Args:
|
||||
course_key (CourseLocator): The course to retrieve grades for.
|
||||
@@ -236,7 +267,7 @@ class GradeViewMixin(DeveloperErrorViewMixin):
|
||||
related_models: Optional list of related models to join to the CourseEnrollment table.
|
||||
|
||||
Returns:
|
||||
An iterator of CourseGrade objects for users enrolled in the given course.
|
||||
A list of users, pulled from a paginated queryset of enrollments, who are enrolled in the given course.
|
||||
"""
|
||||
filter_kwargs = {
|
||||
'course_id': course_key,
|
||||
@@ -248,11 +279,7 @@ class GradeViewMixin(DeveloperErrorViewMixin):
|
||||
enrollments_in_course = enrollments_in_course.select_related(*related_models)
|
||||
|
||||
paged_enrollments = self.paginate_queryset(enrollments_in_course)
|
||||
users = (enrollment.user for enrollment in paged_enrollments)
|
||||
grades = CourseGradeFactory().iter(users, course_key=course_key)
|
||||
|
||||
for user, course_grade, exc in grades:
|
||||
yield user, course_grade, exc
|
||||
return [enrollment.user for enrollment in paged_enrollments]
|
||||
|
||||
def _serialize_user_grade(self, user, course_key, course_grade):
|
||||
"""
|
||||
@@ -409,9 +436,12 @@ class CourseGradesView(GradeViewMixin, PaginatedAPIView):
|
||||
A serializable list of grade responses
|
||||
"""
|
||||
user_grades = []
|
||||
for user, course_grade, exc in self._iter_user_grades(course_key):
|
||||
if not exc:
|
||||
user_grades.append(self._serialize_user_grade(user, course_key, course_grade))
|
||||
users = self._paginate_users(course_key)
|
||||
|
||||
with bulk_course_grade_context(course_key, users):
|
||||
for user, course_grade, exc in CourseGradeFactory().iter(users, course_key=course_key):
|
||||
if not exc:
|
||||
user_grades.append(self._serialize_user_grade(user, course_key, course_grade))
|
||||
|
||||
return self.get_paginated_response(user_grades)
|
||||
|
||||
@@ -650,12 +680,14 @@ class GradebookView(GradeViewMixin, PaginatedAPIView):
|
||||
if request.GET.get('enrollment_mode'):
|
||||
filter_kwargs['mode'] = request.GET.get('enrollment_mode')
|
||||
|
||||
user_grades = self._iter_user_grades(course_key, filter_kwargs, related_models)
|
||||
|
||||
entries = []
|
||||
for user, course_grade, exc in user_grades:
|
||||
if not exc:
|
||||
entries.append(self._gradebook_entry(user, course, course_grade))
|
||||
users = self._paginate_users(course_key, filter_kwargs, related_models)
|
||||
|
||||
with bulk_gradebook_view_context(course_key, users):
|
||||
for user, course_grade, exc in CourseGradeFactory().iter(users, course_key=course_key):
|
||||
if not exc:
|
||||
entries.append(self._gradebook_entry(user, course, course_grade))
|
||||
|
||||
serializer = StudentGradebookEntrySerializer(entries, many=True)
|
||||
return self.get_paginated_response(serializer.data)
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@ persisted, course grades are also immune to changes in course content.
|
||||
import json
|
||||
import logging
|
||||
from base64 import b64encode
|
||||
from collections import namedtuple
|
||||
from collections import defaultdict, namedtuple
|
||||
from hashlib import sha1
|
||||
|
||||
from django.contrib.auth.models import User
|
||||
@@ -23,10 +23,9 @@ from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
|
||||
from coursewarehistoryextended.fields import UnsignedBigIntAutoField, UnsignedBigIntOneToOneField
|
||||
from lms.djangoapps.grades import events
|
||||
from openedx.core.lib.cache_utils import get_cache
|
||||
|
||||
import events
|
||||
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -310,12 +309,15 @@ class PersistentSubsectionGrade(TimeStampedModel):
|
||||
visible_blocks = models.ForeignKey(VisibleBlocks, db_column='visible_blocks_hash', to_field='hashed',
|
||||
on_delete=models.CASCADE)
|
||||
|
||||
_CACHE_NAMESPACE = u'grades.models.PersistentSubsectionGrade'
|
||||
|
||||
@property
|
||||
def full_usage_key(self):
|
||||
"""
|
||||
Returns the "correct" usage key value with the run filled in.
|
||||
"""
|
||||
if self.usage_key.run is None:
|
||||
# pylint: disable=unexpected-keyword-arg,no-value-for-parameter
|
||||
return self.usage_key.replace(course_key=self.course_id)
|
||||
else:
|
||||
return self.usage_key
|
||||
@@ -339,6 +341,28 @@ class PersistentSubsectionGrade(TimeStampedModel):
|
||||
self.first_attempted,
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def prefetch(cls, course_key, users):
|
||||
"""
|
||||
Prefetches grades for the given users in the given course.
|
||||
"""
|
||||
cache_key = cls._cache_key(course_key)
|
||||
get_cache(cls._CACHE_NAMESPACE)[cache_key] = defaultdict(list)
|
||||
cached_grades = get_cache(cls._CACHE_NAMESPACE)[cache_key]
|
||||
queryset = cls.objects.select_related('visible_blocks', 'override').filter(
|
||||
user_id__in=[user.id for user in users],
|
||||
course_id=course_key,
|
||||
)
|
||||
for record in queryset:
|
||||
cached_grades[record.user_id].append(record)
|
||||
|
||||
@classmethod
|
||||
def clear_prefetched_data(cls, course_key):
|
||||
"""
|
||||
Clears prefetched grades for this course from the RequestCache.
|
||||
"""
|
||||
get_cache(cls._CACHE_NAMESPACE).pop(cls._cache_key(course_key), None)
|
||||
|
||||
@classmethod
|
||||
def read_grade(cls, user_id, usage_key):
|
||||
"""
|
||||
@@ -365,10 +389,20 @@ class PersistentSubsectionGrade(TimeStampedModel):
|
||||
user_id: The user associated with the desired grades
|
||||
course_key: The course identifier for the desired grades
|
||||
"""
|
||||
return cls.objects.select_related('visible_blocks', 'override').filter(
|
||||
user_id=user_id,
|
||||
course_id=course_key,
|
||||
)
|
||||
try:
|
||||
prefetched_grades = get_cache(cls._CACHE_NAMESPACE)[cls._cache_key(course_key)]
|
||||
try:
|
||||
return prefetched_grades[user_id]
|
||||
except KeyError:
|
||||
# The user's grade is not in the cached dict of subsection grades,
|
||||
# so return an empty list.
|
||||
return []
|
||||
except KeyError:
|
||||
# subsection grades were not prefetched for the course, so get them from the DB
|
||||
return cls.objects.select_related('visible_blocks', 'override').filter(
|
||||
user_id=user_id,
|
||||
course_id=course_key,
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def update_or_create_grade(cls, **params):
|
||||
@@ -461,6 +495,10 @@ class PersistentSubsectionGrade(TimeStampedModel):
|
||||
def _emit_grade_calculated_event(grade):
|
||||
events.subsection_grade_calculated(grade)
|
||||
|
||||
@classmethod
|
||||
def _cache_key(cls, course_id):
|
||||
return u"subsection_grades_cache.{}".format(course_id)
|
||||
|
||||
|
||||
class PersistentCourseGrade(TimeStampedModel):
|
||||
"""
|
||||
@@ -527,6 +565,13 @@ class PersistentCourseGrade(TimeStampedModel):
|
||||
cls.objects.filter(user_id__in=[user.id for user in users], course_id=course_id)
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def clear_prefetched_data(cls, course_key):
|
||||
"""
|
||||
Clears prefetched grades for this course from the RequestCache.
|
||||
"""
|
||||
get_cache(cls._CACHE_NAMESPACE).pop(cls._cache_key(course_key), None)
|
||||
|
||||
@classmethod
|
||||
def read(cls, user_id, course_id):
|
||||
"""
|
||||
@@ -543,7 +588,7 @@ class PersistentCourseGrade(TimeStampedModel):
|
||||
try:
|
||||
return prefetched_grades[user_id]
|
||||
except KeyError:
|
||||
# user's grade is not in the prefetched list, so
|
||||
# user's grade is not in the prefetched dict, so
|
||||
# assume they have no grade
|
||||
raise cls.DoesNotExist
|
||||
except KeyError:
|
||||
@@ -611,6 +656,15 @@ class PersistentSubsectionGradeOverride(models.Model):
|
||||
|
||||
_CACHE_NAMESPACE = u"grades.models.PersistentSubsectionGradeOverride"
|
||||
|
||||
def __unicode__(self):
|
||||
return u', '.join([
|
||||
u"{}".format(type(self).__name__),
|
||||
u"earned_all_override: {}".format(self.earned_all_override),
|
||||
u"possible_all_override: {}".format(self.possible_all_override),
|
||||
u"earned_graded_override: {}".format(self.earned_graded_override),
|
||||
u"possible_graded_override: {}".format(self.possible_graded_override),
|
||||
])
|
||||
|
||||
@classmethod
|
||||
def prefetch(cls, user_id, course_key):
|
||||
get_cache(cls._CACHE_NAMESPACE)[(user_id, str(course_key))] = {
|
||||
@@ -634,11 +688,6 @@ class PersistentSubsectionGradeOverride(models.Model):
|
||||
pass
|
||||
|
||||
|
||||
def prefetch(user, course_key):
|
||||
PersistentSubsectionGradeOverride.prefetch(user.id, course_key)
|
||||
VisibleBlocks.bulk_read(user.id, course_key)
|
||||
|
||||
|
||||
class PersistentSubsectionGradeOverrideHistory(models.Model):
|
||||
"""
|
||||
A django model tracking persistent grades override audit records.
|
||||
@@ -689,3 +738,8 @@ class PersistentSubsectionGradeOverrideHistory(models.Model):
|
||||
self.action,
|
||||
self.created
|
||||
)
|
||||
|
||||
|
||||
def prefetch(user, course_key):
|
||||
PersistentSubsectionGradeOverride.prefetch(user.id, course_key)
|
||||
VisibleBlocks.bulk_read(user.id, course_key)
|
||||
|
||||
@@ -22,7 +22,7 @@ from instructor_analytics.basic import list_problem_responses
|
||||
from instructor_analytics.csvs import format_dictlist
|
||||
from lms.djangoapps.certificates.models import CertificateWhitelist, GeneratedCertificate, certificate_info_for_user
|
||||
from lms.djangoapps.grades.context import grading_context, grading_context_for_course
|
||||
from lms.djangoapps.grades.models import PersistentCourseGrade
|
||||
from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsectionGrade
|
||||
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
|
||||
from lms.djangoapps.teams.models import CourseTeamMembership
|
||||
from lms.djangoapps.verify_student.services import IDVerificationService
|
||||
@@ -190,6 +190,7 @@ class _CourseGradeBulkContext(object):
|
||||
bulk_cache_cohorts(context.course_id, users)
|
||||
BulkRoleCache.prefetch(users)
|
||||
PersistentCourseGrade.prefetch(context.course_id, users)
|
||||
PersistentSubsectionGrade.prefetch(context.course_id, users)
|
||||
BulkCourseTags.prefetch(context.course_id, users)
|
||||
|
||||
|
||||
|
||||
@@ -413,7 +413,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
|
||||
|
||||
RequestCache.clear_all_namespaces()
|
||||
|
||||
expected_query_count = 48
|
||||
expected_query_count = 49
|
||||
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
|
||||
with check_mongo_calls(mongo_count):
|
||||
with self.assertNumQueries(expected_query_count):
|
||||
|
||||
@@ -74,3 +74,7 @@ MKTG_URLS = {
|
||||
CREDENTIALS_SERVICE_USERNAME = 'credentials_worker'
|
||||
|
||||
COURSE_CATALOG_API_URL = 'http://edx.devstack.discovery:18381/api/v1/'
|
||||
|
||||
# Uncomment the lines below if you'd like to see SQL statements in your devstack LMS log.
|
||||
# LOGGING['handlers']['console']['level'] = 'DEBUG'
|
||||
# LOGGING['loggers']['django.db.backends'] = {'handlers': ['console'], 'level': 'DEBUG', 'propagate': False}
|
||||
|
||||
Reference in New Issue
Block a user