Skip to content

E
edx-platform
Commit b137f8d7 by Don Mitchell
Options

Explain query counts 

Fix trivial excesses (2 splits configured & don't fetch courses if there are none)
LMS-11220, LMS-11391, LMS-11390
parent e18e18a8
Showing with 108 additions and 70 deletions
cms/djangoapps/contentstore/tests/test_course_listing.py
......@@ -200,12 +200,14 @@ class TestCourseListing(ModuleStoreTestCase):
self.assertGreaterEqual(iteration_over_courses_time_2.elapsed, iteration_over_groups_time_2.elapsed)
# Now count the db queries
store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo)
with check_mongo_calls(USER_COURSES_COUNT):
_accessible_courses_list_from_groups(self.request)
# TODO: LMS-11220: Document why this takes 6 calls
with check_mongo_calls(6):
# Calls:
# 1) query old mongo
# 2) get_more on old mongo
# 3) query split (but no courses so no fetching of data)
with check_mongo_calls(3):
_accessible_courses_list(self.request)
def test_get_course_list_with_same_course_id(self):
......@@ -276,7 +278,7 @@ class TestCourseListing(ModuleStoreTestCase):
course_location = SlashSeparatedCourseKey('testOrg', 'erroredCourse', 'RunBabyRun')
course = self._create_course_with_access_groups(course_location, self.user)
course_db_record = store._find_one(course.location)
course_db_record.setdefault('metadata', {}).get('tabs', []).append({"type": "wiko", "name": "Wiki" })
course_db_record.setdefault('metadata', {}).get('tabs', []).append({"type": "wiko", "name": "Wiki"})
store.collection.update(
{'_id': course.location.to_deprecated_son()},
{'$set': {
......
cms/envs/test.py
......@@ -73,17 +73,6 @@ STATICFILES_STORAGE='pipeline.storage.NonPackagingPipelineStorage'
STATIC_URL = "/static/"
PIPELINE_ENABLED=False
# Add split as another store for testing
MODULESTORE['default']['OPTIONS']['stores'].append(
{
'NAME': 'split',
'ENGINE': 'xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore',
'DOC_STORE_CONFIG': DOC_STORE_CONFIG,
'OPTIONS': {
'render_template': 'edxmako.shortcuts.render_to_string',
}
},
)
# Update module store settings per defaults for tests
update_module_store_settings(
MODULESTORE,
......
common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
......@@ -681,6 +681,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
version_guids.append(version_guid)
id_version_map[version_guid] = course_index
if not version_guids:
return []
matching_structures = self.find_structures_by_id(version_guids)
# get the blocks for each course index (s/b the root)
......
common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
......@@ -290,11 +290,9 @@ class TestMixedModuleStore(unittest.TestCase):
self.store.create_course('org_x', 'course_y', 'run_z', self.user_id)
# Draft:
# - One lookup to locate an item that exists
# - Two lookups to determine an item doesn't exist (one to check mongo, one to check split)
# problem: One lookup to locate an item that exists
# fake: one w/ wildcard version
# split has one lookup for the course and then one for the course items
# TODO: LMS-11220: Document why draft find count is [1, 1]
# TODO: LMS-11220: Document why split find count is [2, 2]
@ddt.data(('draft', [1, 1], 0), ('split', [2, 2], 0))
@ddt.unpack
def test_has_item(self, default_ms, max_find, max_send):
......@@ -317,10 +315,12 @@ class TestMixedModuleStore(unittest.TestCase):
with self.assertRaises(UnsupportedRevisionError):
self.store.has_item(self.fake_location, revision=ModuleStoreEnum.RevisionOption.draft_preferred)
# draft is 2 to compute inheritance
# split is 2 (would be 3 on course b/c it looks up the wiki_slug in definitions)
# TODO: LMS-11220: Document why draft find count is [2, 2]
# TODO: LMS-11220: Document why split find count is [3, 3]
# draft queries:
# problem: find draft item, find all items pertinent to inheritance computation
# non-existent problem: find draft, find published
# split:
# problem: active_versions, structure, then active_versions again?
# non-existent problem: ditto
@ddt.data(('draft', [2, 2], 0), ('split', [3, 3], 0))
@ddt.unpack
def test_get_item(self, default_ms, max_find, max_send):
......@@ -345,8 +345,10 @@ class TestMixedModuleStore(unittest.TestCase):
with self.assertRaises(UnsupportedRevisionError):
self.store.get_item(self.fake_location, revision=ModuleStoreEnum.RevisionOption.draft_preferred)
# compared to get_item for the course, draft asks for both draft and published
# TODO: LMS-11220: Document why split find count is 3
# Draft:
# wildcard query, 6! load pertinent items for inheritance calls, course root fetch (why)
# Split:
# active_versions (with regex), structure, and spurious active_versions refetch
@ddt.data(('draft', 8, 0), ('split', 3, 0))
@ddt.unpack
def test_get_items(self, default_ms, max_find, max_send):
......@@ -372,10 +374,11 @@ class TestMixedModuleStore(unittest.TestCase):
revision=ModuleStoreEnum.RevisionOption.draft_preferred
)
# draft: 2 to look in draft and then published and then 5 for updating ancestors.
# split: 3 to get the course structure & the course definition (show_calculator is scope content)
# before the change. 1 during change to refetch the definition. 3 afterward (b/c it calls get_item to return the "new" object).
# 2 sends to update index & structure (calculator is a setting field)
# draft: get draft, count parents, get parents, count & get grandparents, count & get greatgrand,
# count & get next ancestor (chapter's parent), count non-existent next ancestor, get inheritance
# sends: update problem and then each ancestor up to course (edit info)
# split: active_versions, definitions (calculator field), structures
# 2 sends to update index & structure (note, it would also be definition if a content field changed)
@ddt.data(('draft', 11, 5), ('split', 3, 2))
@ddt.unpack
def test_update_item(self, default_ms, max_find, max_send):
......@@ -638,8 +641,14 @@ class TestMixedModuleStore(unittest.TestCase):
# Check the parent for changes should return True and not throw an exception
self.assertTrue(self.store.has_changes(parent))
# TODO: LMS-11220: Document why split find count is 4
# TODO: LMS-11220: Document why split send count is 3
# 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.
# 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.unpack
def test_delete_item(self, default_ms, max_find, max_send):
......@@ -656,11 +665,17 @@ class TestMixedModuleStore(unittest.TestCase):
self.store.delete_item(self.writable_chapter_location, self.user_id)
# verify it's gone
# FIXME check both published and draft branches
with self.assertRaises(ItemNotFoundError):
self.store.get_item(self.writable_chapter_location)
# TODO: LMS-11220: Document why split find count is 4
# TODO: LMS-11220: Document why split send count is 3
# Draft:
# queries: find parent (definition.children), count versions of item, get parent, count grandparents,
# inheritance items, draft item, draft child, get errors, 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.unpack
def test_delete_private_vertical(self, default_ms, max_find, max_send):
......@@ -706,7 +721,12 @@ class TestMixedModuleStore(unittest.TestCase):
self.assertFalse(self.store.has_item(leaf_loc))
self.assertNotIn(vert_loc, course.children)
# TODO: LMS-11220: Document why split find count is 2
# Draft:
# find: find parent (definition.children) 2x, find draft item, check error state, 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.unpack
def test_delete_draft_vertical(self, default_ms, max_find, max_send):
......@@ -740,9 +760,15 @@ class TestMixedModuleStore(unittest.TestCase):
with check_mongo_calls(max_find, max_send):
self.store.delete_item(private_leaf.location, self.user_id)
# TODO: LMS-11220: Document why split find count is 5
# TODO: LMS-11220: Document why draft find count is 4
@ddt.data(('draft', 4, 0), ('split', 5, 0))
# Draft:
# 1) find all courses (wildcard),
# 2) get each course 1 at a time (1 course),
# 3) wildcard split if it has any (1) but it doesn't
# Split:
# 1) wildcard split search,
# 2-4) active_versions, structure, definition (s/b lazy; so, unnecessary)
# 5) wildcard draft mongo which has none
@ddt.data(('draft', 3, 0), ('split', 5, 0))
@ddt.unpack
def test_get_courses(self, default_ms, max_find, max_send):
self.initdb(default_ms)
......@@ -785,9 +811,8 @@ class TestMixedModuleStore(unittest.TestCase):
with self.assertRaises(AttributeError):
xml_store.create_course("org", "course", "run", self.user_id)
# draft is 2 to compute inheritance
# split is 3 (one for the index, one for the definition to check if the wiki is set, and one for the course structure
# TODO: LMS-11220: Document why split find count is 4
# draft is 2: find out which ms owns course, get item
# split: find out which ms owns course, active_versions, structure, definition (definition s/b unnecessary unless lazy is false)
@ddt.data(('draft', 2, 0), ('split', 4, 0))
@ddt.unpack
def test_get_course(self, default_ms, max_find, max_send):
......@@ -805,7 +830,8 @@ class TestMixedModuleStore(unittest.TestCase):
# notice this doesn't test getting a public item via draft_preferred which draft would have 2 hits (split
# still only 2)
# TODO: LMS-11220: Document why draft find count is 2
# Draft: count via definition.children query, then fetch via that query
# Split: active_versions, structure
@ddt.data(('draft', 2, 0), ('split', 2, 0))
@ddt.unpack
def test_get_parent_locations(self, default_ms, max_find, max_send):
......@@ -902,20 +928,22 @@ class TestMixedModuleStore(unittest.TestCase):
(child_to_delete_location, None, ModuleStoreEnum.RevisionOption.published_only),
])
# Mongo reads:
# First location:
# - count problem (1)
# - For each level of ancestors: (5)
# - Count ancestor
# - retrieve ancestor
# - compute inheritable data
# Second location:
# - load vertical
# - load inheritance data
# TODO: LMS-11220: Document why draft send count is 5
# TODO: LMS-11220: Document why draft find count is [19, 6]
# TODO: LMS-11220: Document why split find count is [2, 2]
# Draft:
# Problem path:
# 1. Get problem
# 2-3. count matches definition.children called 2x?
# 4. get parent via definition.children query
# 5-7. 2 counts and 1 get grandparent via definition.children
# 8-10. ditto for great-grandparent
# 11-13. ditto for next ancestor
# 14. fail count query looking for parent of course (unnecessary)
# 15. get course record direct query (not via definition.children) (already fetched in 13)
# 16. get items for inheritance computation
# 17. get vertical (parent of problem)
# 18. get items for inheritance computation (why? caching should handle)
# 19-20. get vertical_x1b (? why? this is the only ref in trace) & items for inheritance computation
# Chapter path: get chapter, count parents 2x, get parents, count non-existent grandparents
# Split: active_versions & structure
@ddt.data(('draft', [20, 5], 0), ('split', [2, 2], 0))
@ddt.unpack
def test_path_to_location(self, default_ms, num_finds, num_sends):
......@@ -936,6 +964,7 @@ class TestMixedModuleStore(unittest.TestCase):
)
for location, expected in should_work:
# each iteration has different find count, pop this iter's find count
with check_mongo_calls(num_finds.pop(0), num_sends):
self.assertEqual(path_to_location(self.store, location), expected)
......@@ -1074,6 +1103,8 @@ class TestMixedModuleStore(unittest.TestCase):
# It does not discard the child vertical, even though that child is a draft (with no published version)
self.assertEqual(num_children, len(reverted_parent.children))
# Draft: get all items which can be or should have parents
# Split: active_versions, structure
@ddt.data(('draft', 1, 0), ('split', 2, 0))
@ddt.unpack
def test_get_orphans(self, default_ms, max_find, max_send):
......@@ -1212,8 +1243,8 @@ class TestMixedModuleStore(unittest.TestCase):
self.assertEqual(self.user_id, block.subtree_edited_by)
self.assertGreater(datetime.datetime.now(UTC), block.subtree_edited_on)
# TODO: LMS-11220: Document why split find count is 2
# TODO: LMS-11220: Document why draft find count is 2
# Draft: wildcard search of draft and split
# Split: wildcard search of draft and split
@ddt.data(('draft', 2, 0), ('split', 2, 0))
@ddt.unpack
def test_get_courses_for_wiki(self, default_ms, max_find, max_send):
......@@ -1242,15 +1273,16 @@ class TestMixedModuleStore(unittest.TestCase):
self.assertEqual(len(self.store.get_courses_for_wiki('edX.simple.2012_Fall')), 0)
self.assertEqual(len(self.store.get_courses_for_wiki('no_such_wiki')), 0)
# Mongo reads:
# - load vertical
# - load vertical children
# - get last error
# Split takes 1 query to read the course structure, deletes all of the entries in memory, and loads the module from an in-memory cache
# Draft:
# Find: find vertical, find children, get last error
# 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
# 3-6. insert the 3 problems and 1 html as published
# Split: active_versions, 2 structures (pre & post published?)
# Sends:
# - insert structure
# - write index entry
# TODO: LMS-11220: Document why split find count is 3
@ddt.data(('draft', 3, 6), ('split', 3, 2))
@ddt.unpack
def test_unpublish(self, default_ms, max_find, max_send):
......@@ -1285,6 +1317,8 @@ class TestMixedModuleStore(unittest.TestCase):
)
self.assertIsNotNone(draft_xblock)
# Draft: specific query for revision None
# Split: active_versions, structure
@ddt.data(('draft', 1, 0), ('split', 2, 0))
@ddt.unpack
def test_has_published_version(self, default_ms, max_find, max_send):
......@@ -1727,9 +1761,9 @@ class TestMixedModuleStore(unittest.TestCase):
with self.store.default_store(fake_store):
pass # pragma: no cover
#=============================================================================================================
# ============================================================================================================
# General utils for not using django settings
#=============================================================================================================
# ============================================================================================================
def load_function(path):
......
common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
......@@ -89,11 +89,21 @@ class TestPublish(SplitWMongoCourseBoostrapper):
"""
vert_location = self.old_course_key.make_usage_key('vertical', block_id='Vert1')
item = self.draft_mongo.get_item(vert_location, 2)
# Vert1 has 3 children; so, publishes 4 nodes which may mean 4 inserts & 1 bulk remove
# TODO: LMS-11220: Document why find count is 25
# 25-June-2014 find calls are 19. Probably due to inheritance recomputation?
# 02-July-2014 send calls are 7. 5 from above, plus 2 for updating subtree edit info for Chapter1 and course
# find calls are 22. 19 from above, plus 3 for finding the parent of Vert1, Chapter1, and course
# Finds:
# 1 get draft vert,
# 2-10 for each child: (3 children x 3 queries each)
# get draft and then published child
# compute inheritance
# 11 get published vert
# 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
# 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):
self.draft_mongo.publish(item.location, self.user_id)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment