Add request caching to reduce ORM queries

This adds request caching to the following places: * course expiration wrapper (displayed in Units) * offer banner generation (displayed in Units) * get_enrollment * user_by_anonymous_id * youtube_disabled_for_course On a sample course with edx-val enabled, this reduced the queries for a large sequence from 450 to 155.
2019-12-04 19:39:18 -05:00
parent e326520c03
commit 2cb2265ffe
11 changed files with 147 additions and 45 deletions
--- a/common/djangoapps/course_modes/views.py
+++ b/common/djangoapps/course_modes/views.py
@@ -36,7 +36,6 @@ from openedx.core.djangoapps.embargo import api as embargo_api
 from openedx.core.djangoapps.enrollments.permissions import ENROLL_IN_COURSE
 from openedx.features.content_type_gating.models import ContentTypeGatingConfig
 from openedx.features.course_duration_limits.models import CourseDurationLimitConfig
-from openedx.features.discounts.utils import get_first_purchase_offer_banner_fragment
 from openedx.features.discounts.applicability import discount_percentage
 from student.models import CourseEnrollment
 from util.db import outer_atomic
--- a/common/djangoapps/student/models.py
+++ b/common/djangoapps/student/models.py
@@ -209,9 +209,17 @@ def user_by_anonymous_id(uid):
    if uid is None:
        return None

+    request_cache = RequestCache('user_by_anonymous_id')
+    cache_response = request_cache.get_cached_response(uid)
+    if cache_response.is_found:
+        return cache_response.value
+
    try:
-        return User.objects.get(anonymoususerid__anonymous_user_id=uid)
+        user = User.objects.get(anonymoususerid__anonymous_user_id=uid)
+        request_cache.set(uid, user)
+        return user
    except ObjectDoesNotExist:
+        request_cache.set(uid, None)
        return None


@@ -1207,13 +1215,24 @@ class CourseEnrollment(models.Model):
        if user.is_anonymous:
            return None
        try:
+            request_cache = RequestCache('get_enrollment')
+            if select_related:
+                cache_key = (user.id, course_key, ','.join(select_related))
+            else:
+                cache_key = (user.id, course_key)
+            cache_response = request_cache.get_cached_response(cache_key)
+            if cache_response.is_found:
+                return cache_response.value
+
            query = cls.objects
            if select_related is not None:
                query = query.select_related(*select_related)
-            return query.get(
+            enrollment = query.get(
                user=user,
                course_id=course_key
            )
+            request_cache.set(cache_key, enrollment)
+            return enrollment
        except cls.DoesNotExist:
            return None

@@ -1257,6 +1276,8 @@ class CourseEnrollment(models.Model):
        This saves immediately.

        """
+        RequestCache('get_enrollment').clear()
+
        activation_changed = False
        # if is_active is None, then the call to update_enrollment didn't specify
        # any value, so just leave is_active as it is
@@ -1488,6 +1509,8 @@ class CourseEnrollment(models.Model):

        `skip_refund` can be set to True to avoid the refund process.
        """
+        RequestCache('get_enrollment').clear()
+
        try:
            record = cls.objects.get(user=user, course_id=course_id)
            record.update_enrollment(is_active=False, skip_refund=skip_refund)
@@ -1509,6 +1532,8 @@ class CourseEnrollment(models.Model):

        `course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
        """
+        RequestCache('get_enrollment').clear()
+
        try:
            user = User.objects.get(email=email)
            return cls.unenroll(user, course_id)
--- a/common/lib/xmodule/xmodule/video_module/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module/video_module.py
@@ -22,6 +22,7 @@ from operator import itemgetter

 import six
 from django.conf import settings
+from edx_django_utils.cache import RequestCache
 from lxml import etree
 from opaque_keys.edx.locator import AssetLocator
 from web_fragments.fragment import Fragment
@@ -202,10 +203,14 @@ class VideoBlock(
    def youtube_disabled_for_course(self):
        if not self.location.context_key.is_course:
            return False  # Only courses have this flag
-        if CourseYoutubeBlockedFlag.feature_enabled(self.location.course_key):
-            return True
-        else:
-            return False
+        request_cache = RequestCache('youtube_disabled_for_course')
+        cache_response = request_cache.get_cached_response(self.location.context_key)
+        if cache_response.is_found:
+            return cache_response.value
+
+        youtube_is_disabled = CourseYoutubeBlockedFlag.feature_enabled(self.location.course_key)
+        request_cache.set(self.location.context_key, youtube_is_disabled)
+        return youtube_is_disabled

    def prioritize_hls(self, youtube_streams, html5_sources):
        """
--- a/lms/djangoapps/ccx/tests/test_field_override_performance.py
+++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py
@@ -244,7 +244,7 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
    __test__ = True

    # TODO: decrease query count as part of REVO-28
-    QUERY_COUNT = 32
+    QUERY_COUNT = 29
    TEST_DATA = {
        # (providers, course_width, enable_ccx, view_as_ccx): (
        #     # of sql queries to default,
@@ -273,7 +273,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
    __test__ = True

    # TODO: decrease query count as part of REVO-28
-    QUERY_COUNT = 32
+    QUERY_COUNT = 29

    TEST_DATA = {
        ('no_overrides', 1, True, False): (QUERY_COUNT, 3),
--- a/lms/djangoapps/courseware/tests/test_views.py
+++ b/lms/djangoapps/courseware/tests/test_views.py
@@ -260,8 +260,8 @@ class IndexQueryTestCase(ModuleStoreTestCase):
    NUM_PROBLEMS = 20

    @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 10, 182),
-        (ModuleStoreEnum.Type.split, 4, 178),
+        (ModuleStoreEnum.Type.mongo, 10, 176),
+        (ModuleStoreEnum.Type.split, 4, 174),
    )
    @ddt.unpack
    def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count):
@@ -1494,8 +1494,8 @@ class ProgressPageTests(ProgressPageBaseTests):
            self.assertContains(resp, u"Download Your Certificate")

    @ddt.data(
-        (True, 52),
-        (False, 51)
+        (True, 49),
+        (False, 48)
    )
    @ddt.unpack
    def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1508,8 +1508,8 @@ class ProgressPageTests(ProgressPageBaseTests):

    @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
    @ddt.data(
-        (False, 60, 40),
-        (True, 51, 35)
+        (False, 57, 37),
+        (True, 48, 32)
    )
    @ddt.unpack
    def test_progress_queries(self, enable_waffle, initial, subsequent):
--- a/lms/djangoapps/discussion/tests/test_views.py
+++ b/lms/djangoapps/discussion/tests/test_views.py
@@ -464,18 +464,18 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase):
        # course is outside the context manager that is verifying the number of queries,
        # and with split mongo, that method ends up querying disabled_xblocks (which is then
        # cached and hence not queried as part of call_single_thread).
-        (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 23, 8),
-        (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 23, 8),
+        (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 23, 7),
+        (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 23, 7),
        # split mongo: 3 queries, regardless of thread response size.
-        (ModuleStoreEnum.Type.split, False, 1, 3, 3, 23, 8),
-        (ModuleStoreEnum.Type.split, False, 50, 3, 3, 23, 8),
+        (ModuleStoreEnum.Type.split, False, 1, 3, 3, 23, 7),
+        (ModuleStoreEnum.Type.split, False, 50, 3, 3, 23, 7),

        # Enabling Enterprise integration should have no effect on the number of mongo queries made.
-        (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 23, 8),
-        (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 23, 8),
+        (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 23, 7),
+        (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 23, 7),
        # split mongo: 3 queries, regardless of thread response size.
-        (ModuleStoreEnum.Type.split, True, 1, 3, 3, 23, 8),
-        (ModuleStoreEnum.Type.split, True, 50, 3, 3, 23, 8),
+        (ModuleStoreEnum.Type.split, True, 1, 3, 3, 23, 7),
+        (ModuleStoreEnum.Type.split, True, 50, 3, 3, 23, 7),
    )
    @ddt.unpack
    def test_number_of_mongo_queries(
--- a/lms/djangoapps/grades/tests/test_course_grade_factory.py
+++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py
@@ -100,32 +100,32 @@ class TestCourseGradeFactory(GradeTestBase):
        with self.assertNumQueries(3), mock_get_score(1, 2):
            _assert_read(expected_pass=False, expected_percent=0)  # start off with grade of 0

-        num_queries = 46
+        num_queries = 45
        with self.assertNumQueries(num_queries), mock_get_score(1, 2):
            grade_factory.update(self.request.user, self.course, force_update_subsections=True)

-        with self.assertNumQueries(4):
+        with self.assertNumQueries(3):
            _assert_read(expected_pass=True, expected_percent=0.5)  # updated to grade of .5

-        num_queries = 8
+        num_queries = 7
        with self.assertNumQueries(num_queries), mock_get_score(1, 4):
            grade_factory.update(self.request.user, self.course, force_update_subsections=False)

-        with self.assertNumQueries(4):
+        with self.assertNumQueries(3):
            _assert_read(expected_pass=True, expected_percent=0.5)  # NOT updated to grade of .25

-        num_queries = 25
+        num_queries = 24
        with self.assertNumQueries(num_queries), mock_get_score(2, 2):
            grade_factory.update(self.request.user, self.course, force_update_subsections=True)

-        with self.assertNumQueries(4):
+        with self.assertNumQueries(3):
            _assert_read(expected_pass=True, expected_percent=1.0)  # updated to grade of 1.0

-        num_queries = 29
+        num_queries = 28
        with self.assertNumQueries(num_queries), mock_get_score(0, 0):  # the subsection now is worth zero
            grade_factory.update(self.request.user, self.course, force_update_subsections=True)

-        with self.assertNumQueries(4):
+        with self.assertNumQueries(3):
            _assert_read(expected_pass=False, expected_percent=0.0)  # updated to grade of 0.0

    @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
--- a/openedx/features/course_duration_limits/access.py
+++ b/openedx/features/course_duration_limits/access.py
@@ -11,6 +11,7 @@ import six
 from django.utils import timezone
 from django.utils.translation import get_language
 from django.utils.translation import ugettext as _
+from edx_django_utils.cache import RequestCache
 from web_fragments.fragment import Fragment

 from course_modes.models import CourseMode
@@ -229,11 +230,42 @@ def generate_course_expired_message(user, course):
 def generate_course_expired_fragment(user, course):
    message = generate_course_expired_message(user, course)
    if message:
-        return Fragment(HTML(u"""\
+        return generate_fragment_from_message(message)
+
+
+def generate_fragment_from_message(message):
+    return Fragment(HTML(u"""\
            <div class="course-expiration-message">{}</div>
        """).format(message))


+def generate_course_expired_fragment_from_key(user, course_key):
+    """
+    Like `generate_course_expired_fragment`, but using a CourseKey instead of
+    a CourseOverview and using request-level caching.
+
+    Either returns WebFragment to inject XBlock content into, or None if we
+    shouldn't show a course expired message for this user.
+    """
+    request_cache = RequestCache('generate_course_expired_fragment_from_key')
+    cache_key = u'message:{},{}'.format(user.id, course_key)
+    cache_response = request_cache.get_cached_response(cache_key)
+    if cache_response.is_found:
+        cached_message = cache_response.value
+        # In this case, there is no message to display.
+        if cached_message is None:
+            return None
+        return generate_fragment_from_message(cached_message)
+
+    course = CourseOverview.get_from_id(course_key)
+    message = generate_course_expired_message(user, course)
+    request_cache.set(cache_key, message)
+    if message is None:
+        return None
+
+    return generate_fragment_from_message(message)
+
+
 def course_expiration_wrapper(user, block, view, frag, context):  # pylint: disable=W0613
    """
    An XBlock wrapper that prepends a message to the beginning of a vertical if
@@ -242,9 +274,9 @@ def course_expiration_wrapper(user, block, view, frag, context):  # pylint: disa
    if block.category != "vertical":
        return frag

-    course = CourseOverview.get_from_id(block.course_id)
-    course_expiration_fragment = generate_course_expired_fragment(user, course)
-
+    course_expiration_fragment = generate_course_expired_fragment_from_key(
+        user, block.course_id
+    )
    if not course_expiration_fragment:
        return frag

--- a/openedx/features/course_experience/tests/views/test_course_home.py
+++ b/openedx/features/course_experience/tests/views/test_course_home.py
@@ -219,7 +219,7 @@ class TestCourseHomePage(CourseHomePageTestCase):

        # Fetch the view and verify the query counts
        # TODO: decrease query count as part of REVO-28
-        with self.assertNumQueries(90, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
+        with self.assertNumQueries(74, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
            with check_mongo_calls(4):
                url = course_home_url(self.course)
                self.client.get(url)
--- a/openedx/features/course_experience/tests/views/test_course_updates.py
+++ b/openedx/features/course_experience/tests/views/test_course_updates.py
@@ -134,7 +134,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase):

        # Fetch the view and verify that the query counts haven't changed
        # TODO: decrease query count as part of REVO-28
-        with self.assertNumQueries(53, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
+        with self.assertNumQueries(51, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
            with check_mongo_calls(4):
                url = course_updates_url(self.course)
                self.client.get(url)
--- a/openedx/features/discounts/utils.py
+++ b/openedx/features/discounts/utils.py
@@ -6,6 +6,7 @@ from datetime import datetime

 import six
 from django.utils.translation import ugettext as _
+from edx_django_utils.cache import RequestCache
 import pytz

 from course_modes.models import get_course_prices, format_course_price
@@ -32,8 +33,9 @@ def offer_banner_wrapper(user, block, view, frag, context):  # pylint: disable=W
    if block.category != "vertical":
        return frag

-    course = CourseOverview.get_from_id(block.course_id)
-    offer_banner_fragment = get_first_purchase_offer_banner_fragment(user, course)
+    offer_banner_fragment = get_first_purchase_offer_banner_fragment_from_key(
+        user, block.course_id
+    )

    if not offer_banner_fragment:
        return frag
@@ -97,11 +99,12 @@ def format_strikeout_price(user, course, base_price=None, check_for_discount=Tru
        return (HTML(u"<span class='price'>{}</span>").format(original_price), False)


-def get_first_purchase_offer_banner_fragment(user, course):
+def generate_offer_html(user, course):
    """
-    Return an HTML Fragment with First Purcahse Discount message,
-    which has the discount_expiration_date, price,
-    discount percentage and a link to upgrade.
+    Create the actual HTML object with the offer text in it.
+
+    Returns a openedx.core.djangolib.markup.HTML object, or None if the user
+    should not be shown an offer message.
    """
    if user and not user.is_anonymous and course:
        now = datetime.now(tz=pytz.UTC).strftime(u"%Y-%m-%d %H:%M:%S%z")
@@ -119,7 +122,8 @@ def get_first_purchase_offer_banner_fragment(user, course):
            offer_message = _(u'{banner_open} Upgrade by {discount_expiration_date} and save {percentage}% '
                              u'[{strikeout_price}]{span_close}{br}Discount will be automatically applied at checkout. '
                              u'{a_open}Upgrade Now{a_close}{div_close}')
-            return Fragment(HTML(offer_message).format(
+
+            message_html = HTML(offer_message).format(
                a_open=HTML(u'<a href="{upgrade_link}">').format(
                    upgrade_link=verified_upgrade_deadline_link(user=user, course=course)
                ),
@@ -134,5 +138,42 @@ def get_first_purchase_offer_banner_fragment(user, course):
                span_close=HTML('</span>'),
                div_close=HTML('</div>'),
                strikeout_price=HTML(format_strikeout_price(user, course, check_for_discount=False)[0])
-            ))
+            )
+            return message_html
    return None
+
+
+def get_first_purchase_offer_banner_fragment(user, course):
+    """
+    Return an HTML Fragment with First Purcahse Discount message,
+    which has the discount_expiration_date, price,
+    discount percentage and a link to upgrade.
+    """
+    offer_html = generate_offer_html(user, course)
+    if offer_html is None:
+        return None
+    return Fragment(offer_html)
+
+
+def get_first_purchase_offer_banner_fragment_from_key(user, course_key):
+    """
+    Like `get_first_purchase_offer_banner_fragment`, but using a CourseKey
+    instead of a CourseOverview and using request-level caching.
+
+    Either returns WebFragment to inject XBlock content into, or None if we
+    shouldn't show a first purchase offer message for this user.
+    """
+    request_cache = RequestCache('get_first_purchase_offer_banner_fragment_from_key')
+    cache_key = u'html:{},{}'.format(user.id, course_key)
+    cache_response = request_cache.get_cached_response(cache_key)
+    if cache_response.is_found:
+        cached_html = cache_response.value
+        if cached_html is None:
+            return None
+        return Fragment(cached_html)
+
+    course = CourseOverview.get_from_id(course_key)
+    offer_html = generate_offer_html(user, course)
+    request_cache.set(cache_key, offer_html)
+
+    return Fragment(offer_html)