Unit test for import performance STUD-1994

cms/djangoapps/contentstore/tests/test_course_listing.py

@@ -200,10 +200,10 @@ class TestCourseListing(ModuleStoreTestCase):
 
         # Now count the db queries
         store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo)
-        with check_mongo_calls(store.collection, USER_COURSES_COUNT):
+        with check_mongo_calls(store, USER_COURSES_COUNT):
             courses_list = _accessible_courses_list_from_groups(self.request)
 
-        with check_mongo_calls(store.collection, 1):
+        with check_mongo_calls(store, 1):
             courses_list = _accessible_courses_list(self.request)
 
     def test_get_course_list_with_same_course_id(self):

cms/djangoapps/contentstore/tests/test_import.py

 # -*- coding: utf-8 -*-
 # pylint: disable=E1101
+# pylint: disable=protected-access
 """
 Tests for import_from_xml using the mongo modulestore.
 """
@@ -10,8 +11,10 @@ from django.conf import settings
 import copy
 
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
+from xmodule.modulestore.tests.factories import check_exact_number_of_calls, check_number_of_calls
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
 from xmodule.modulestore.xml_importer import import_from_xml
 from xmodule.exceptions import NotFoundError
@@ -149,6 +152,21 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         print "course tabs = {0}".format(course.tabs)
         self.assertEqual(course.tabs[2]['name'], 'Syllabus')
 
+    def test_import_performance_mongo(self):
+        store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo)
+
+        # we try to refresh the inheritance tree for each update_item in the import
+        with check_exact_number_of_calls(store, store.refresh_cached_metadata_inheritance_tree, 46):
+
+            # the post-publish step loads each item in the subtree, which calls _get_cached_metadata_inheritance_tree
+            with check_exact_number_of_calls(store, store._get_cached_metadata_inheritance_tree, 22):
+
+                # with bulk-edit in progress, the inheritance tree should be recomputed only at the end of the import
+                # NOTE: On Jenkins, with memcache enabled, the number of calls here is only 1.
+                #       Locally, without memcache, the number of calls is actually 2 (once more during the publish step)
+                with check_number_of_calls(store, store._compute_metadata_inheritance_tree, 2):
+                    self.load_test_import_course()
+
     def test_rewrite_reference_list(self):
         module_store = modulestore()
         target_course_id = SlashSeparatedCourseKey('testX', 'conditional_copy', 'copy_run')

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

@@ -9,7 +9,7 @@ from xblock.core import XBlock
 from xmodule.tabs import StaticTab
 from decorator import contextmanager
 from mock import Mock, patch
-from nose.tools import assert_less_equal
+from nose.tools import assert_less_equal, assert_greater_equal
 
 
 class Dummy(object):
@@ -220,30 +220,57 @@ class ItemFactory(XModuleFactory):
 
 
 @contextmanager
-def check_mongo_calls(mongo_store, max_finds=0, max_sends=None):
+def check_exact_number_of_calls(object_with_method, method, num_calls, method_name=None):
     """
-    Instruments the given store to count the number of calls to find (incl find_one) and the number
-    of calls to send_message which is for insert, update, and remove (if you provide max_sends). At the
-    end of the with statement, it compares the counts to the max_finds and max_sends using a simple
-    assertLessEqual.
+    Instruments the given method on the given object to verify the number of calls to the
+    method is exactly equal to 'num_calls'.
+    """
+    with check_number_of_calls(object_with_method, method, num_calls, num_calls, method_name):
+        yield
 
-    :param mongo_store: the MongoModulestore or subclass to watch
-    :param max_finds: the maximum number of find calls to allow
-    :param max_sends: If none, don't instrument the send calls. If non-none, count and compare to
-        the given int value.
+
+@contextmanager
+def check_number_of_calls(object_with_method, method, maximum_calls, minimum_calls=1, method_name=None):
+    """
+    Instruments the given method on the given object to verify the number of calls to the method is
+    less than or equal to the expected maximum_calls and greater than or equal to the expected minimum_calls.
     """
+    method_wrap = Mock(wraps=method)
+    wrap_patch = patch.object(object_with_method, method_name or method.__name__, method_wrap)
+
     try:
-        find_wrap = Mock(wraps=mongo_store.collection.find)
-        wrap_patch = patch.object(mongo_store.collection, 'find', find_wrap)
         wrap_patch.start()
-        if max_sends:
-            sends_wrap = Mock(wraps=mongo_store.database.connection._send_message)
-            sends_patch = patch.object(mongo_store.database.connection, '_send_message', sends_wrap)
-            sends_patch.start()
         yield
+
     finally:
         wrap_patch.stop()
-        if max_sends:
-            sends_patch.stop()
-            assert_less_equal(sends_wrap.call_count, max_sends)
-        assert_less_equal(find_wrap.call_count, max_finds)
+
+        # verify the counter actually worked by ensuring we have counted greater than (or equal to) the minimum calls
+        assert_greater_equal(method_wrap.call_count, minimum_calls)
+
+        # now verify the number of actual calls is less than (or equal to) the expected maximum
+        assert_less_equal(method_wrap.call_count, maximum_calls)
+
+
+@contextmanager
+def check_mongo_calls(mongo_store, num_finds=0, num_sends=None):
+    """
+    Instruments the given store to count the number of calls to find (incl find_one) and the number
+    of calls to send_message which is for insert, update, and remove (if you provide num_sends). At the
+    end of the with statement, it compares the counts to the num_finds and num_sends.
+
+    :param mongo_store: the MongoModulestore or subclass to watch
+    :param num_finds: the exact number of find calls expected
+    :param num_sends: If none, don't instrument the send calls. If non-none, count and compare to
+        the given int value.
+    """
+    with check_exact_number_of_calls(mongo_store.collection, mongo_store.collection.find, num_finds):
+        if num_sends:
+            with check_exact_number_of_calls(
+                mongo_store.database.connection,
+                mongo_store.database.connection._send_message,  # pylint: disable=protected-access
+                num_sends,
+            ):
+                yield
+        else:
+            yield

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

@@ -19,23 +19,23 @@ class TestPublish(SplitWMongoCourseBoostrapper):
         # There are 12 created items and 7 parent updates
         # create course: finds: 1 to verify uniqueness, 1 to find parents
         # sends: 1 to create course, 1 to create overview
-        with check_mongo_calls(self.draft_mongo, 6, 2):
+        with check_mongo_calls(self.draft_mongo, 5, 2):
             super(TestPublish, self)._create_course(split=False)  # 2 inserts (course and overview)
 
         # with bulk will delay all inheritance computations which won't be added into the mongo_calls
         with self.draft_mongo.bulk_write_operations(self.old_course_key):
-            # finds: 1 for parent to add child, 1 for parent to update edit info
-            # sends: 1 for insert, 2 for parent (add child, change edit info)
-            with check_mongo_calls(self.draft_mongo, 5, 3):
+            # finds: 1 for parent to add child
+            # sends: 1 for insert, 1 for parent (add child)
+            with check_mongo_calls(self.draft_mongo, 1, 2):
                 self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid', split=False)
 
-            with check_mongo_calls(self.draft_mongo, 5, 3):
+            with check_mongo_calls(self.draft_mongo, 2, 2):
                 self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid', split=False)
             # update info propagation is 2 levels. create looks for draft and then published and then creates
-            with check_mongo_calls(self.draft_mongo, 16, 8):
+            with check_mongo_calls(self.draft_mongo, 8, 6):
                 self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', split=False)
                 self._create_item('vertical', 'Vert2', {}, {'display_name': 'Vertical 2'}, 'chapter', 'Chapter1', split=False)
-            with check_mongo_calls(self.draft_mongo, 36, 36):
+            with check_mongo_calls(self.draft_mongo, 20, 12):
                 self._create_item('html', 'Html1', "<p>Goodbye</p>", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', split=False)
                 self._create_item(
                     'discussion', 'Discussion1',
@@ -62,7 +62,8 @@ class TestPublish(SplitWMongoCourseBoostrapper):
                     'vertical', 'Vert2',
                     split=False
                 )
-            with check_mongo_calls(self.draft_mongo, 2, 2):
+
+            with check_mongo_calls(self.draft_mongo, 0, 2):
                 # 2 finds b/c looking for non-existent parents
                 self._create_item('static_tab', 'staticuno', "<p>tab</p>", {'display_name': 'Tab uno'}, None, None, split=False)
                 self._create_item('course_info', 'updates', "<ol><li><h2>Sep 22</h2><p>test</p></li></ol>", {}, None, None, split=False)