feat: cache course index queries

common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py

@@ -25,7 +25,8 @@
 from xmodule.modulestore import BlockData
 from xmodule.modulestore.split_mongo import BlockKey
 from xmodule.mongo_utils import connect_to_mongodb, create_collection_index
-
+from openedx.core.lib.cache_utils import request_cached
+from edx_django_utils.cache import RequestCache
 
 log = logging.getLogger(__name__)
 
@@ -261,6 +262,9 @@ def __init__(
         # only before returning. Also makes pymongo report write errors.
         kwargs['w'] = 1
 
+        #make sure the course index cache is fresh.
+        RequestCache(namespace="course_index_cache").clear()
+
         self.database = connect_to_mongodb(
             db, host,
             port=port, tz_aware=tz_aware, user=user, password=password,
@@ -532,6 +536,7 @@ def close_connections(self):
         """
         Closes any open connections to the underlying databases
         """
+        RequestCache(namespace="course_index_cache").clear()
         self.database.client.close()
 
     def _drop_database(self, database=True, collections=True, connections=True):
@@ -546,6 +551,7 @@ def _drop_database(self, database=True, collections=True, connections=True):
 
         If connections is True, then close the connection to the database as well.
         """
+        RequestCache(namespace="course_index_cache").clear()
         connection = self.database.client
 
         if database:
@@ -571,7 +577,13 @@ class DjangoFlexPersistenceBackend(MongoPersistenceBackend):
 
     # Structures and definitions are only supported in MongoDB for now.
     # Course indexes are read from MySQL and written to both MongoDB and MySQL
-
+    # Course indexes are cached within the process using their key and ignore_case atrributes as keys.
+    # This method is request cached. The keys to the cache are the arguements to the method.
+    # The `self` arguement is discarded as a key using an isinstance check.
+    # This is because the DjangoFlexPersistenceBackend could be different in reference to the same course key.
+    @request_cached(
+        "course_index_cache",
+        arg_map_function=lambda arg: str(arg) if not isinstance(arg, DjangoFlexPersistenceBackend) else "")
     def get_course_index(self, key, ignore_case=False):
         """
         Get the course_index from the persistence mechanism whose id is the given key
@@ -650,6 +662,10 @@ def insert_course_index(self, course_index, course_context=None):  # pylint: dis
         """
         Create the course_index in the db
         """
+        # clear the whole course_index request cache, required for sucessfully cloning a course.
+        # This is a relatively large hammer for the problem, but we mostly only use one course at a time.
+        RequestCache(namespace="course_index_cache").clear()
+
         course_index['last_update'] = datetime.datetime.now(pytz.utc)
         new_index = SplitModulestoreCourseIndex(**SplitModulestoreCourseIndex.fields_from_v1_schema(course_index))
         new_index.save()
@@ -669,6 +685,7 @@ def update_course_index(self, course_index, from_index=None, course_context=None
         # "last_update not only tells us when this course was last updated but also helps prevent collisions"
         # This code is just copying the behavior of the existing MongoPersistenceBackend
         # See https://github.com/edx/edx-platform/pull/5200 for context
+        RequestCache(namespace="course_index_cache").clear()
         course_index['last_update'] = datetime.datetime.now(pytz.utc)
         # Find the SplitModulestoreCourseIndex entry that we'll be updating:
         try:
@@ -730,6 +747,7 @@ def delete_course_index(self, course_key):
         """
         Delete the course_index from the persistence mechanism whose id is the given course_index
         """
+        RequestCache(namespace="course_index_cache").clear()
         SplitModulestoreCourseIndex.objects.filter(course_id=course_key).delete()
         # TEMP: Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well:
         super().delete_course_index(course_key)
@@ -738,6 +756,7 @@ def _drop_database(self, database=True, collections=True, connections=True):
         """
         Reset data for testing.
         """
+        RequestCache(namespace="course_index_cache").clear()
         try:
             SplitModulestoreCourseIndex.objects.all().delete()
         except TransactionManagementError as err:

common/lib/xmodule/xmodule/modulestore/tests/django_utils.py

@@ -275,7 +275,7 @@ def my_test(self):
     """
     MODULESTORE = functools.partial(mixed_store_config, mkdtemp_clean(), {})
     CONTENTSTORE = functools.partial(contentstore_config)
-    ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
+    ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache', 'course_index_cache']
 
     # List of modulestore signals enabled for this test. Defaults to an empty
     # list. The list of signals available is found on the SignalHandler class,

common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py

@@ -368,7 +368,7 @@ def test_duplicate_course_error_with_different_case_ids(self, default_store):
     #    fake: one w/ wildcard version
     # split: has one lookup for the course and then one for the course items
     #    but the active_versions check is done in MySQL
-    @ddt.data((ModuleStoreEnum.Type.mongo, [1, 1], 0), (ModuleStoreEnum.Type.split, [2, 2], 0))
+    @ddt.data((ModuleStoreEnum.Type.mongo, [1, 1], 0), (ModuleStoreEnum.Type.split, [2, 1], 0))
     @ddt.unpack
     def test_has_item(self, default_ms, max_find, max_send):
         self.initdb(default_ms)
@@ -391,7 +391,7 @@ def test_has_item(self, default_ms, max_find, max_send):
     # split:
     #   problem: active_versions, structure
     #   non-existent problem: ditto
-    @ddt.data((ModuleStoreEnum.Type.mongo, 0, [3, 2], 0), (ModuleStoreEnum.Type.split, 0, [2, 2], 0))
+    @ddt.data((ModuleStoreEnum.Type.mongo, 0, [3, 2], 0), (ModuleStoreEnum.Type.split, 0, [2, 1], 0))
     @ddt.unpack
     def test_get_item(self, default_ms, num_mysql, max_find, max_send):
         self.initdb(default_ms)
@@ -414,7 +414,7 @@ def test_get_item(self, default_ms, num_mysql, max_find, max_send):
     # Split:
     #    mysql: fetch course's active version from SplitModulestoreCourseIndex, spurious refetch x2
     #    find: get structure
-    @ddt.data((ModuleStoreEnum.Type.mongo, 0, 14, 0), (ModuleStoreEnum.Type.split, 0, 4, 0))
+    @ddt.data((ModuleStoreEnum.Type.mongo, 0, 14, 0), (ModuleStoreEnum.Type.split, 0, 3, 0))
     @ddt.unpack
     def test_get_items(self, default_ms, num_mysql, max_find, max_send):
         self.initdb(default_ms)
@@ -522,7 +522,7 @@ def test_get_items_include_orphans(self, default_ms, expected_items_in_tree, orp
     #    mysql: SplitModulestoreCourseIndex - select 2x (by course_id, by objectid), update, update historical record
     #    find: definitions (calculator field), structures
     #    sends: 2 sends to update index & structure (note, it would also be definition if a content field changed)
-    @ddt.data((ModuleStoreEnum.Type.mongo, 0, 7, 5), (ModuleStoreEnum.Type.split, 3, 3, 2))
+    @ddt.data((ModuleStoreEnum.Type.mongo, 0, 7, 5), (ModuleStoreEnum.Type.split, 3, 2, 2))
     @ddt.unpack
     def test_update_item(self, default_ms, num_mysql, max_find, max_send):
         """
@@ -1043,7 +1043,7 @@ def test_delete_draft_vertical(self, default_ms, num_mysql, max_find, max_send):
     #          executed twice, possibly unnecessarily)
     #   find: 2 reads of structure, definition (s/b lazy; so, unnecessary),
     #         plus 1 wildcard find in draft mongo which has none
-    @ddt.data((ModuleStoreEnum.Type.mongo, 0, 3, 0), (ModuleStoreEnum.Type.split, 0, 6, 0))
+    @ddt.data((ModuleStoreEnum.Type.mongo, 0, 3, 0), (ModuleStoreEnum.Type.split, 0, 5, 0))
     @ddt.unpack
     def test_get_courses(self, default_ms, num_mysql, max_find, max_send):
         self.initdb(default_ms)
@@ -1638,7 +1638,7 @@ def test_get_parent_location_draft(self, default_ms):
     #    8-9. get vertical, compute inheritance
     #    10-11. get other vertical_x1b (why?) and compute inheritance
     # Split: loading structure from mongo (also loads active version from MySQL, not tracked here)
-    @ddt.data((ModuleStoreEnum.Type.mongo, 0, [12, 3], 0), (ModuleStoreEnum.Type.split, 0, [3, 2], 0))
+    @ddt.data((ModuleStoreEnum.Type.mongo, 0, [12, 3], 0), (ModuleStoreEnum.Type.split, 0, [3, 1], 0))
     @ddt.unpack
     def test_path_to_location(self, default_ms, num_mysql, num_finds, num_sends):
         """
@@ -1802,6 +1802,7 @@ def test_reset_course_to_version(self):
         # Find the version_guid of our course by diving into Split Mongo.
         split = self._get_split_modulestore()
         course_index = split.get_course_index(self.course.location.course_key)
+        log.warning(f"Banana course index: {course_index}")
         original_version_guid = course_index["versions"]["published-branch"]
 
         # Reset course to currently-published version.

lms/djangoapps/course_api/blocks/tests/test_api.py

@@ -226,8 +226,8 @@ def test_query_counts_cached(self, store_type, with_storage_backing):
     @ddt.data(
         (ModuleStoreEnum.Type.mongo, 5, True, 24),
         (ModuleStoreEnum.Type.mongo, 5, False, 14),
-        (ModuleStoreEnum.Type.split, 3, True, 24),
-        (ModuleStoreEnum.Type.split, 3, False, 14),
+        (ModuleStoreEnum.Type.split, 2, True, 24),
+        (ModuleStoreEnum.Type.split, 2, False, 14),
     )
     @ddt.unpack
     def test_query_counts_uncached(self, store_type, expected_mongo_queries, with_storage_backing, num_sql_queries):

lms/djangoapps/courseware/tests/test_module_render.py

@@ -1016,7 +1016,7 @@ def setup_request_and_course(self, num_finds, num_sends):
     #     - 1 for 5 definitions
     # Split makes 1 MySQL query to render the toc:
     #     - 1 MySQL for the active version at the start of the bulk operation (no mongo calls)
-    @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0, 0), (ModuleStoreEnum.Type.split, 2, 0, 1))
+    @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0, 0), (ModuleStoreEnum.Type.split, 2, 0, 0))
     @ddt.unpack
     def test_toc_toy_from_chapter(self, default_ms, setup_finds, setup_sends, toc_finds):
         with self.store.default_store(default_ms):
@@ -1056,7 +1056,7 @@ def test_toc_toy_from_chapter(self, default_ms, setup_finds, setup_sends, toc_fi
     #     - 1 for 5 definitions
     # Split makes 1 MySQL query to render the toc:
     #     - 1 MySQL for the active version at the start of the bulk operation (no mongo calls)
-    @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0, 0), (ModuleStoreEnum.Type.split, 2, 0, 1))
+    @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0, 0), (ModuleStoreEnum.Type.split, 2, 0, 0))
     @ddt.unpack
     def test_toc_toy_from_section(self, default_ms, setup_finds, setup_sends, toc_finds):
         with self.store.default_store(default_ms):

lms/djangoapps/courseware/testutils.py

@@ -162,9 +162,9 @@ def verify_response(self, expected_response_code=200, url_params=None):
 
     @ddt.data(
         ('vertical_block', ModuleStoreEnum.Type.mongo, 13),
-        ('vertical_block', ModuleStoreEnum.Type.split, 6),
+        ('vertical_block', ModuleStoreEnum.Type.split, 5),
         ('html_block', ModuleStoreEnum.Type.mongo, 14),
-        ('html_block', ModuleStoreEnum.Type.split, 6),
+        ('html_block', ModuleStoreEnum.Type.split, 5),
     )
     @ddt.unpack
     def test_courseware_html(self, block_name, default_store, mongo_calls):

lms/djangoapps/discussion/django_comment_client/base/tests.py

@@ -402,7 +402,7 @@ def inner(self, default_store, module_count, mongo_calls, sql_queries, *args, **
 
     @ddt.data(
         (ModuleStoreEnum.Type.mongo, 3, 4, 39),
-        (ModuleStoreEnum.Type.split, 3, 13, 39),
+        (ModuleStoreEnum.Type.split, 3, 11, 39),
     )
     @ddt.unpack
     @count_queries
@@ -411,7 +411,7 @@ def test_create_thread(self, mock_request):
 
     @ddt.data(
         (ModuleStoreEnum.Type.mongo, 3, 3, 35),
-        (ModuleStoreEnum.Type.split, 3, 10, 35),
+        (ModuleStoreEnum.Type.split, 3, 9, 35),
     )
     @ddt.unpack
     @count_queries

lms/djangoapps/discussion/tests/test_views.py

@@ -483,15 +483,15 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase):
         (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 21, 7),
         (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 21, 7),
         # split mongo: 3 queries, regardless of thread response size.
-        (ModuleStoreEnum.Type.split, False, 1, 3, 3, 21, 8),
-        (ModuleStoreEnum.Type.split, False, 50, 3, 3, 21, 8),
+        (ModuleStoreEnum.Type.split, False, 1, 2, 2, 21, 8),
+        (ModuleStoreEnum.Type.split, False, 50, 2, 2, 21, 8),
 
         # Enabling Enterprise integration should have no effect on the number of mongo queries made.
         (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 21, 7),
         (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 21, 7),
         # split mongo: 3 queries, regardless of thread response size.
-        (ModuleStoreEnum.Type.split, True, 1, 3, 3, 21, 8),
-        (ModuleStoreEnum.Type.split, True, 50, 3, 3, 21, 8),
+        (ModuleStoreEnum.Type.split, True, 1, 2, 2, 21, 8),
+        (ModuleStoreEnum.Type.split, True, 50, 2, 2, 21, 8),
     )
     @ddt.unpack
     def test_number_of_mongo_queries(

lms/djangoapps/grades/tests/test_tasks.py

@@ -164,8 +164,9 @@ def test_block_structure_created_only_once(self):
     @ddt.data(
         (ModuleStoreEnum.Type.mongo, 1, 40, True),
         (ModuleStoreEnum.Type.mongo, 1, 40, False),
-        (ModuleStoreEnum.Type.split, 3, 40, True),
-        (ModuleStoreEnum.Type.split, 3, 40, False),
+        (ModuleStoreEnum.Type.split, 2, 40, True),
+        (ModuleStoreEnum.Type.split, 2, 40, False),
+
     )
     @ddt.unpack
     def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections):
@@ -178,7 +179,7 @@ def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, creat
 
     @ddt.data(
         (ModuleStoreEnum.Type.mongo, 1, 40),
-        (ModuleStoreEnum.Type.split, 3, 40),
+        (ModuleStoreEnum.Type.split, 2, 40),
     )
     @ddt.unpack
     def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
@@ -224,7 +225,7 @@ def test_other_inaccessible_subsection(self, mock_subsection_signal):
 
     @ddt.data(
         (ModuleStoreEnum.Type.mongo, 1, 23),
-        (ModuleStoreEnum.Type.split, 3, 23),
+        (ModuleStoreEnum.Type.split, 2, 23),
     )
     @ddt.unpack
     def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):
@@ -239,7 +240,7 @@ def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_
 
     @ddt.data(
         (ModuleStoreEnum.Type.mongo, 1, 41),
-        (ModuleStoreEnum.Type.split, 3, 41),
+        (ModuleStoreEnum.Type.split, 2, 41),
     )
     @ddt.unpack
     def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):

lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py

@@ -673,6 +673,6 @@ def test_spoc_gradebook_mongo_calls(self):
 
         # check MongoDB calls count
         url = reverse('spoc_gradebook', kwargs={'course_id': self.course.id})
-        with check_mongo_calls(9):
+        with check_mongo_calls(7):
             response = self.client.get(url)
             assert response.status_code == 200

openedx/core/djangoapps/bookmarks/tests/test_models.py

@@ -376,11 +376,12 @@ def test_path(self, seconds_delta, paths, get_path_call_count, mock_get_path):
         # (ModuleStoreEnum.Type.mongo, 6, 3, 3), Too slow.
         (ModuleStoreEnum.Type.mongo, 2, 4, 4),
         # (ModuleStoreEnum.Type.mongo, 4, 4, 4),
-        (ModuleStoreEnum.Type.split, 2, 2, 2),
-        (ModuleStoreEnum.Type.split, 4, 2, 2),
-        (ModuleStoreEnum.Type.split, 2, 3, 2),
+        (ModuleStoreEnum.Type.split, 2, 2, 1),
+        (ModuleStoreEnum.Type.split, 4, 2, 1),
+        (ModuleStoreEnum.Type.split, 2, 3, 1),
         # (ModuleStoreEnum.Type.split, 4, 3, 1),
-        (ModuleStoreEnum.Type.split, 2, 4, 2),
+        (ModuleStoreEnum.Type.split, 2, 4, 1),
+
     )
     @ddt.unpack
     def test_get_path_queries(self, store_type, children_per_block, depth, expected_mongo_calls):

openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py

@@ -378,7 +378,7 @@ def test_malformed_grading_policy(self):
         course_overview = CourseOverview._create_or_update(course)  # pylint: disable=protected-access
         assert course_overview.lowest_passing_grade is None
 
-    @ddt.data((ModuleStoreEnum.Type.mongo, 4, 4), (ModuleStoreEnum.Type.split, 3, 3))
+    @ddt.data((ModuleStoreEnum.Type.mongo, 4, 4), (ModuleStoreEnum.Type.split, 2, 2))
     @ddt.unpack
     def test_versioning(self, modulestore_type, min_mongo_calls, max_mongo_calls):
         """