diff --git a/lms/djangoapps/grades/api/v1/views.py b/lms/djangoapps/grades/api/v1/views.py index fc539c3a21..451a59a933 100644 --- a/lms/djangoapps/grades/api/v1/views.py +++ b/lms/djangoapps/grades/api/v1/views.py @@ -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) diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index b90cd000e0..d860ec72ce 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -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) diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py index 01bd3830bd..db3cabb90b 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/grades.py +++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py @@ -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) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 4cf42490b5..11f16658a5 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -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): diff --git a/lms/envs/devstack_docker.py b/lms/envs/devstack_docker.py index 115ff5b9bd..4d8ab29d11 100644 --- a/lms/envs/devstack_docker.py +++ b/lms/envs/devstack_docker.py @@ -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}