Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove full table scan of VerificationDeadline. (#21119)

Browse Source 
* Remove full table scan of VerificationDeadline.

Before this commit, we were doing a full table scan of
student_verificationdeadline, loading the results into a giant
dict, and reading/writing that to the cache. This was fine when the
code was introduced and there were dozens of courses, but now that
we're over 12K courses, it's becoming a major performance issue for
the Student Dashboard.

This uses a subquery to the course enrollment table so that we're
only ever pulling back the deadlines to a student's enrolled courses
for any given request. It removes the cache access entirely.
This commit is contained in:
David Ormsbee
2019-07-17 10:25:08 -04:00
committed by GitHub
parent f88bb74550
commit a712f1f93b
3 changed files with 18 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
common/djangoapps/student/helpers.py
View File

@@ -37,6 +37,7 @@ from openedx.core.djangoapps.theming import helpers as theming_helpers
from openedx.core.djangoapps.theming.helpers import get_themes
from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect
from student.models import (
CourseEnrollment,
LinkedInAddToProfileConfiguration,
Registration,
UserAttribute,
@@ -118,9 +119,9 @@ def check_verify_status_by_course(user, course_enrollments):
verification_expiring_soon = is_verification_expiring_soon(expiration_datetime)
# Retrieve verification deadlines for the enrolled courses
enrolled_course_keys = [enrollment.course_id for enrollment in course_enrollments]
course_deadlines = VerificationDeadline.deadlines_for_courses(enrolled_course_keys)
course_deadlines = VerificationDeadline.deadlines_for_enrollments(
CourseEnrollment.enrollments_for_user(user)
)
recent_verification_datetime = None
for enrollment in course_enrollments:

37
lms/djangoapps/verify_student/models.py
View File

@@ -1007,8 +1007,6 @@ class VerificationDeadline(TimeStampedModel):
# overwrite the manual setting of the field.
deadline_is_explicit = models.BooleanField(default=False)
ALL_DEADLINES_CACHE_KEY = "verify_student.all_verification_deadlines"
@classmethod
def set_deadline(cls, course_key, deadline, is_explicit=False):
"""
@@ -1037,29 +1035,27 @@ class VerificationDeadline(TimeStampedModel):
record.save()
@classmethod
def deadlines_for_courses(cls, course_keys):
def deadlines_for_enrollments(cls, enrollments_qs):
"""
Retrieve verification deadlines for particular courses.
Retrieve verification deadlines for a user's enrolled courses.
Arguments:
course_keys (list): List of `CourseKey`s.
enrollments_qs: CourseEnrollment queryset. For performance reasons
we want the queryset here instead of passing in a
big list of course_keys in an "SELECT IN" query.
If we have a queryset, Django is smart enough to do
a performant subquery at the MySQL layer instead of
passing down all the course_keys through Python.
Returns:
dict: Map of course keys to datetimes (verification deadlines)
"""
all_deadlines = cache.get(cls.ALL_DEADLINES_CACHE_KEY)
if all_deadlines is None:
all_deadlines = {
deadline.course_key: deadline.deadline
for deadline in VerificationDeadline.objects.all()
}
cache.set(cls.ALL_DEADLINES_CACHE_KEY, all_deadlines)
verification_deadlines = VerificationDeadline.objects.filter(
course_key__in=enrollments_qs.values('course_id')
)
return {
course_key: all_deadlines[course_key]
for course_key in course_keys
if course_key in all_deadlines
deadline.course_key: deadline.deadline
for deadline in verification_deadlines
}
@classmethod
@@ -1079,10 +1075,3 @@ class VerificationDeadline(TimeStampedModel):
return deadline.deadline
except cls.DoesNotExist:
return None
@receiver(models.signals.post_save, sender=VerificationDeadline)
@receiver(models.signals.post_delete, sender=VerificationDeadline)
def invalidate_deadline_caches(sender, **kwargs): # pylint: disable=unused-argument
"""Invalidate the cached verification deadline information. """
cache.delete(VerificationDeadline.ALL_DEADLINES_CACHE_KEY)

47
lms/djangoapps/verify_student/tests/test_models.py
View File

@@ -19,8 +19,7 @@ from lms.djangoapps.verify_student.models import (
SoftwareSecurePhotoVerification,
SSOVerification,
ManualVerification,
VerificationDeadline,
VerificationException
VerificationException,
)
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
from student.tests.factories import UserFactory
@@ -438,47 +437,3 @@ class ManualVerificationTest(TestVerification):
user = UserFactory.create()
verification = ManualVerification.objects.create(user=user)
self.verification_active_at_datetime(verification)
class VerificationDeadlineTest(CacheIsolationTestCase):
"""
Tests for the VerificationDeadline model.
"""
ENABLED_CACHES = ['default']
def test_caching(self):
deadlines = {
CourseKey.from_string("edX/DemoX/Fall"): now(),
CourseKey.from_string("edX/DemoX/Spring"): now() + timedelta(days=1)
}
course_keys = deadlines.keys()
# Initially, no deadlines are set
with self.assertNumQueries(1):
all_deadlines = VerificationDeadline.deadlines_for_courses(course_keys)
self.assertEqual(all_deadlines, {})
# Create the deadlines
for course_key, deadline in deadlines.iteritems():
VerificationDeadline.objects.create(
course_key=course_key,
deadline=deadline,
)
# Warm the cache
with self.assertNumQueries(1):
VerificationDeadline.deadlines_for_courses(course_keys)
# Load the deadlines from the cache
with self.assertNumQueries(0):
all_deadlines = VerificationDeadline.deadlines_for_courses(course_keys)
self.assertEqual(all_deadlines, deadlines)
# Delete the deadlines
VerificationDeadline.objects.all().delete()
# Verify that the deadlines are updated correctly
with self.assertNumQueries(1):
all_deadlines = VerificationDeadline.deadlines_for_courses(course_keys)
self.assertEqual(all_deadlines, {})