refactor: convert global modulestore to request-cached

Having a global modulestore reference is a holdover from the early days,
when the XML modulestore would be extremely expensive to initialize.
That is no longer the case, and we should eliminate the global here to
reduce the chances of multi-threading bugs that might mutate the global.

Unfortunately, a lot of code currently assumes that calls to the
modulestore() function are essentially free, instead of the 1-2 ms it
really is. There are cases where this may be called thousands of times
deep in loops somewhere while doing course content traversal, so it's
risky to eliminate the caching behavior altogether. The thought here is
that we'll tie to the RequestCache, which should at least not be shared
across users/threads.

Author: ormsbee

xmodule/modulestore/django.py

@@ -26,6 +26,7 @@
 from django.utils.translation import get_language, to_locale  # lint-amnesty, pylint: disable=wrong-import-position
 from edx_django_utils.cache import DEFAULT_REQUEST_CACHE  # lint-amnesty, pylint: disable=wrong-import-position
 
+from openedx.core.lib.cache_utils import request_cached
 from xmodule.contentstore.django import contentstore  # lint-amnesty, pylint: disable=wrong-import-position
 from xmodule.modulestore.draft_and_published import BranchSettingMixin  # lint-amnesty, pylint: disable=wrong-import-position
 from xmodule.modulestore.mixed import MixedModuleStore  # lint-amnesty, pylint: disable=wrong-import-position
@@ -323,46 +324,27 @@ def fetch_disabled_xblock_types():
         **_options
     )
 
-
-# A singleton instance of the Mixed Modulestore
-_MIXED_MODULESTORE = None
-
-
+@request_cached("modulestore")
 def modulestore():
     """
     Returns the Mixed modulestore
     """
-    global _MIXED_MODULESTORE  # pylint: disable=global-statement
-    if _MIXED_MODULESTORE is None:
-        _MIXED_MODULESTORE = create_modulestore_instance(
-            settings.MODULESTORE['default']['ENGINE'],
-            contentstore(),
-            settings.MODULESTORE['default'].get('DOC_STORE_CONFIG', {}),
-            settings.MODULESTORE['default'].get('OPTIONS', {})
-        )
-
-        if settings.FEATURES.get('CUSTOM_COURSES_EDX'):
-            # TODO: This import prevents a circular import issue, but is
-            # symptomatic of a lib having a dependency on code in lms.  This
-            # should be updated to have a setting that enumerates modulestore
-            # wrappers and then uses that setting to wrap the modulestore in
-            # appropriate wrappers depending on enabled features.
-            from lms.djangoapps.ccx.modulestore import CCXModulestoreWrapper
-            _MIXED_MODULESTORE = CCXModulestoreWrapper(_MIXED_MODULESTORE)
-
-    return _MIXED_MODULESTORE
-
-
-def clear_existing_modulestores():
-    """
-    Clear the existing modulestore instances, causing
-    them to be re-created when accessed again.
-
-    This is useful for flushing state between unit tests.
-    """
-    global _MIXED_MODULESTORE  # pylint: disable=global-statement
-    _MIXED_MODULESTORE = None
-
+    mixed_modulestore = create_modulestore_instance(
+        settings.MODULESTORE['default']['ENGINE'],
+        contentstore(),
+        settings.MODULESTORE['default'].get('DOC_STORE_CONFIG', {}),
+        settings.MODULESTORE['default'].get('OPTIONS', {})
+    )
+    if settings.FEATURES.get('CUSTOM_COURSES_EDX'):
+        # TODO: This import prevents a circular import issue, but is
+        # symptomatic of a lib having a dependency on code in lms.  This
+        # should be updated to have a setting that enumerates modulestore
+        # wrappers and then uses that setting to wrap the modulestore in
+        # appropriate wrappers depending on enabled features.
+        from lms.djangoapps.ccx.modulestore import CCXModulestoreWrapper
+        mixed_modulestore = CCXModulestoreWrapper(mixed_modulestore)
+
+    return mixed_modulestore
 
 class XBlockI18nService:
     """

xmodule/modulestore/tests/django_utils.py

@@ -19,7 +19,7 @@
 from xmodule.contentstore.content import StaticContent
 from xmodule.contentstore.django import _CONTENTSTORE
 from xmodule.modulestore import ModuleStoreEnum
-from xmodule.modulestore.django import SignalHandler, clear_existing_modulestores, modulestore
+from xmodule.modulestore.django import SignalHandler, modulestore
 from xmodule.modulestore.tests.factories import XMODULE_FACTORY_LOCK
 from xmodule.modulestore.tests.mongo_connection import MONGO_HOST, MONGO_PORT_NUM
 
@@ -321,7 +321,6 @@ def start_modulestore_isolation(cls):
         override.__enter__()  # pylint: disable=unnecessary-dunder-call
         cls.__settings_overrides.append(override)
         XMODULE_FACTORY_LOCK.enable()
-        clear_existing_modulestores()
         cls.store = modulestore()
 
     @classmethod