Commit 274e6de2 by utkjad

PLAT-734 Fix slow transaction due to multiple calls to unpickling

parent 72ab1fbc
......@@ -6,13 +6,12 @@ from io import BytesIO
from pytz import UTC
from PIL import Image
import json
from mock import patch
from django.conf import settings
from contentstore.tests.utils import CourseTestCase
from contentstore.views import assets
from contentstore.utils import reverse_course_url
from xmodule.assetstore.assetmgr import AssetMetadataFoundTemporary
from xmodule.assetstore import AssetMetadata
from xmodule.contentstore.content import StaticContent
from xmodule.contentstore.django import contentstore
......@@ -331,23 +330,13 @@ class DownloadTestCase(AssetsTestCase):
resp = self.client.get(url, HTTP_ACCEPT='text/html')
self.assertEquals(resp.status_code, 404)
def test_metadata_found_in_modulestore(self):
# Insert asset metadata into the modulestore (with no accompanying asset).
asset_key = self.course.id.make_asset_key(AssetMetadata.GENERAL_ASSET_TYPE, 'pic1.jpg')
asset_md = AssetMetadata(asset_key, {
'internal_name': 'EKMND332DDBK',
'basename': 'pix/archive',
'locked': False,
'curr_version': '14',
'prev_version': '13'
})
modulestore().save_asset_metadata(asset_md, 15)
# Get the asset metadata and have it be found in the modulestore.
# Currently, no asset metadata should be found in the modulestore. The code is not yet storing it there.
# If asset metadata *is* found there, an exception is raised. This test ensures the exception is indeed raised.
# THIS IS TEMPORARY. Soon, asset metadata *will* be stored in the modulestore.
with self.assertRaises((AssetMetadataFoundTemporary, NameError)):
self.client.get(unicode(asset_key), HTTP_ACCEPT='text/html')
@patch('xmodule.modulestore.mixed.MixedModuleStore.find_asset_metadata')
def test_pickling_calls(self, patched_find_asset_metadata):
""" Tests if assets are not calling find_asset_metadata
"""
patched_find_asset_metadata.return_value = None
self.client.get(self.uploaded_url, HTTP_ACCEPT='text/html')
self.assertFalse(patched_find_asset_metadata.called)
class AssetToJsonTestCase(AssetsTestCase):
......
......@@ -9,11 +9,12 @@ Handles:
Phase 1: Checks to see if an asset's metadata can be found in the course's modulestore.
If not found, fails over to access the asset from the contentstore.
At first, the asset metadata will never be found, since saving isn't implemented yet.
Note: Hotfix (PLAT-734) No asset calls find_asset_metadata, and directly accesses from contentstore.
"""
from contracts import contract, new_contract
from opaque_keys.edx.keys import AssetKey
from xmodule.modulestore.django import modulestore
from xmodule.contentstore.django import contentstore
......@@ -49,14 +50,11 @@ class AssetManager(object):
@contract(asset_key='AssetKey', throw_on_not_found='bool', as_stream='bool')
def find(asset_key, throw_on_not_found=True, as_stream=False):
"""
Finds a course asset either in the assetstore -or- in the deprecated contentstore.
Finds course asset in the deprecated contentstore.
This method was previously searching for the course asset in the assetstore first, then in the deprecated
contentstore. However, the asset was never found in the assetstore since an asset's metadata is
not yet stored there.(removed calls to modulestore().find_asset_metadata(asset_key))
The assetstore search was removed due to performance issues caused by each call unpickling the pickled and
compressed course structure from the structure cache.
"""
content_md = modulestore().find_asset_metadata(asset_key)
# If found, raise an exception.
if content_md:
# For now, no asset metadata should be found in the modulestore.
raise AssetMetadataFoundTemporary()
else:
# If not found, load the asset via the contentstore.
return contentstore().find(asset_key, throw_on_not_found, as_stream)
return contentstore().find(asset_key, throw_on_not_found, as_stream)
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