Sort in python and add an index to improve StackedConfigModel.current query performance

openedx/core/djangoapps/config_model_utils/models.py

@@ -43,6 +43,11 @@ class StackedConfigurationModel(ConfigurationModel):
     """
     class Meta(object):
         abstract = True
+        indexes = [
+            # This index optimizes the .object.current_set() query
+            # by preventing a filesort
+            models.Index(fields=['site', 'org', 'course'])
+        ]
 
     KEY_FIELDS = ('site', 'org', 'course')
     STACKABLE_FIELDS = ('enabled',)
@@ -134,23 +139,18 @@ class StackedConfigurationModel(ConfigurationModel):
             site_override_q |
             org_override_q |
             course_override_q
-        ).order_by(
-            # Sort nulls first, and in reverse specificity order
-            # so that the overrides are in the order of general to specific.
-            #
-            # Site | Org  | Course
-            # --------------------
-            # Null | Null | Null
-            # site | Null | Null
-            # Null | org  | Null
-            # Null | Null | Course
-            F('course').desc(nulls_first=True),
-            F('org').desc(nulls_first=True),
-            F('site').desc(nulls_first=True),
         )
 
         provenances = defaultdict(lambda: Provenance.default)
-        for override in overrides:
+        # We are sorting in python to avoid doing a filesort in the database for
+        # what will only be 4 rows at maximum
+        for override in sorted(
+            overrides,
+            # This particular sort order sorts None before not-None (because False < True)
+            # It sorts global first (because all entries are None), then site entries
+            # (because course_id and org are None), then org and course (by the same logic)
+            key=lambda o: (o.course_id is not None, o.org is not None, o.site_id is not None)
+        ):
             for field in stackable_fields:
                 value = field.value_from_object(override)
                 if value != field_defaults[field.name]:

openedx/features/content_type_gating/migrations/0006_auto_20190308_1447.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.20 on 2019-03-08 14:47
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('content_type_gating', '0005_auto_20190306_1547'),
+    ]
+
+    operations = [
+        migrations.AddIndex(
+            model_name='contenttypegatingconfig',
+            index=models.Index(fields=['site', 'org', 'course'], name='content_typ_site_id_e91576_idx'),
+        ),
+    ]

openedx/features/course_duration_limits/migrations/0006_auto_20190308_1447.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.20 on 2019-03-08 14:47
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('course_duration_limits', '0005_auto_20190306_1546'),
+    ]
+
+    operations = [
+        migrations.AddIndex(
+            model_name='coursedurationlimitconfig',
+            index=models.Index(fields=['site', 'org', 'course'], name='course_dura_site_id_424016_idx'),
+        ),
+    ]

openedx/features/course_duration_limits/tests/test_models.py

@@ -157,10 +157,10 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase):
 
     @ddt.data(
         # Generate all combinations of setting each configuration level to True/False/None
-        *itertools.product(*[(True, False, None)] * 4)
+        *itertools.product(*([(True, False, None)] * 4 + [(True, False)]))
     )
     @ddt.unpack
-    def test_config_overrides(self, global_setting, site_setting, org_setting, course_setting):
+    def test_config_overrides(self, global_setting, site_setting, org_setting, course_setting, reverse_order):
         """
         Test that the stacked configuration overrides happen in the correct order and priority.
 
@@ -189,10 +189,16 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase):
         test_course = CourseOverviewFactory.create(org='test-org')
         test_site_cfg = SiteConfigurationFactory.create(values={'course_org_filter': test_course.org})
 
-        CourseDurationLimitConfig.objects.create(enabled=global_setting)
-        CourseDurationLimitConfig.objects.create(course=test_course, enabled=course_setting)
-        CourseDurationLimitConfig.objects.create(org=test_course.org, enabled=org_setting)
-        CourseDurationLimitConfig.objects.create(site=test_site_cfg.site, enabled=site_setting)
+        if reverse_order:
+            CourseDurationLimitConfig.objects.create(site=test_site_cfg.site, enabled=site_setting)
+            CourseDurationLimitConfig.objects.create(org=test_course.org, enabled=org_setting)
+            CourseDurationLimitConfig.objects.create(course=test_course, enabled=course_setting)
+            CourseDurationLimitConfig.objects.create(enabled=global_setting)
+        else:
+            CourseDurationLimitConfig.objects.create(enabled=global_setting)
+            CourseDurationLimitConfig.objects.create(course=test_course, enabled=course_setting)
+            CourseDurationLimitConfig.objects.create(org=test_course.org, enabled=org_setting)
+            CourseDurationLimitConfig.objects.create(site=test_site_cfg.site, enabled=site_setting)
 
         expected_global_setting = self._resolve_settings([global_setting])
         expected_site_setting = self._resolve_settings([global_setting, site_setting])