Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Preload definitions when collecting block structures

Browse Source
This commit is contained in:
Nimisha Asthagiri
2016-12-22 13:09:42 -05:00
parent 82647ee8c0
commit 99e6d53939
13 changed files with 287 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
View File

@@ -432,9 +432,11 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
if len(ids):
# Query the db for the definitions.
defs_from_db = self.db_connection.get_definitions(list(ids), course_key)
defs_from_db = list(self.db_connection.get_definitions(list(ids), course_key))
defs_dict = {d.get('_id'): d for d in defs_from_db}
# Add the retrieved definitions to the cache.
bulk_write_record.definitions.update({d.get('_id'): d for d in defs_from_db})
bulk_write_record.definitions_in_db.update(defs_dict.iterkeys())
bulk_write_record.definitions.update(defs_dict)
definitions.extend(defs_from_db)
return definitions
@@ -772,11 +774,16 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
Load the definitions into each block if lazy is in kwargs and is False;
otherwise, do not load the definitions - they'll be loaded later when needed.
"""
lazy = kwargs.pop('lazy', True)
should_cache_items = not lazy
runtime = self._get_cache(course_entry.structure['_id'])
if runtime is None:
lazy = kwargs.pop('lazy', True)
runtime = self.create_runtime(course_entry, lazy)
self._add_cache(course_entry.structure['_id'], runtime)
should_cache_items = True
if should_cache_items:
self.cache_items(runtime, block_keys, course_entry.course_key, depth, lazy)
return [runtime.load_item(block_key, course_entry, **kwargs) for block_key in block_keys]

106
common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py
View File

@@ -13,7 +13,8 @@ from xmodule.modulestore.xml_exporter import export_course_to_xml
from xmodule.modulestore.tests.factories import check_mongo_calls
from xmodule.modulestore.tests.utils import (
MixedModulestoreBuilder, VersioningModulestoreBuilder,
MongoModulestoreBuilder, TEST_DATA_DIR
MongoModulestoreBuilder, TEST_DATA_DIR,
MemoryCache,
)
MIXED_OLD_MONGO_MODULESTORE_BUILDER = MixedModulestoreBuilder([('draft', MongoModulestoreBuilder())])
@@ -87,6 +88,46 @@ class CountMongoCallsCourseTraversal(TestCase):
to the leaf nodes.
"""
def _traverse_blocks_in_course(self, course, access_all_block_fields):
"""
Traverses all the blocks in the given course.
If access_all_block_fields is True, also reads all the
xblock fields in each block in the course.
"""
all_blocks = []
stack = [course]
while stack:
curr_block = stack.pop()
all_blocks.append(curr_block)
if curr_block.has_children:
for block in reversed(curr_block.get_children()):
stack.append(block)
if access_all_block_fields:
# Read the fields on each block in order to ensure each block and its definition is loaded.
for xblock in all_blocks:
for __, field in xblock.fields.iteritems():
if field.is_set_on(xblock):
__ = field.read_from(xblock)
def _import_course(self, content_store, modulestore):
"""
Imports a course for testing.
Returns the course key.
"""
course_key = modulestore.make_course_key('a', 'course', 'course')
import_course_from_xml(
modulestore,
'test_user',
TEST_DATA_DIR,
source_dirs=['manual-testing-complete'],
static_content_store=content_store,
target_id=course_key,
create_if_not_present=True,
raise_on_failure=True,
)
return course_key
# Suppose you want to traverse a course - maybe accessing the fields of each XBlock in the course,
# maybe not. What parameters should one use for get_course() in order to minimize the number of
# mongo calls? The tests below both ensure that code changes don't increase the number of mongo calls
@@ -109,52 +150,43 @@ class CountMongoCallsCourseTraversal(TestCase):
# (if you'll eventually access all the fields and load all the definitions anyway).
(MIXED_SPLIT_MODULESTORE_BUILDER, None, False, True, 4),
(MIXED_SPLIT_MODULESTORE_BUILDER, None, True, True, 38),
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, True, 131),
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, True, 38),
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, True, 38),
(MIXED_SPLIT_MODULESTORE_BUILDER, None, False, False, 4),
(MIXED_SPLIT_MODULESTORE_BUILDER, None, True, False, 4),
# TODO: The call count below seems like a bug - should be 4?
# Seems to be related to using self.lazy in CachingDescriptorSystem.get_module_data().
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, False, 131),
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, False, 4),
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, False, 4)
)
@ddt.unpack
def test_number_mongo_calls(self, store, depth, lazy, access_all_block_fields, num_mongo_calls):
with store.build() as (source_content, source_store):
source_course_key = source_store.make_course_key('a', 'course', 'course')
# First, import a course.
import_course_from_xml(
source_store,
'test_user',
TEST_DATA_DIR,
source_dirs=['manual-testing-complete'],
static_content_store=source_content,
target_id=source_course_key,
create_if_not_present=True,
raise_on_failure=True,
)
def test_number_mongo_calls(self, store_builder, depth, lazy, access_all_block_fields, num_mongo_calls):
request_cache = MemoryCache()
with store_builder.build(request_cache=request_cache) as (content_store, modulestore):
course_key = self._import_course(content_store, modulestore)
# Course traversal modeled after the traversal done here:
# lms/djangoapps/mobile_api/video_outlines/serializers.py:BlockOutline
# Starting at the root course block, do a breadth-first traversal using
# get_children() to retrieve each block's children.
with check_mongo_calls(num_mongo_calls):
with source_store.bulk_operations(source_course_key):
start_block = source_store.get_course(source_course_key, depth=depth, lazy=lazy)
all_blocks = []
stack = [start_block]
while stack:
curr_block = stack.pop()
all_blocks.append(curr_block)
if curr_block.has_children:
for block in reversed(curr_block.get_children()):
stack.append(block)
with modulestore.bulk_operations(course_key):
start_block = modulestore.get_course(course_key, depth=depth, lazy=lazy)
self._traverse_blocks_in_course(start_block, access_all_block_fields)
if access_all_block_fields:
# Read the fields on each block in order to ensure each block and its definition is loaded.
for xblock in all_blocks:
for __, field in xblock.fields.iteritems():
if field.is_set_on(xblock):
__ = field.read_from(xblock)
@ddt.data(
(MIXED_OLD_MONGO_MODULESTORE_BUILDER, 176),
(MIXED_SPLIT_MODULESTORE_BUILDER, 5),
)
@ddt.unpack
def test_lazy_when_course_previously_cached(self, store_builder, num_mongo_calls):
request_cache = MemoryCache()
with store_builder.build(request_cache=request_cache) as (content_store, modulestore):
course_key = self._import_course(content_store, modulestore)
with check_mongo_calls(num_mongo_calls):
with modulestore.bulk_operations(course_key):
# assume the course was retrieved earlier
course = modulestore.get_course(course_key, depth=0, lazy=True)
# and then subsequently retrieved with the lazy and depth=None values
course = modulestore.get_item(course.location, depth=None, lazy=False)
self._traverse_blocks_in_course(course, access_all_block_fields=True)

15
common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py
View File

@@ -1,3 +1,7 @@
"""
Tests for bulk operations in Split Modulestore.
"""
# pylint: disable=protected-access
import copy
import ddt
import unittest
@@ -444,6 +448,17 @@ class TestBulkWriteMixinFindMethods(TestBulkWriteMixin):
else:
self.assertNotIn(db_definition(_id), results)
def test_get_definitions_doesnt_update_db(self):
test_ids = [1, 2]
db_definition = lambda _id: {'db': 'definition', '_id': _id}
db_definitions = [db_definition(_id) for _id in test_ids]
self.conn.get_definitions.return_value = db_definitions
self.bulk._begin_bulk_operation(self.course_key)
self.bulk.get_definitions(self.course_key, test_ids)
self.bulk._end_bulk_operation(self.course_key)
self.assertFalse(self.conn.insert_definition.called)
def test_no_bulk_find_structures_derived_from(self):
ids = [Mock(name='id')]
self.conn.find_structures_derived_from.return_value = [MagicMock(name='result')]

25
common/lib/xmodule/xmodule/modulestore/tests/utils.py
View File

@@ -194,7 +194,7 @@ class MemoryCache(object):
the modulestore, and stores the data in a dictionary in memory.
"""
def __init__(self):
self._data = {}
self.data = {}
def get(self, key, default=None):
"""
@@ -204,7 +204,7 @@ class MemoryCache(object):
key: The key to update.
default: The value to return if the key hasn't been set previously.
"""
return self._data.get(key, default)
return self.data.get(key, default)
def set(self, key, value):
"""
@@ -214,7 +214,7 @@ class MemoryCache(object):
key: The key to update.
value: The value change the key to.
"""
self._data[key] = value
self.data[key] = value
class MongoContentstoreBuilder(object):
@@ -255,19 +255,19 @@ class StoreBuilderBase(object):
"""
contentstore = kwargs.pop('contentstore', None)
if not contentstore:
with self.build_without_contentstore() as (contentstore, modulestore):
with self.build_without_contentstore(**kwargs) as (contentstore, modulestore):
yield contentstore, modulestore
else:
with self.build_with_contentstore(contentstore) as modulestore:
with self.build_with_contentstore(contentstore, **kwargs) as modulestore:
yield modulestore
@contextmanager
def build_without_contentstore(self):
def build_without_contentstore(self, **kwargs):
"""
Build both the contentstore and the modulestore.
"""
with MongoContentstoreBuilder().build() as contentstore:
with self.build_with_contentstore(contentstore) as modulestore:
with self.build_with_contentstore(contentstore, **kwargs) as modulestore:
yield contentstore, modulestore
@@ -276,7 +276,7 @@ class MongoModulestoreBuilder(StoreBuilderBase):
A builder class for a DraftModuleStore.
"""
@contextmanager
def build_with_contentstore(self, contentstore):
def build_with_contentstore(self, contentstore, **kwargs):
"""
A contextmanager that returns an isolated mongo modulestore, and then deletes
all of its data at the end of the context.
@@ -324,7 +324,7 @@ class VersioningModulestoreBuilder(StoreBuilderBase):
A builder class for a VersioningModuleStore.
"""
@contextmanager
def build_with_contentstore(self, contentstore):
def build_with_contentstore(self, contentstore, **kwargs):
"""
A contextmanager that returns an isolated versioning modulestore, and then deletes
all of its data at the end of the context.
@@ -347,6 +347,7 @@ class VersioningModulestoreBuilder(StoreBuilderBase):
fs_root,
render_template=repr,
xblock_mixins=XBLOCK_MIXINS,
**kwargs
)
modulestore.ensure_indexes()
@@ -369,7 +370,7 @@ class XmlModulestoreBuilder(StoreBuilderBase):
"""
# pylint: disable=unused-argument
@contextmanager
def build_with_contentstore(self, contentstore=None, course_ids=None):
def build_with_contentstore(self, contentstore=None, course_ids=None, **kwargs):
"""
A contextmanager that returns an isolated xml modulestore
@@ -403,7 +404,7 @@ class MixedModulestoreBuilder(StoreBuilderBase):
self.mixed_modulestore = None
@contextmanager
def build_with_contentstore(self, contentstore):
def build_with_contentstore(self, contentstore, **kwargs):
"""
A contextmanager that returns a mixed modulestore built on top of modulestores
generated by other builder classes.
@@ -414,7 +415,7 @@ class MixedModulestoreBuilder(StoreBuilderBase):
"""
names, generators = zip(*self.store_builders)
with nested(*(gen.build_with_contentstore(contentstore) for gen in generators)) as modulestores:
with nested(*(gen.build_with_contentstore(contentstore, **kwargs) for gen in generators)) as modulestores:
# Make the modulestore creation function just return the already-created modulestores
store_iterator = iter(modulestores)
next_modulestore = lambda *args, **kwargs: store_iterator.next()