fix #1806: N+1 query issue on dashboard index page

dashboard/models.py

@@ -6,6 +6,7 @@
 from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
 from django.contrib.contenttypes.models import ContentType
 from django.db import connections, models
+from django.db.models.functions import JSONObject
 from django.utils.translation import gettext_lazy as _
 from django_hosts.resolvers import reverse
 
@@ -33,6 +34,35 @@ def __str__(self):
         return self.name
 
 
+class MetricQuerySet(models.QuerySet):
+    def with_latest(self):
+        """
+        Annotate the queryset with a `latest` JSON object containing two keys:
+            * `measurement` (int): the value of the most recent datum for that metric
+            * `timestamp` (str): the timestamp of the most recent datum
+        """
+        data = Datum.objects.filter(
+            content_type=self.model.content_type(),
+            object_id=models.OuterRef("pk"),
+        )
+        jsonobj = JSONObject(
+            measurement=models.F("measurement"),
+            timestamp=models.F("timestamp"),
+        )
+        latest = models.Subquery(data.values_list(jsonobj).order_by("-timestamp")[:1])
+
+        return self.annotate(latest=latest)
+
+    def for_dashboard(self):
+        """
+        Return a queryset optimized for being displayed on the dashboard index
+        page.
+        """
+        return (
+            self.filter(show_on_dashboard=True).select_related("category").with_latest()
+        )
+
+
 class Metric(models.Model):
     name = models.CharField(max_length=300)
     slug = models.SlugField()
@@ -49,6 +79,8 @@ class Metric(models.Model):
     unit = models.CharField(max_length=100)
     unit_plural = models.CharField(max_length=100)
 
+    objects = MetricQuerySet.as_manager()
+
     class Meta:
         abstract = True
 
@@ -105,7 +137,6 @@ def _gather_data_periodic(self, since, period):
         scale but works for now.
         """
         OFFSET = "2 hours"  # HACK!
-        ctid = ContentType.objects.get_for_model(self).id
 
         c = connections["default"].cursor()
         c.execute(
@@ -117,10 +148,14 @@ def _gather_data_periodic(self, since, period):
                        AND object_id = %s
                        AND timestamp >= %s
                      GROUP BY 1;""",
-            [period, OFFSET, ctid, self.id, since],
+            [period, OFFSET, self.content_type().id, self.id, since],
         )
         return [(calendar.timegm(t.timetuple()), float(m)) for (t, m) in c.fetchall()]
 
+    @classmethod
+    def content_type(cls):
+        return ContentType.objects.get_for_model(cls)
+
 
 class TracTicketMetric(Metric):
     query = models.TextField()

dashboard/templates/dashboard/index.html

@@ -3,23 +3,25 @@
 
 {% block content %}
   <div class="dashboard-index">
-    {% for report in data %}
-      {% ifchanged report.metric.category %}
-        {% if report.metric.category %}<h2>{{ report.metric.category }}</h2>{% endif %}
+    {% for metric in data %}
+      {% ifchanged metric.category %}
+        {% if metric.category %}<h2>{{ metric.category }}</h2>{% endif %}
       {% endifchanged %}
-      <div class="metric{% if report.metric.show_sparkline %} has-sparkline{% endif %}">
-        <h3><a href="{{ report.metric.link }}">{{ report.metric.name }}</a></h3>
+      <div class="metric{% if metric.show_sparkline %} has-sparkline{% endif %}">
+        <h3><a href="{{ metric.link }}">{{ metric.name }}</a></h3>
         <div class="value" >
-          <a href="{{ report.metric.get_absolute_url }}">{{ report.latest.measurement }}{% if report.metric.unit == "%" %}%{% endif %}</a>
+          <a href="{{ metric.get_absolute_url }}">{{ metric.latest.measurement }}{% if metric.unit == "%" %}%{% endif %}</a>
           <div class="timestamp">&nbsp;</div>
         </div>
-        {% if report.metric.show_sparkline %}
-          <div class="sparkline" id="spark{{ forloop.counter0 }}" data-path="{% url "metric-list" host "dashboard" %}" data-metric="{{ report.metric.slug }}"></div>
+        {% if metric.show_sparkline %}
+          <div class="sparkline" id="spark{{ forloop.counter0 }}" data-path="{% url "metric-list" host "dashboard" %}" data-metric="{{ metric.slug }}"></div>
         {% endif %}
       </div>
     {% endfor %}
-    <p class="updated">
-      {% blocktranslate with timestamp=data.0.latest.timestamp|timesince %}Updated {{ timestamp }} ago.{% endblocktranslate %}
-    </p>
+    {% if last_updated %}
+      <p class="updated">
+        {% blocktranslate with timestamp=last_updated|timesince %}Updated {{ timestamp }} ago.{% endblocktranslate %}
+      </p>
+    {% endif %}
   </div>
 {% endblock %}

dashboard/tests.py

@@ -16,6 +16,7 @@
 from .models import (
     METRIC_PERIOD_DAILY,
     METRIC_PERIOD_WEEKLY,
+    Category,
     GithubItemCountMetric,
     GitHubSearchCountMetric,
     Metric,
@@ -70,6 +71,53 @@ def test_metric_json(self):
         self.assertEqual(response.status_code, 200)
 
 
+class AbstractMetricTestCase(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        category = Category.objects.create(name="test category")
+        cls.metrics = [
+            TracTicketMetric.objects.create(
+                slug=f"test{i}", name=f"Test metric {i}", category=category
+            )
+            for i in range(3)
+        ]
+        for metric, measurement, year in [
+            (0, 1, 2020),
+            (0, 2, 2021),
+            (0, 3, 2022),
+            (1, 4, 2023),
+        ]:
+            cls.metrics[metric].data.create(
+                measurement=measurement,
+                timestamp=datetime.datetime(year, 1, 1),
+            )
+
+    def test_with_latest(self):
+        self.assertQuerySetEqual(
+            TracTicketMetric.objects.with_latest().order_by("name"),
+            [
+                (
+                    "Test metric 0",
+                    {"measurement": 3, "timestamp": "2022-01-01T00:00:00-06:00"},
+                ),
+                (
+                    "Test metric 1",
+                    {"measurement": 4, "timestamp": "2023-01-01T00:00:00-06:00"},
+                ),
+                ("Test metric 2", None),
+            ],
+            transform=attrgetter("name", "latest"),
+        )
+
+    def test_for_dashboard(self):
+        with self.assertNumQueries(1):
+            for row in TracTicketMetric.objects.for_dashboard():
+                # weird asserts to make sure the related objects are evaluated
+                self.assertTrue(row.category.name)
+                self.assertTrue(row.latest is None or row.latest["timestamp"])
+                self.assertTrue(row.latest is None or row.latest["measurement"])
+
+
 class MetricMixin:
     def test_str(self):
         self.assertEqual(str(self.instance), self.instance.name)

dashboard/views.py

@@ -17,17 +17,23 @@ def index(request):
 
     data = cache.get(key, version=generation)
     if data is None:
-        metrics = []
-        for MC in Metric.__subclasses__():
-            metrics.extend(MC.objects.filter(show_on_dashboard=True))
-        metrics = sorted(metrics, key=operator.attrgetter("display_position"))
-
-        data = []
-        for metric in metrics:
-            data.append({"metric": metric, "latest": metric.data.latest()})
+        data = [m for MC in Metric.__subclasses__() for m in MC.objects.for_dashboard()]
+        data.sort(key=operator.attrgetter("display_position"))
         cache.set(key, data, 60 * 60, version=generation)
 
-    return render(request, "dashboard/index.html", {"data": data})
+    # Due to the way `with_latest()` is implemented, the timestamps we get back
+    # are actually strings (because JSON) so they need converting to proper
+    # datetime objects first.
+    timestamps = [
+        datetime.datetime.fromisoformat(m.latest["timestamp"])
+        for m in data
+        if m.latest is not None
+    ]
+    last_updated = max(timestamps, default=None)
+
+    return render(
+        request, "dashboard/index.html", {"data": data, "last_updated": last_updated}
+    )
 
 
 def metric_detail(request, metric_slug):