Commit 70ea8c41 by Clinton Blackburn Committed by GitHub

Merge pull request #316 from edx/clintonb/performance-updates

Query Performance Improvements
parents da1b6185 5646091d
...@@ -141,9 +141,10 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixi ...@@ -141,9 +141,10 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixi
CourseRunFactory(enrollment_end=enrollment_end, course__title='ABC Test Course 2') CourseRunFactory(enrollment_end=enrollment_end, course__title='ABC Test Course 2')
CourseRunFactory(enrollment_end=enrollment_end, course=self.course) CourseRunFactory(enrollment_end=enrollment_end, course=self.course)
response = self.client.get(url) with self.assertNumQueries(40):
self.assertEqual(response.status_code, 200) response = self.client.get(url)
self.assertListEqual(response.data['results'], self.serialize_catalog_course(courses, many=True)) self.assertEqual(response.status_code, 200)
self.assertListEqual(response.data['results'], self.serialize_catalog_course(courses, many=True))
def test_contains(self): def test_contains(self):
""" Verify the endpoint returns a filtered list of courses contained in the catalog. """ """ Verify the endpoint returns a filtered list of courses contained in the catalog. """
...@@ -165,7 +166,9 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixi ...@@ -165,7 +166,9 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixi
SeatFactory(type='credit', course_run=self.course_run, credit_provider='Hogwarts', credit_hours=4) SeatFactory(type='credit', course_run=self.course_run, credit_provider='Hogwarts', credit_hours=4)
url = reverse('api:v1:catalog-csv', kwargs={'id': self.catalog.id}) url = reverse('api:v1:catalog-csv', kwargs={'id': self.catalog.id})
response = self.client.get(url)
with self.assertNumQueries(24):
response = self.client.get(url)
course_run = self.serialize_catalog_flat_course_run(self.course_run) course_run = self.serialize_catalog_flat_course_run(self.course_run)
expected = ','.join([ expected = ','.join([
......
...@@ -35,7 +35,9 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase): ...@@ -35,7 +35,9 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
""" Verify the endpoint returns the details for a single course. """ """ Verify the endpoint returns the details for a single course. """
url = reverse('api:v1:course_run-detail', kwargs={'key': self.course_run.key}) url = reverse('api:v1:course_run-detail', kwargs={'key': self.course_run.key})
response = self.client.get(url) with self.assertNumQueries(9):
response = self.client.get(url)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertEqual(response.data, self.serialize_course_run(self.course_run)) self.assertEqual(response.data, self.serialize_course_run(self.course_run))
...@@ -43,7 +45,9 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase): ...@@ -43,7 +45,9 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
""" Verify the endpoint returns a list of all catalogs. """ """ Verify the endpoint returns a list of all catalogs. """
url = reverse('api:v1:course_run-list') url = reverse('api:v1:course_run-list')
response = self.client.get(url) with self.assertNumQueries(11):
response = self.client.get(url)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertListEqual( self.assertListEqual(
response.data['results'], response.data['results'],
...@@ -57,7 +61,9 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase): ...@@ -57,7 +61,9 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
query = 'title:Some random title' query = 'title:Some random title'
url = '{root}?q={query}'.format(root=reverse('api:v1:course_run-list'), query=query) url = '{root}?q={query}'.format(root=reverse('api:v1:course_run-list'), query=query)
response = self.client.get(url) with self.assertNumQueries(37):
response = self.client.get(url)
actual_sorted = sorted(response.data['results'], key=lambda course_run: course_run['key']) actual_sorted = sorted(response.data['results'], key=lambda course_run: course_run['key'])
expected_sorted = sorted(self.serialize_course_run(course_runs, many=True), expected_sorted = sorted(self.serialize_course_run(course_runs, many=True),
key=lambda course_run: course_run['key']) key=lambda course_run: course_run['key'])
......
...@@ -19,20 +19,22 @@ class CourseViewSetTests(SerializationMixin, APITestCase): ...@@ -19,20 +19,22 @@ class CourseViewSetTests(SerializationMixin, APITestCase):
""" Verify the endpoint returns the details for a single course. """ """ Verify the endpoint returns the details for a single course. """
url = reverse('api:v1:course-detail', kwargs={'key': self.course.key}) url = reverse('api:v1:course-detail', kwargs={'key': self.course.key})
response = self.client.get(url) with self.assertNumQueries(19):
self.assertEqual(response.status_code, 200) response = self.client.get(url)
self.assertEqual(response.data, self.serialize_course(self.course)) self.assertEqual(response.status_code, 200)
self.assertEqual(response.data, self.serialize_course(self.course))
def test_list(self): def test_list(self):
""" Verify the endpoint returns a list of all courses. """ """ Verify the endpoint returns a list of all courses. """
url = reverse('api:v1:course-list') url = reverse('api:v1:course-list')
response = self.client.get(url) with self.assertNumQueries(25):
self.assertEqual(response.status_code, 200) response = self.client.get(url)
self.assertListEqual( self.assertEqual(response.status_code, 200)
response.data['results'], self.assertListEqual(
self.serialize_course(Course.objects.all().order_by(Lower('key')), many=True) response.data['results'],
) self.serialize_course(Course.objects.all().order_by(Lower('key')), many=True)
)
def test_list_query(self): def test_list_query(self):
""" Verify the endpoint returns a filtered list of courses """ """ Verify the endpoint returns a filtered list of courses """
...@@ -42,8 +44,9 @@ class CourseViewSetTests(SerializationMixin, APITestCase): ...@@ -42,8 +44,9 @@ class CourseViewSetTests(SerializationMixin, APITestCase):
query = 'title:' + title query = 'title:' + title
url = '{root}?q={query}'.format(root=reverse('api:v1:course-list'), query=query) url = '{root}?q={query}'.format(root=reverse('api:v1:course-list'), query=query)
response = self.client.get(url) with self.assertNumQueries(62):
self.assertListEqual(response.data['results'], self.serialize_course(courses, many=True)) response = self.client.get(url)
self.assertListEqual(response.data['results'], self.serialize_course(courses, many=True))
def test_list_key_filter(self): def test_list_key_filter(self):
""" Verify the endpoint returns a list of courses filtered by the specified keys. """ """ Verify the endpoint returns a list of courses filtered by the specified keys. """
...@@ -52,5 +55,6 @@ class CourseViewSetTests(SerializationMixin, APITestCase): ...@@ -52,5 +55,6 @@ class CourseViewSetTests(SerializationMixin, APITestCase):
keys = ','.join([course.key for course in courses]) keys = ','.join([course.key for course in courses])
url = '{root}?keys={keys}'.format(root=reverse('api:v1:course-list'), keys=keys) url = '{root}?keys={keys}'.format(root=reverse('api:v1:course-list'), keys=keys)
response = self.client.get(url) with self.assertNumQueries(38):
self.assertListEqual(response.data['results'], self.serialize_course(courses, many=True)) response = self.client.get(url)
self.assertListEqual(response.data['results'], self.serialize_course(courses, many=True))
...@@ -36,6 +36,45 @@ from course_discovery.apps.course_metadata.models import Course, CourseRun, Part ...@@ -36,6 +36,45 @@ from course_discovery.apps.course_metadata.models import Course, CourseRun, Part
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
User = get_user_model() User = get_user_model()
PREFETCH_FIELDS = {
'course_run': (
'course__partner', 'course__level_type', 'course__programs', 'course__programs__type',
'course__programs__partner', 'seats', 'transcript_languages', 'seats__currency', 'staff',
'staff__position', 'staff__position__organization', 'language',
),
'course': (
'level_type', 'video', 'programs', 'course_runs', 'subjects', 'prerequisites', 'expected_learning_items',
'authoring_organizations', 'authoring_organizations__tags', 'authoring_organizations__partner',
'sponsoring_organizations', 'sponsoring_organizations__tags', 'sponsoring_organizations__partner',
),
}
def prefetch_related_objects_for_courses(queryset):
"""
Pre-fetches the related objects that will be serialized with a `Course`.
Pre-fetching allows us to consolidate our database queries rather than run
thousands of queries as we serialize the data. For details, see the links below:
- https://docs.djangoproject.com/en/1.10/ref/models/querysets/#select-related
- https://docs.djangoproject.com/en/1.10/ref/models/querysets/#prefetch-related
Args:
queryset (QuerySet): original query
Returns:
QuerySet
"""
# Prefetch the data for the related course runs
course_run_prefetch_fields = PREFETCH_FIELDS['course_run']
course_run_prefetch_fields = ['course_runs__' + field for field in course_run_prefetch_fields]
queryset = queryset.prefetch_related(*course_run_prefetch_fields)
queryset = queryset.select_related('level_type', 'video')
queryset = queryset.prefetch_related(*PREFETCH_FIELDS['course'])
return queryset
# pylint: disable=no-member # pylint: disable=no-member
class CatalogViewSet(viewsets.ModelViewSet): class CatalogViewSet(viewsets.ModelViewSet):
...@@ -110,6 +149,7 @@ class CatalogViewSet(viewsets.ModelViewSet): ...@@ -110,6 +149,7 @@ class CatalogViewSet(viewsets.ModelViewSet):
catalog = self.get_object() catalog = self.get_object()
queryset = catalog.courses().active() queryset = catalog.courses().active()
queryset = prefetch_related_objects_for_courses(queryset)
page = self.paginate_queryset(queryset) page = self.paginate_queryset(queryset)
serializer = serializers.CourseSerializerExcludingClosedRuns(page, many=True, context={'request': request}) serializer = serializers.CourseSerializerExcludingClosedRuns(page, many=True, context={'request': request})
...@@ -187,6 +227,7 @@ class CourseViewSet(viewsets.ReadOnlyModelViewSet): ...@@ -187,6 +227,7 @@ class CourseViewSet(viewsets.ReadOnlyModelViewSet):
queryset = Course.search(q) queryset = Course.search(q)
else: else:
queryset = super(CourseViewSet, self).get_queryset() queryset = super(CourseViewSet, self).get_queryset()
queryset = prefetch_related_objects_for_courses(queryset)
return queryset.order_by(Lower('key')) return queryset.order_by(Lower('key'))
...@@ -247,7 +288,10 @@ class CourseRunViewSet(viewsets.ReadOnlyModelViewSet): ...@@ -247,7 +288,10 @@ class CourseRunViewSet(viewsets.ReadOnlyModelViewSet):
qs.model = self.queryset.model qs.model = self.queryset.model
return qs return qs
else: else:
return super(CourseRunViewSet, self).get_queryset().filter(course__partner=partner) queryset = super(CourseRunViewSet, self).get_queryset().filter(course__partner=partner)
queryset = queryset.select_related('course', 'language', 'video')
queryset = queryset.prefetch_related(*PREFETCH_FIELDS['course_run'])
return queryset
def list(self, request, *args, **kwargs): def list(self, request, *args, **kwargs):
""" List all courses runs. """ List all courses runs.
......
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('course_metadata', '0025_remove_program_category'),
]
operations = [
migrations.AlterField(
model_name='courserun',
name='end',
field=models.DateTimeField(null=True, db_index=True, blank=True),
),
migrations.AlterField(
model_name='courserun',
name='enrollment_end',
field=models.DateTimeField(null=True, db_index=True, blank=True),
),
migrations.AlterField(
model_name='historicalcourserun',
name='end',
field=models.DateTimeField(null=True, db_index=True, blank=True),
),
migrations.AlterField(
model_name='historicalcourserun',
name='enrollment_end',
field=models.DateTimeField(null=True, db_index=True, blank=True),
),
]
...@@ -346,9 +346,9 @@ class CourseRun(TimeStampedModel): ...@@ -346,9 +346,9 @@ class CourseRun(TimeStampedModel):
help_text=_( help_text=_(
"Title specific for this run of a course. Leave this value blank to default to the parent course's title.")) "Title specific for this run of a course. Leave this value blank to default to the parent course's title."))
start = models.DateTimeField(null=True, blank=True) start = models.DateTimeField(null=True, blank=True)
end = models.DateTimeField(null=True, blank=True) end = models.DateTimeField(null=True, blank=True, db_index=True)
enrollment_start = models.DateTimeField(null=True, blank=True) enrollment_start = models.DateTimeField(null=True, blank=True)
enrollment_end = models.DateTimeField(null=True, blank=True) enrollment_end = models.DateTimeField(null=True, blank=True, db_index=True)
announcement = models.DateTimeField(null=True, blank=True) announcement = models.DateTimeField(null=True, blank=True)
short_description_override = models.CharField( short_description_override = models.CharField(
max_length=255, default=None, null=True, blank=True, max_length=255, default=None, null=True, blank=True,
......
...@@ -373,3 +373,34 @@ TAGGIT_CASE_INSENSITIVE = True ...@@ -373,3 +373,34 @@ TAGGIT_CASE_INSENSITIVE = True
# django-solo configuration (https://github.com/lazybird/django-solo#settings) # django-solo configuration (https://github.com/lazybird/django-solo#settings)
SOLO_CACHE = 'default' SOLO_CACHE = 'default'
SOLO_CACHE_TIMEOUT = 3600 SOLO_CACHE_TIMEOUT = 3600
# Django Debug Toolbar settings
# http://django-debug-toolbar.readthedocs.org/en/latest/installation.html
if os.environ.get('ENABLE_DJANGO_TOOLBAR', False):
INSTALLED_APPS += [
'debug_toolbar',
'elastic_panel',
]
MIDDLEWARE_CLASSES += (
'debug_toolbar.middleware.DebugToolbarMiddleware',
)
DEBUG_TOOLBAR_PATCH_SETTINGS = False
DEBUG_TOOLBAR_PANELS = [
'debug_toolbar.panels.versions.VersionsPanel',
'debug_toolbar.panels.timer.TimerPanel',
'debug_toolbar.panels.settings.SettingsPanel',
'debug_toolbar.panels.headers.HeadersPanel',
'debug_toolbar.panels.request.RequestPanel',
'debug_toolbar.panels.sql.SQLPanel',
'debug_toolbar.panels.staticfiles.StaticFilesPanel',
'debug_toolbar.panels.templates.TemplatesPanel',
'debug_toolbar.panels.cache.CachePanel',
'debug_toolbar.panels.signals.SignalsPanel',
'debug_toolbar.panels.logging.LoggingPanel',
'debug_toolbar.panels.redirects.RedirectsPanel',
'elastic_panel.panel.ElasticDebugPanel'
]
from course_discovery.settings.production import * from course_discovery.settings.production import *
from course_discovery.settings.shared.debug_toolbar import *
DEBUG = True DEBUG = True
...@@ -8,8 +7,9 @@ LOGGING['handlers']['local'] = { ...@@ -8,8 +7,9 @@ LOGGING['handlers']['local'] = {
'class': 'logging.NullHandler', 'class': 'logging.NullHandler',
} }
# Determine which requests should render Django Debug Toolbar
INTERNAL_IPS = ('127.0.0.1',) INTERNAL_IPS = ('127.0.0.1',)
# END TOOLBAR CONFIGURATION
HAYSTACK_CONNECTIONS = { HAYSTACK_CONNECTIONS = {
......
from course_discovery.settings.base import * from course_discovery.settings.base import *
from course_discovery.settings.shared.debug_toolbar import *
DEBUG = True DEBUG = True
...@@ -31,8 +30,8 @@ DATABASES = { ...@@ -31,8 +30,8 @@ DATABASES = {
EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'
# END EMAIL CONFIGURATION # END EMAIL CONFIGURATION
# Determine which requests should render Django Debug Toolbar
INTERNAL_IPS = ('127.0.0.1',) INTERNAL_IPS = ('127.0.0.1',)
# END TOOLBAR CONFIGURATION
# AUTHENTICATION # AUTHENTICATION
# Set these to the correct values for your OAuth2/OpenID Connect provider (e.g., devstack) # Set these to the correct values for your OAuth2/OpenID Connect provider (e.g., devstack)
......
"""
Django Debug Toolbar settings.
http://django-debug-toolbar.readthedocs.org/en/latest/installation.html
"""
from course_discovery.settings.base import *
if os.environ.get('ENABLE_DJANGO_TOOLBAR', False):
INSTALLED_APPS += [
'debug_toolbar',
'elastic_panel',
]
MIDDLEWARE_CLASSES += (
'debug_toolbar.middleware.DebugToolbarMiddleware',
)
DEBUG_TOOLBAR_PATCH_SETTINGS = False
DEBUG_TOOLBAR_PANELS = [
'debug_toolbar.panels.versions.VersionsPanel',
'debug_toolbar.panels.timer.TimerPanel',
'debug_toolbar.panels.settings.SettingsPanel',
'debug_toolbar.panels.headers.HeadersPanel',
'debug_toolbar.panels.request.RequestPanel',
'debug_toolbar.panels.sql.SQLPanel',
'debug_toolbar.panels.staticfiles.StaticFilesPanel',
'debug_toolbar.panels.templates.TemplatesPanel',
'debug_toolbar.panels.cache.CachePanel',
'debug_toolbar.panels.signals.SignalsPanel',
'debug_toolbar.panels.logging.LoggingPanel',
'debug_toolbar.panels.redirects.RedirectsPanel',
'elastic_panel.panel.ElasticDebugPanel'
]
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