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

Merge pull request #13069 from edx/christina/cache-disable-state

Browse Source 
Use request cache for disabled_xblock_types
This commit is contained in:
Christina Roberts
2016-07-25 17:20:49 -04:00
committed by GitHub
parent 27d585e0fc 682ee5eac1
commit 0438878ab9
17 changed files with 103 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
common/lib/xmodule/xmodule/modulestore/__init__.py
View File

@@ -1170,7 +1170,7 @@ class ModuleStoreReadBase(BulkOperationsMixin, ModuleStoreRead):
contentstore=None,
doc_store_config=None, # ignore if passed up
metadata_inheritance_cache_subsystem=None, request_cache=None,
xblock_mixins=(), xblock_select=None, xblock_field_data_wrappers=(), disabled_xblock_types=(), # pylint: disable=bad-continuation
xblock_mixins=(), xblock_select=None, xblock_field_data_wrappers=(), disabled_xblock_types=lambda: [], # pylint: disable=bad-continuation
# temporary parms to enable backward compatibility. remove once all envs migrated
db=None, collection=None, host=None, port=None, tz_aware=True, user=None, password=None,
# allow lower level init args to pass harmlessly

39
common/lib/xmodule/xmodule/modulestore/django.py
View File

@@ -19,7 +19,6 @@ from django.conf import settings
if not settings.configured:
settings.configure()
from django.db.models.signals import post_save
from django.core.cache import caches, InvalidCacheBackendError
import django.dispatch
import django.utils
@@ -51,10 +50,8 @@ except ImportError:
try:
from xblock_django.api import disabled_xblocks
from xblock_django.models import XBlockConfiguration
except ImportError:
disabled_xblocks = None
XBlockConfiguration = None
log = logging.getLogger(__name__)
ASSET_IGNORE_REGEX = getattr(settings, "ASSET_IGNORE_REGEX", r"(^\._.*$)|(^\.DS_Store$)|(^.*~$)")
@@ -191,13 +188,26 @@ def create_modulestore_instance(
if 'read_preference' in doc_store_config:
doc_store_config['read_preference'] = getattr(ReadPreference, doc_store_config['read_preference'])
if disabled_xblocks:
disabled_xblock_types = [block.name for block in disabled_xblocks()]
else:
disabled_xblock_types = ()
xblock_field_data_wrappers = [load_function(path) for path in settings.XBLOCK_FIELD_DATA_WRAPPERS]
def fetch_disabled_xblock_types():
"""
Get the disabled xblock names, using the request_cache if possible to avoid hitting
a database every time the list is needed.
"""
# If the import could not be loaded, return an empty list.
if disabled_xblocks is None:
return []
if request_cache:
if 'disabled_xblock_types' not in request_cache.data:
request_cache.data['disabled_xblock_types'] = [block.name for block in disabled_xblocks()]
return request_cache.data['disabled_xblock_types']
else:
disabled_xblock_types = [block.name for block in disabled_xblocks()]
return disabled_xblock_types
return class_(
contentstore=content_store,
metadata_inheritance_cache_subsystem=metadata_inheritance_cache,
@@ -205,7 +215,7 @@ def create_modulestore_instance(
xblock_mixins=getattr(settings, 'XBLOCK_MIXINS', ()),
xblock_select=getattr(settings, 'XBLOCK_SELECT_FUNCTION', None),
xblock_field_data_wrappers=xblock_field_data_wrappers,
disabled_xblock_types=disabled_xblock_types,
disabled_xblock_types=fetch_disabled_xblock_types,
doc_store_config=doc_store_config,
i18n_service=i18n_service or ModuleI18nService,
fs_service=fs_service or xblock.reference.plugins.FSService(),
@@ -255,17 +265,6 @@ def clear_existing_modulestores():
_MIXED_MODULESTORE = None
if XBlockConfiguration and disabled_xblocks:
@django.dispatch.receiver(post_save, sender=XBlockConfiguration)
def reset_disabled_xblocks(sender, instance, **kwargs): # pylint: disable=unused-argument
"""
If XBlockConfiguation and disabled_xblocks are available, register a signal handler
to update disabled_xblocks on model changes.
"""
disabled_xblock_types = [block.name for block in disabled_xblocks()]
modulestore().disabled_xblock_types = disabled_xblock_types
class ModuleI18nService(object):
"""
Implement the XBlock runtime "i18n" service.

19
common/lib/xmodule/xmodule/modulestore/mixed.py
View File

@@ -185,25 +185,6 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
self.mappings[course_key] = store
self.modulestores.append(store)
@property
def disabled_xblock_types(self):
"""
Returns the list of disabled xblock types.
"""
return None if not hasattr(self, "_disabled_xblock_types") else self._disabled_xblock_types
@disabled_xblock_types.setter
def disabled_xblock_types(self, value):
"""
Sets the list of disabled xblock types, and propagates down
to child modulestores if available.
"""
self._disabled_xblock_types = value # pylint: disable=attribute-defined-outside-init
if hasattr(self, 'modulestores'):
for store in self.modulestores:
store.disabled_xblock_types = value
def _clean_locator_for_mapping(self, locator):
"""
In order for mapping to work, the locator must be minimal--no version, no branch--

4
common/lib/xmodule/xmodule/x_module.py
View File

@@ -1371,7 +1371,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):
"""
# pylint: disable=bad-continuation
def __init__(
self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=(), **kwargs
self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], **kwargs
):
"""
load_item: Takes a Location and returns an XModuleDescriptor
@@ -1439,7 +1439,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):
"""
Returns a subclass of :class:`.XBlock` that corresponds to the specified `block_type`.
"""
if block_type in self.disabled_xblock_types:
if block_type in self.disabled_xblock_types():
return self.default_class
return super(DescriptorSystem, self).load_block_type(block_type)

54
lms/djangoapps/ccx/tests/test_field_override_performance.py
View File

@@ -229,18 +229,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
# # of sql queries to default,
# # of mongo queries,
# )
('no_overrides', 1, True, False): (34, 6),
('no_overrides', 2, True, False): (40, 6),
('no_overrides', 3, True, False): (50, 6),
('ccx', 1, True, False): (34, 6),
('ccx', 2, True, False): (40, 6),
('ccx', 3, True, False): (50, 6),
('no_overrides', 1, False, False): (34, 6),
('no_overrides', 2, False, False): (40, 6),
('no_overrides', 3, False, False): (50, 6),
('ccx', 1, False, False): (34, 6),
('ccx', 2, False, False): (40, 6),
('ccx', 3, False, False): (50, 6),
('no_overrides', 1, True, False): (35, 6),
('no_overrides', 2, True, False): (41, 6),
('no_overrides', 3, True, False): (51, 6),
('ccx', 1, True, False): (35, 6),
('ccx', 2, True, False): (41, 6),
('ccx', 3, True, False): (51, 6),
('no_overrides', 1, False, False): (35, 6),
('no_overrides', 2, False, False): (41, 6),
('no_overrides', 3, False, False): (51, 6),
('ccx', 1, False, False): (35, 6),
('ccx', 2, False, False): (41, 6),
('ccx', 3, False, False): (51, 6),
}
@@ -252,19 +252,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True
TEST_DATA = {
('no_overrides', 1, True, False): (34, 3),
('no_overrides', 2, True, False): (40, 3),
('no_overrides', 3, True, False): (50, 3),
('ccx', 1, True, False): (34, 3),
('ccx', 2, True, False): (40, 3),
('ccx', 3, True, False): (50, 3),
('ccx', 1, True, True): (35, 3),
('ccx', 2, True, True): (41, 3),
('ccx', 3, True, True): (51, 3),
('no_overrides', 1, False, False): (34, 3),
('no_overrides', 2, False, False): (40, 3),
('no_overrides', 3, False, False): (50, 3),
('ccx', 1, False, False): (34, 3),
('ccx', 2, False, False): (40, 3),
('ccx', 3, False, False): (50, 3),
('no_overrides', 1, True, False): (35, 3),
('no_overrides', 2, True, False): (41, 3),
('no_overrides', 3, True, False): (51, 3),
('ccx', 1, True, False): (35, 3),
('ccx', 2, True, False): (41, 3),
('ccx', 3, True, False): (51, 3),
('ccx', 1, True, True): (36, 3),
('ccx', 2, True, True): (42, 3),
('ccx', 3, True, True): (52, 3),
('no_overrides', 1, False, False): (35, 3),
('no_overrides', 2, False, False): (41, 3),
('no_overrides', 3, False, False): (51, 3),
('ccx', 1, False, False): (35, 3),
('ccx', 2, False, False): (41, 3),
('ccx', 3, False, False): (51, 3),
}

4
lms/djangoapps/courseware/tests/test_course_info.py
View File

@@ -317,7 +317,7 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest
self.assertEqual(resp.status_code, 200)
def test_num_queries_instructor_paced(self):
self.fetch_course_info_with_queries(self.instructor_paced_course, 22, 4)
self.fetch_course_info_with_queries(self.instructor_paced_course, 23, 4)
def test_num_queries_self_paced(self):
self.fetch_course_info_with_queries(self.self_paced_course, 22, 4)
self.fetch_course_info_with_queries(self.self_paced_course, 23, 4)

4
lms/djangoapps/courseware/tests/test_entrance_exam.py
View File

@@ -293,7 +293,9 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest
"""
test entrance exam score. we will hit the method get_entrance_exam_score to verify exam score.
"""
with self.assertNumQueries(1):
# One query is for getting the list of disabled XBlocks (which is
# then stored in the request).
with self.assertNumQueries(2):
exam_score = get_entrance_exam_score(self.request, self.course)
self.assertEqual(exam_score, 0)

27
lms/djangoapps/courseware/tests/test_module_render.py
View File

@@ -19,7 +19,6 @@ from mock import MagicMock, patch, Mock
from opaque_keys.edx.keys import UsageKey, CourseKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from pyquery import PyQuery
from courseware.module_render import hash_resource
from xblock.field_data import FieldData
from xblock.runtime import Runtime
from xblock.fields import ScopeIds
@@ -2225,9 +2224,7 @@ class TestDisabledXBlockTypes(ModuleStoreTestCase):
# pylint: disable=no-member
def setUp(self):
super(TestDisabledXBlockTypes, self).setUp()
for store in self.store.modulestores:
store.disabled_xblock_types = ('video',)
XBlockConfiguration(name='video', enabled=False).save()
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_get_item(self, default_ms):
@@ -2240,15 +2237,27 @@ class TestDisabledXBlockTypes(ModuleStoreTestCase):
"""Tests that the list of disabled xblocks can dynamically update."""
with self.store.default_store(default_ms):
course = CourseFactory()
self._verify_descriptor('problem', course, 'CapaDescriptorWithMixins')
item_usage_id = self._verify_descriptor('problem', course, 'CapaDescriptorWithMixins')
XBlockConfiguration(name='problem', enabled=False).save()
self._verify_descriptor('problem', course, 'RawDescriptorWithMixins')
def _verify_descriptor(self, category, course, descriptor):
# First verify that the cached value is used until there is a new request cache.
self._verify_descriptor('problem', course, 'CapaDescriptorWithMixins', item_usage_id)
# Now simulate a new request cache.
self.store.request_cache.data = {}
self._verify_descriptor('problem', course, 'RawDescriptorWithMixins', item_usage_id)
def _verify_descriptor(self, category, course, descriptor, item_id=None):
"""
Helper method that gets an item with the specified category from the
modulestore and verifies that it has the expected descriptor name.
Returns the item's usage_id.
"""
item = ItemFactory(category=category, parent=course)
item = self.store.get_item(item.scope_ids.usage_id)
if not item_id:
item = ItemFactory(category=category, parent=course)
item_id = item.scope_ids.usage_id
item = self.store.get_item(item_id)
self.assertEqual(item.__class__.__name__, descriptor)
return item_id

2
lms/djangoapps/courseware/tests/test_views.py
View File

@@ -1346,7 +1346,7 @@ class ProgressPageTests(ModuleStoreTestCase):
self.assertContains(resp, u"Download Your Certificate")
@ddt.data(
*itertools.product(((47, 4, True), (47, 4, False)), (True, False))
*itertools.product(((48, 4, True), (48, 4, False)), (True, False))
)
@ddt.unpack
def test_query_counts(self, (sql_calls, mongo_calls, self_paced), self_paced_enabled):

8
lms/djangoapps/django_comment_client/base/tests.py
View File

@@ -372,8 +372,8 @@ class ViewsQueryCountTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSet
return inner
@ddt.data(
(ModuleStoreEnum.Type.mongo, 3, 4, 31),
(ModuleStoreEnum.Type.split, 3, 13, 31),
(ModuleStoreEnum.Type.mongo, 3, 4, 32),
(ModuleStoreEnum.Type.split, 3, 13, 32),
)
@ddt.unpack
@count_queries
@@ -381,8 +381,8 @@ class ViewsQueryCountTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSet
self.create_thread_helper(mock_request)
@ddt.data(
(ModuleStoreEnum.Type.mongo, 3, 3, 25),
(ModuleStoreEnum.Type.split, 3, 10, 25),
(ModuleStoreEnum.Type.mongo, 3, 3, 26),
(ModuleStoreEnum.Type.split, 3, 10, 26),
)
@ddt.unpack
@count_queries

11
lms/djangoapps/django_comment_client/forum/tests.py
View File

@@ -341,9 +341,14 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
MODULESTORE = TEST_DATA_MONGO_MODULESTORE
@ddt.data(
# old mongo with cache
(ModuleStoreEnum.Type.mongo, 1, 6, 4, 17, 8),
(ModuleStoreEnum.Type.mongo, 50, 6, 4, 17, 8),
# Old mongo with cache. There is an additional SQL query for old mongo
# because the first time that disabled_xblocks is queried is in call_single_thread,
# vs. the creation of the course (CourseFactory.create). The creation of the
# course is outside the context manager that is verifying the number of queries,
# and with split mongo, that method ends up querying disabled_xblocks (which is then
# cached and hence not queried as part of call_single_thread).
(ModuleStoreEnum.Type.mongo, 1, 6, 4, 18, 8),
(ModuleStoreEnum.Type.mongo, 50, 6, 4, 18, 8),
# split mongo: 3 queries, regardless of thread response size.
(ModuleStoreEnum.Type.split, 1, 3, 3, 17, 8),
(ModuleStoreEnum.Type.split, 50, 3, 3, 17, 8),

4
lms/djangoapps/grades/tests/test_grades.py
View File

@@ -380,7 +380,9 @@ class TestGetModuleScore(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
"""
Test test_get_module_score
"""
with self.assertNumQueries(1):
# One query is for getting the list of disabled XBlocks (which is
# then stored in the request).
with self.assertNumQueries(2):
score = get_module_score(self.request.user, self.course, self.seq1)
self.assertEqual(score, 0)

4
openedx/core/djangoapps/bookmarks/tests/test_api.py
View File

@@ -123,7 +123,7 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
"""
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
with self.assertNumQueries(9):
with self.assertNumQueries(10):
bookmark_data = api.create_bookmark(user=self.user, usage_key=self.vertical_2.location)
self.assert_bookmark_event_emitted(
@@ -144,7 +144,7 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
"""
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
with self.assertNumQueries(9):
with self.assertNumQueries(10):
bookmark_data = api.create_bookmark(user=self.user, usage_key=self.vertical_2.location)
self.assert_bookmark_event_emitted(

6
openedx/core/djangoapps/bookmarks/tests/test_models.py
View File

@@ -48,9 +48,9 @@ class BookmarksTestsBase(ModuleStoreTestCase):
self.admin = AdminFactory()
self.user = UserFactory.create(password=self.TEST_PASSWORD)
self.other_user = UserFactory.create(password=self.TEST_PASSWORD)
self.setup_test_data(self.STORE_TYPE)
self.setup_data(self.STORE_TYPE)
def setup_test_data(self, store_type=ModuleStoreEnum.Type.mongo):
def setup_data(self, store_type=ModuleStoreEnum.Type.mongo):
""" Create courses and add some test blocks. """
with self.store.default_store(store_type):
@@ -273,7 +273,7 @@ class BookmarkModelTests(BookmarksTestsBase):
is needed to fetch the parent blocks.
"""
self.setup_test_data(store_type)
self.setup_data(store_type)
user = UserFactory.create()
expected_path = [PathItem(

2
openedx/core/djangoapps/bookmarks/tests/test_services.py
View File

@@ -70,7 +70,7 @@ class BookmarksServiceTests(BookmarksTestsBase):
self.bookmark_service.set_bookmarked(usage_key=UsageKey.from_string("i4x://ed/ed/ed/interactive"))
)
with self.assertNumQueries(9):
with self.assertNumQueries(10):
self.assertTrue(self.bookmark_service.set_bookmarked(usage_key=self.vertical_2.location))
def test_unset_bookmarked(self):

8
openedx/core/djangoapps/bookmarks/tests/test_tasks.py
View File

@@ -4,6 +4,8 @@ Tests for tasks.
import ddt
from nose.plugins.attrib import attr
from django.conf import settings
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.factories import check_mongo_calls, ItemFactory
@@ -152,6 +154,12 @@ class XBlockCacheTaskTests(BookmarksTestsBase):
"""
course = getattr(self, course_attr)
if settings.ROOT_URLCONF == 'lms.urls':
# When the tests run under LMS, there is a certificates course_published
# signal handler that runs and causes the number of queries to be one more
# (due to the check for disabled_xblocks in django.py).
expected_sql_queries += 1
with self.assertNumQueries(expected_sql_queries):
_update_xblocks_cache(course.id)

2
openedx/core/djangoapps/credit/tests/test_api.py
View File

@@ -557,7 +557,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
self.assertFalse(api.is_user_eligible_for_credit("bob", self.course_key))
# Satisfy the other requirement
with self.assertNumQueries(20):
with self.assertNumQueries(21):
api.set_credit_requirement_status(
"bob",
self.course_key,