Merge pull request #5288 from edx/split/info_perf

Upgrade pymongo

cms/djangoapps/contentstore/tests/utils.py

@@ -72,10 +72,10 @@ class CourseTestCase(ModuleStoreTestCase):
         will be cleared out before each test case execution and deleted
         afterwards.
         """
-        user_password = super(CourseTestCase, self).setUp()
+        self.user_password = super(CourseTestCase, self).setUp()
 
         self.client = AjaxEnabledTestClient()
-        self.client.login(username=self.user.username, password=user_password)
+        self.client.login(username=self.user.username, password=self.user_password)
 
         self.course = CourseFactory.create()
 

cms/djangoapps/contentstore/views/tests/test_course_index.py

@@ -8,17 +8,21 @@ import datetime
 from contentstore.tests.utils import CourseTestCase
 from contentstore.utils import reverse_course_url, add_instructor
 from contentstore.views.access import has_course_access
-from contentstore.views.course import course_outline_initial_state
+from contentstore.views.course import course_outline_initial_state, _course_outline_json
 from contentstore.views.item import create_xblock_info, VisibilityState
 from course_action_state.models import CourseRerunState
 from util.date_utils import get_default_time_display
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls, \
+    mongo_uses_error_check
 from opaque_keys.edx.locator import CourseLocator
 from student.tests.factories import UserFactory
 from course_action_state.managers import CourseRerunUIStateManager
 from django.conf import settings
+import ddt
+import threading
+import pytz
 
 
 class TestCourseIndex(CourseTestCase):
@@ -302,9 +306,37 @@ class TestCourseOutline(CourseTestCase):
         self.assertEqual(_get_release_date(response), 'Unscheduled')
         _assert_settings_link_present(response)
 
-        self.course.start = datetime.datetime(2014, 1, 1)
+        self.course.start = datetime.datetime(2014, 1, 1, tzinfo=pytz.utc)
         modulestore().update_item(self.course, ModuleStoreEnum.UserID.test)
         response = self.client.get(outline_url, {}, HTTP_ACCEPT='text/html')
 
         self.assertEqual(_get_release_date(response), get_default_time_display(self.course.start))
         _assert_settings_link_present(response)
+
+
+@ddt.ddt
+class OutlinePerfTest(TestCourseOutline):
+    def setUp(self):
+        with modulestore().default_store(ModuleStoreEnum.Type.split):
+            super(OutlinePerfTest, self).setUp()
+
+    @ddt.data(1, 2, 4, 8)
+    def test_query_counts(self, num_threads):
+        """
+        Test that increasing threads does not increase query counts
+        """
+        def test_client():
+            with modulestore().default_store(ModuleStoreEnum.Type.split):
+                with modulestore().bulk_operations(self.course.id):
+                    course = modulestore().get_course(self.course.id, depth=0)
+                    return _course_outline_json(None, course)
+
+        if mongo_uses_error_check(modulestore()):
+            per_thread = 5
+        else:
+            per_thread = 4
+        with check_mongo_calls(per_thread * num_threads, 0):
+            outline_threads = [threading.Thread(target=test_client) for __ in range(num_threads)]
+            [thread.start() for thread in outline_threads]
+            # now wait until they all finish
+            [thread.join() for thread in outline_threads]

cms/envs/bok_choy.auth.json

@@ -11,9 +11,7 @@
             "host": [
                 "localhost"
             ],
-            "password": "password",
-            "port": 27017,
-            "user": "edxapp"
+            "port": 27017
         },
         "ENGINE": "xmodule.contentstore.mongo.MongoContentStore",
         "OPTIONS": {
@@ -21,9 +19,7 @@
             "host": [
                 "localhost"
             ],
-            "password": "password",
-            "port": 27017,
-            "user": "edxapp"
+            "port": 27017
         }
     },
     "DATABASES": {
@@ -42,9 +38,7 @@
         "host": [
             "localhost"
         ],
-        "password": "password",
-        "port": 27017,
-        "user": "edxapp"
+        "port": 27017
     },
     "MODULESTORE": {
         "default": {
@@ -60,9 +54,7 @@
                             "host": [
                                 "localhost"
                             ],
-                            "password": "password",
-                            "port": 27017,
-                            "user": "edxapp"
+                            "port": 27017
                         },
                         "ENGINE": "xmodule.modulestore.mongo.DraftMongoModuleStore",
                         "OPTIONS": {
@@ -73,10 +65,8 @@
                             "host": [
                                 "localhost"
                             ],
-                            "password": "password",
                             "port": 27017,
-                            "render_template": "edxmako.shortcuts.render_to_string",
-                            "user": "edxapp"
+                            "render_template": "edxmako.shortcuts.render_to_string"
                         }
                     },
                     {

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -488,6 +488,13 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
         """
         self.collection.database.connection.close()
 
+    def mongo_wire_version(self):
+        """
+        Returns the wire version for mongo. Only used to unit tests which instrument the connection.
+        """
+        self.database.connection._ensure_connected()
+        return self.database.connection.max_wire_version
+
     def _drop_database(self):
         """
         A destructive operation to drop the underlying database and close all connections.
@@ -1137,7 +1144,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
         '''
         return self.get_course(location.course_key, depth)
 
-    def _update_single_item(self, location, update):
+    def _update_single_item(self, location, update, allow_not_found=False):
         """
         Set update on the specified item, and raises ItemNotFoundError
         if the location doesn't exist
@@ -1150,10 +1157,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
             {'_id': location.to_deprecated_son()},
             {'$set': update},
             multi=False,
-            upsert=True,
-            # Must include this to avoid the django debug toolbar (which defines the deprecated "safe=False")
-            # from overriding our default value set in the init method.
-            safe=self.collection.safe
+            upsert=allow_not_found,
+            w=1,  # wait until primary commits
         )
         if result['n'] == 0:
             raise ItemNotFoundError(location)
@@ -1205,7 +1210,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
             if xblock.has_children:
                 children = self._serialize_scope(xblock, Scope.children)
                 payload.update({'definition.children': children['children']})
-            self._update_single_item(xblock.scope_ids.usage_id, payload)
+            self._update_single_item(xblock.scope_ids.usage_id, payload, allow_not_found=allow_not_found)
 
             # update subtree edited info for ancestors
             # don't update the subtree info for descendants of the publish root for efficiency
@@ -1282,7 +1287,6 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
                         {'$set': {'definition.children': []}},
                         multi=False,
                         upsert=True,
-                        safe=self.collection.safe
                     )
                 elif ancestor_loc.category == 'course':
                     # once we reach the top location of the tree and if the location is not an orphan then the

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -677,7 +677,11 @@ class DraftModuleStore(MongoModuleStore):
                                 # So, do not delete the child.  It will be published when the new parent is published.
                                 pass
 
-            super(DraftModuleStore, self).update_item(item, user_id, isPublish=True, is_publish_root=is_root)
+            # update the published (not draft) item (ignoring that item is "draft"). The published
+            # may not exist; (if original_published is None); so, allow_not_found
+            super(DraftModuleStore, self).update_item(
+                item, user_id, isPublish=True, is_publish_root=is_root, allow_not_found=True
+            )
             to_be_deleted.append(as_draft(item_location).to_deprecated_son())
 
         # verify input conditions

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

@@ -303,4 +303,3 @@ class MongoConnection(object):
             ],
             unique=True
         )
-

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

@@ -196,6 +196,7 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
                 course_key.replace(org=None, course=None, run=None, branch=None)
             ]
 
+        # handle ignore case and general use
         return super(SplitBulkWriteMixin, self)._get_bulk_ops_record(
             course_key.replace(branch=None, version_guid=None), ignore_case
         )
@@ -608,6 +609,12 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         """
         self.db.connection.close()
 
+    def mongo_wire_version(self):
+        """
+        Returns the wire version for mongo. Only used to unit tests which instrument the connection.
+        """
+        return self.db.connection.max_wire_version
+
     def _drop_database(self):
         """
         A destructive operation to drop the underlying database and close all connections.

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

@@ -279,6 +279,17 @@ def check_sum_of_calls(object_, methods, maximum_calls, minimum_calls=1):
     assert_less_equal(call_count, maximum_calls)
 
 
+def mongo_uses_error_check(store):
+    """
+    Does mongo use the error check as a separate message?
+    """
+    if hasattr(store, 'mongo_wire_version'):
+        return store.mongo_wire_version() <= 1
+    if hasattr(store, 'modulestores'):
+        return any([mongo_uses_error_check(substore) for substore in store.modulestores])
+    return False
+
+
 @contextmanager
 def check_mongo_calls(num_finds=0, num_sends=None):
     """
@@ -299,7 +310,8 @@ def check_mongo_calls(num_finds=0, num_sends=None):
         if num_sends is not None:
             with check_sum_of_calls(
                 pymongo.message,
-                ['insert', 'update', 'delete'],
+                # mongo < 2.6 uses insert, update, delete and _do_batched_insert. >= 2.6 _do_batched_write
+                ['insert', 'update', 'delete', '_do_batched_write_command', '_do_batched_insert', ],
                 num_sends,
                 num_sends
             ):

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

@@ -31,7 +31,8 @@ from xmodule.modulestore.draft_and_published import UnsupportedRevisionError, Mo
 from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError, NoPathToItem
 from xmodule.modulestore.mixed import MixedModuleStore
 from xmodule.modulestore.search import path_to_location
-from xmodule.modulestore.tests.factories import check_mongo_calls, check_exact_number_of_calls
+from xmodule.modulestore.tests.factories import check_mongo_calls, check_exact_number_of_calls, \
+    mongo_uses_error_check
 from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST
 from xmodule.tests import DATA_DIR, CourseComparisonTest
 
@@ -660,18 +661,20 @@ class TestMixedModuleStore(CourseComparisonTest):
     # Draft
     #   Find: find parents (definition.children query), get parent, get course (fill in run?),
     #         find parents of the parent (course), get inheritance items,
-    #         get errors, get item (to delete subtree), get inheritance again.
+    #         get item (to delete subtree), get inheritance again.
     #   Sends: delete item, update parent
     # Split
     #   Find: active_versions, 2 structures (published & draft), definition (unnecessary)
     #   Sends: updated draft and published structures and active_versions
-    @ddt.data(('draft', 8, 2), ('split', 4, 3))
+    @ddt.data(('draft', 7, 2), ('split', 4, 3))
     @ddt.unpack
     def test_delete_item(self, default_ms, max_find, max_send):
         """
         Delete should reject on r/o db and work on r/w one
         """
         self.initdb(default_ms)
+        if default_ms == 'draft' and mongo_uses_error_check(self.store):
+            max_find += 1
 
         # r/o try deleting the chapter (is here to ensure it can't be deleted)
         with self.assertRaises(NotImplementedError):
@@ -690,12 +693,12 @@ class TestMixedModuleStore(CourseComparisonTest):
 
     # Draft:
     #    queries: find parent (definition.children), count versions of item, get parent, count grandparents,
-    #             inheritance items, draft item, draft child, get errors, inheritance
+    #             inheritance items, draft item, draft child, inheritance
     #    sends: delete draft vertical and update parent
     # Split:
     #    queries: active_versions, draft and published structures, definition (unnecessary)
     #    sends: update published (why?), draft, and active_versions
-    @ddt.data(('draft', 9, 2), ('split', 4, 3))
+    @ddt.data(('draft', 8, 2), ('split', 4, 3))
     @ddt.unpack
     def test_delete_private_vertical(self, default_ms, max_find, max_send):
         """
@@ -703,6 +706,8 @@ class TestMixedModuleStore(CourseComparisonTest):
         behavioral properties which this deletion test gets at.
         """
         self.initdb(default_ms)
+        if default_ms == 'draft' and mongo_uses_error_check(self.store):
+            max_find += 1
         # create and delete a private vertical with private children
         private_vert = self.store.create_child(
             # don't use course_location as it may not be the repr
@@ -741,12 +746,12 @@ class TestMixedModuleStore(CourseComparisonTest):
         self.assertNotIn(vert_loc, course.children)
 
     # Draft:
-    #   find: find parent (definition.children) 2x, find draft item, check error state, get inheritance items
+    #   find: find parent (definition.children) 2x, find draft item, get inheritance items
     #   send: one delete query for specific item
     # Split:
     #   find: active_version & structure
     #   send: update structure and active_versions
-    @ddt.data(('draft', 5, 1), ('split', 2, 2))
+    @ddt.data(('draft', 4, 1), ('split', 2, 2))
     @ddt.unpack
     def test_delete_draft_vertical(self, default_ms, max_find, max_send):
         """
@@ -776,6 +781,8 @@ class TestMixedModuleStore(CourseComparisonTest):
         private_leaf.display_name = 'change me'
         private_leaf = self.store.update_item(private_leaf, self.user_id)
         # test succeeds if delete succeeds w/o error
+        if default_ms == 'draft' and mongo_uses_error_check(self.store):
+            max_find += 1
         with check_mongo_calls(max_find, max_send):
             self.store.delete_item(private_leaf.location, self.user_id)
 
@@ -1294,7 +1301,7 @@ class TestMixedModuleStore(CourseComparisonTest):
         self.assertEqual(len(self.store.get_courses_for_wiki('no_such_wiki')), 0)
 
     # Draft:
-    #    Find: find vertical, find children, get last error
+    #    Find: find vertical, find children
     #    Sends:
     #      1. delete all of the published nodes in subtree
     #      2. insert vertical as published (deleted in step 1) w/ the deleted problems as children
@@ -1303,13 +1310,15 @@ class TestMixedModuleStore(CourseComparisonTest):
     # Sends:
     #    - insert structure
     #    - write index entry
-    @ddt.data(('draft', 3, 6), ('split', 3, 2))
+    @ddt.data(('draft', 2, 6), ('split', 3, 2))
     @ddt.unpack
     def test_unpublish(self, default_ms, max_find, max_send):
         """
         Test calling unpublish
         """
         self.initdb(default_ms)
+        if default_ms == 'draft' and mongo_uses_error_check(self.store):
+            max_find += 1
         self._create_block_hierarchy()
 
         # publish

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

@@ -573,6 +573,7 @@ class TestMongoModuleStore(unittest.TestCase):
                     'published_by': published_by,
                 },
             },
+            allow_not_found=True,
         )
 
         # Retrieve the block and verify its fields

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

@@ -3,7 +3,7 @@ Test the publish code (mostly testing that publishing doesn't result in orphans)
 """
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore.tests.test_split_w_old_mongo import SplitWMongoCourseBoostrapper
-from xmodule.modulestore.tests.factories import check_mongo_calls
+from xmodule.modulestore.tests.factories import check_mongo_calls, mongo_uses_error_check
 from xmodule.modulestore import ModuleStoreEnum
 
 
@@ -98,13 +98,16 @@ class TestPublish(SplitWMongoCourseBoostrapper):
         #   12-15 get each ancestor (count then get): (2 x 2),
         #   16 then fail count of course parent (1)
         #   17 compute inheritance
-        #   18 get last error
-        #   19-20 get draft and published vert
+        #   18-19 get draft and published vert
         # Sends:
         #   delete the subtree of drafts (1 call),
         #   update the published version of each node in subtree (4 calls),
         #   update the ancestors up to course (2 calls)
-        with check_mongo_calls(20, 7):
+        if mongo_uses_error_check(self.draft_mongo):
+            max_find = 20
+        else:
+            max_find = 19
+        with check_mongo_calls(max_find, 7):
             self.draft_mongo.publish(item.location, self.user_id)
 
         # verify status

lms/envs/bok_choy.auth.json

@@ -11,9 +11,7 @@
             "host": [
                 "localhost"
             ],
-            "password": "password",
-            "port": 27017,
-            "user": "edxapp"
+            "port": 27017
         },
         "ENGINE": "xmodule.contentstore.mongo.MongoContentStore",
         "OPTIONS": {
@@ -21,9 +19,7 @@
             "host": [
                 "localhost"
             ],
-            "password": "password",
-            "port": 27017,
-            "user": "edxapp"
+            "port": 27017
         }
     },
     "DATABASES": {
@@ -42,9 +38,7 @@
         "host": [
             "localhost"
         ],
-        "password": "password",
-        "port": 27017,
-        "user": "edxapp"
+        "port": 27017
     },
     "EVENT_TRACKING_BACKENDS": {
         "mongo": {
@@ -69,9 +63,7 @@
                             "host": [
                                 "localhost"
                             ],
-                            "password": "password",
-                            "port": 27017,
-                            "user": "edxapp"
+                            "port": 27017
                         },
                         "ENGINE": "xmodule.modulestore.mongo.DraftMongoModuleStore",
                         "OPTIONS": {
@@ -82,10 +74,8 @@
                             "host": [
                                 "localhost"
                             ],
-                            "password": "password",
                             "port": 27017,
-                            "render_template": "edxmako.shortcuts.render_to_string",
-                            "user": "edxapp"
+                            "render_template": "edxmako.shortcuts.render_to_string"
                         }
                     },
                     {

requirements/edx/base.txt

@@ -61,7 +61,7 @@ polib==1.0.3
 pycrypto>=2.6
 pygments==1.6
 pygraphviz==1.1
-pymongo==2.4.1
+pymongo==2.7.2
 pyparsing==2.0.1
 python-memcached==1.48
 python-openid==2.2.5