Commit e624ff3b by Don Mitchell Committed by Calen Pennington

Make path_to_location use bulk_write_operation for performance.

parent 7d8088d2
...@@ -651,5 +651,8 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -651,5 +651,8 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
If course_id is None, the default store is used. If course_id is None, the default store is used.
""" """
store = self._get_modulestore_for_courseid(course_id) store = self._get_modulestore_for_courseid(course_id)
with store.bulk_write_operations(course_id): if hasattr(store, 'bulk_write_operations'):
with store.bulk_write_operations(course_id):
yield
else:
yield yield
...@@ -68,39 +68,41 @@ def path_to_location(modulestore, usage_key): ...@@ -68,39 +68,41 @@ def path_to_location(modulestore, usage_key):
newpath = (next_usage, path) newpath = (next_usage, path)
queue.append((parent, newpath)) queue.append((parent, newpath))
if not modulestore.has_item(usage_key): # doesn't write but does multiple reads. bulk_write minimizes reads too
raise ItemNotFoundError(usage_key) with modulestore.bulk_write_operations(usage_key.course_key):
if not modulestore.has_item(usage_key):
path = find_path_to_course() raise ItemNotFoundError(usage_key)
if path is None:
raise NoPathToItem(usage_key) path = find_path_to_course()
if path is None:
n = len(path) raise NoPathToItem(usage_key)
course_id = path[0].course_key
# pull out the location names n = len(path)
chapter = path[1].name if n > 1 else None course_id = path[0].course_key
section = path[2].name if n > 2 else None # pull out the location names
# Figure out the position chapter = path[1].name if n > 1 else None
position = None section = path[2].name if n > 2 else None
# Figure out the position
# This block of code will find the position of a module within a nested tree position = None
# of modules. If a problem is on tab 2 of a sequence that's on tab 3 of a
# sequence, the resulting position is 3_2. However, no positional modules # This block of code will find the position of a module within a nested tree
# (e.g. sequential and videosequence) currently deal with this form of # of modules. If a problem is on tab 2 of a sequence that's on tab 3 of a
# representing nested positions. This needs to happen before jumping to a # sequence, the resulting position is 3_2. However, no positional modules
# module nested in more than one positional module will work. # (e.g. sequential and videosequence) currently deal with this form of
if n > 3: # representing nested positions. This needs to happen before jumping to a
position_list = [] # module nested in more than one positional module will work.
for path_index in range(2, n - 1): if n > 3:
category = path[path_index].block_type position_list = []
if category == 'sequential' or category == 'videosequence': for path_index in range(2, n - 1):
section_desc = modulestore.get_item(path[path_index]) category = path[path_index].block_type
# this calls get_children rather than just children b/c old mongo includes private children if category == 'sequential' or category == 'videosequence':
# in children but not in get_children section_desc = modulestore.get_item(path[path_index])
child_locs = [c.location for c in section_desc.get_children()] # this calls get_children rather than just children b/c old mongo includes private children
# positions are 1-indexed, and should be strings to be consistent with # in children but not in get_children
# url parsing. child_locs = [c.location for c in section_desc.get_children()]
position_list.append(str(child_locs.index(path[path_index + 1]) + 1)) # positions are 1-indexed, and should be strings to be consistent with
position = "_".join(position_list) # url parsing.
position_list.append(str(child_locs.index(path[path_index + 1]) + 1))
return (course_id, chapter, section, position) position = "_".join(position_list)
return (course_id, chapter, section, position)
...@@ -717,7 +717,7 @@ class TestMixedModuleStore(unittest.TestCase): ...@@ -717,7 +717,7 @@ class TestMixedModuleStore(unittest.TestCase):
# TODO: LMS-11220: Document why draft send count is 5 # TODO: LMS-11220: Document why draft send count is 5
# TODO: LMS-11220: Document why draft find count is 18 # TODO: LMS-11220: Document why draft find count is 18
# TODO: LMS-11220: Document why split find count is 16 # TODO: LMS-11220: Document why split find count is 16
@ddt.data(('draft', [18, 5], 0), ('split', [16, 6], 0)) @ddt.data(('draft', [19, 5], 0), ('split', [2, 2], 0))
@ddt.unpack @ddt.unpack
def test_path_to_location(self, default_ms, num_finds, num_sends): def test_path_to_location(self, default_ms, num_finds, num_sends):
""" """
...@@ -736,7 +736,6 @@ class TestMixedModuleStore(unittest.TestCase): ...@@ -736,7 +736,6 @@ class TestMixedModuleStore(unittest.TestCase):
(course_key, "Chapter_x", None, None)), (course_key, "Chapter_x", None, None)),
) )
mongo_store = self.store._get_modulestore_for_courseid(self._course_key_from_string(self.MONGO_COURSEID))
for location, expected in should_work: for location, expected in should_work:
with check_mongo_calls(num_finds.pop(0), num_sends): with check_mongo_calls(num_finds.pop(0), num_sends):
self.assertEqual(path_to_location(self.store, location), expected) self.assertEqual(path_to_location(self.store, location), expected)
......
...@@ -21,6 +21,7 @@ from django.contrib.auth.models import User ...@@ -21,6 +21,7 @@ from django.contrib.auth.models import User
from xblock.runtime import KeyValueStore from xblock.runtime import KeyValueStore
from xblock.exceptions import KeyValueMultiSaveError, InvalidScopeError from xblock.exceptions import KeyValueMultiSaveError, InvalidScopeError
from xblock.fields import Scope, UserScope from xblock.fields import Scope, UserScope
from xmodule.modulestore.django import modulestore
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -109,7 +110,8 @@ class FieldDataCache(object): ...@@ -109,7 +110,8 @@ class FieldDataCache(object):
return descriptors return descriptors
descriptors = get_child_descriptors(descriptor, depth, descriptor_filter) with modulestore().bulk_write_operations(descriptor.location.course_key):
descriptors = get_child_descriptors(descriptor, depth, descriptor_filter)
return FieldDataCache(descriptors, course_id, user, select_for_update) return FieldDataCache(descriptors, course_id, user, select_for_update)
......
...@@ -326,14 +326,15 @@ class TestTOC(ModuleStoreTestCase): ...@@ -326,14 +326,15 @@ class TestTOC(ModuleStoreTestCase):
self.request = factory.get(chapter_url) self.request = factory.get(chapter_url)
self.request.user = UserFactory() self.request.user = UserFactory()
self.modulestore = self.store._get_modulestore_for_courseid(self.course_key) self.modulestore = self.store._get_modulestore_for_courseid(self.course_key)
self.toy_course = self.store.get_course(self.toy_loc, depth=2)
with check_mongo_calls(num_finds, num_sends): with check_mongo_calls(num_finds, num_sends):
self.toy_course = self.store.get_course(self.toy_loc, depth=2)
self.field_data_cache = FieldDataCache.cache_for_descriptor_descendents( self.field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
self.toy_loc, self.request.user, self.toy_course, depth=2) self.toy_loc, self.request.user, self.toy_course, depth=2
)
# TODO: LMS-11220: Document why split find count is 21 # TODO: LMS-11220: Document why split find count is 21
@ddt.data((ModuleStoreEnum.Type.mongo, 3, 0), (ModuleStoreEnum.Type.split, 21, 0)) @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 5, 0))
@ddt.unpack @ddt.unpack
def test_toc_toy_from_chapter(self, default_ms, num_finds, num_sends): def test_toc_toy_from_chapter(self, default_ms, num_finds, num_sends):
with self.store.default_store(default_ms): with self.store.default_store(default_ms):
...@@ -361,7 +362,7 @@ class TestTOC(ModuleStoreTestCase): ...@@ -361,7 +362,7 @@ class TestTOC(ModuleStoreTestCase):
self.assertIn(toc_section, actual) self.assertIn(toc_section, actual)
# TODO: LMS-11220: Document why split find count is 21 # TODO: LMS-11220: Document why split find count is 21
@ddt.data((ModuleStoreEnum.Type.mongo, 3, 0), (ModuleStoreEnum.Type.split, 21, 0)) @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 5, 0))
@ddt.unpack @ddt.unpack
def test_toc_toy_from_section(self, default_ms, num_finds, num_sends): def test_toc_toy_from_section(self, default_ms, num_finds, num_sends):
with self.store.default_store(default_ms): with self.store.default_store(default_ms):
......
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