launchpad-reviewers team mailing list archive

[James Westby <jw+debian@xxxxxxxxxxxxxxx>]

James Westby has proposed merging lp:~james-w/python-oops-tools/recent-oopses into lp:python-oops-tools.

Commit message:
Modify the recent oopses query to perform better with lots of oopses.

When there is a report with many prefixes, and the most recent oopses are
not for any of those prefixes postgres will pick a query plan that performs
poorly (not using the most selective index, assuming that the prefix ids
are distributed fairly uniformly through the oopses.)

We instead trick postgres in to using the more selective index using a window
function. Thanks to stub for the query.

We do a second query to load all the infestations for these oopses, to prevent
the ORM from doing 50 queries as we iterate over the list of oopses.

We also drop the pagination because it is not as simple to implement with the
raw query. It could be re-instated if it was really needed, but usually
I find looking at the most recent 50 oopses is sufficient.

Lastly, I re-ordered the columns in the oops listing, to put the oops
first as that is usually what you want to click on. I also dropped the linking
of the URL, as the escaping meant that it never worked, and the URL is often
not something you can simply click on either. The fact that the URL was first
in the table and a link meant that I often clicked on that rather than the
oops, wasting time.

Requested reviews:
  python-oops-tools reviewers (oops-tools-reviewers)

For more details, see:
https://code.launchpad.net/~james-w/python-oops-tools/recent-oopses/+merge/201639

Hi,

This is a few changes to hopefully improve the performance of some recent
oops views. The commit message has all the details.

Thanks,

James
-- 
https://code.launchpad.net/~james-w/python-oops-tools/recent-oopses/+merge/201639
Your team python-oops-tools reviewers is requested to review the proposed merge of lp:~james-w/python-oops-tools/recent-oopses into lp:python-oops-tools.

=== modified file 'src/oopstools/oops/templates/report.html'
--- src/oopstools/oops/templates/report.html	2012-11-29 21:33:52 +0000
+++ src/oopstools/oops/templates/report.html	2014-01-14 18:14:14 +0000
@@ -17,37 +17,22 @@
     <table>
         <th><tr>
             <td>Date/Time</td>
+            <td>Oops</td>
+            <td>Exception Type</td>
+            <td>Duration (ms)</td>
             <td>HTTP Method</td>
             <td>URL</td>
-            <td>Duration (ms)</td>
-            <td>Exception Type</td>
-            <td>Oops</td>
         </tr></th>
-        {% for oops in recent.object_list %}
+        {% for oops, exception_type in recent %}
         <tr>
             <td>{{oops.date|date:"Y-m-d H:i:s"}}</td>
+            <td><a href="{% url oopstools.oops.views.index %}?oopsid={{oops.oopsid}}">{{oops.oopsid}}</a></td>
+            <td>{{exception_type}}</td>
+            <td>{{oops.total_time}}</td>
             <td>{{oops.http_method}}</td>
-            <td>{% if oops.url %}<a href="{{oops.url}}">{{oops.url}}</a>{% endif %}</td>
-            <td>{{oops.total_time}}</td>
-            <td>{{oops.exception_type}}</td>
-            <td><a href="{% url oopstools.oops.views.index %}?oopsid={{oops.oopsid}}">{{oops.oopsid}}</a></td>
+            <td>{% if oops.url %}{{oops.url}}{% endif %}</td>
         </tr>
         {% endfor %}
     </table>
-    <div class="pagination">
-        <span class="step-links">
-            {% if recent.has_previous %}
-            <a href="?page={{ recent.previous_page_number }}">newer</a>
-            {% endif %}
-
-            <span class="current">
-                Page {{ recent.number }} of {{ recent.paginator.num_pages }}.
-            </span>
-
-            {% if recent.has_next %}
-            <a href="?page={{ recent.next_page_number }}">older</a>
-            {% endif %}
-        </span>
-    </div>
   </body>
 </html>

=== modified file 'src/oopstools/oops/views.py'
--- src/oopstools/oops/views.py	2012-12-06 01:07:17 +0000
+++ src/oopstools/oops/views.py	2014-01-14 18:14:14 +0000
@@ -18,13 +18,12 @@
 from datetime import datetime, timedelta
 
 from django.conf import settings
-from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger
 from django.http import Http404, HttpResponseRedirect
 from django.shortcuts import render_to_response
 
 from oopstools.oops import dbsummaries
 from oopstools.oops.helpers import parsedate
-from oopstools.oops.models import Oops, Report
+from oopstools.oops.models import Infestation, Oops, Report
 from oopstools.oops.prefixloader import PrefixLoader
 from oopstools.oops.oopsstore import OopsStore, OopsNotFound
 
@@ -123,6 +122,25 @@
     return page
 
 
+def get_recent_oops_info(report):
+    prefix_ids = list(report.prefixes.values_list('pk', flat=True))
+    recent_oopses = list(Oops.objects.raw("""
+        SELECT whatever.*, pos FROM (
+            select *, rank() OVER w AS pos
+            FROM oops_oops
+            WHERE "oops_oops"."prefix_id" = ANY(%s)
+            WINDOW w AS (PARTITION BY prefix_id ORDER BY date DESC)
+        ) AS whatever
+        WHERE pos <= 50
+        ORDER BY date DESC
+        LIMIT 50;
+    """, [prefix_ids]))
+    infestation_pks = set([oops.oopsinfestation_id for oops in recent_oopses])
+    infestations = Infestation.objects.filter(id__in=infestation_pks).all()
+    exception_types = dict([(i.id, i.exception_type) for i in infestations])
+    return [(oops, exception_types[oops.oopsinfestation_id]) for oops in recent_oopses]
+
+
 def report(request, report_name):
     try:
         r = Report.objects.get(name=report_name, active=True)
@@ -135,17 +153,12 @@
         dates = []
         for day in range(1, 11):
             dates.append(now - timedelta(day))
-        prefix_ids = list(r.prefixes.values_list('pk', flat=True))
-        oopses = Oops.objects.filter(
-                prefix__in=prefix_ids).select_related(
-                    'oopsinfestation').order_by('-date')
-        paginator = Paginator(oopses, 50)
-        recent_oopses = get_page_from_query_args(paginator, request.GET)
+        recent = get_recent_oops_info(report)
         data = {
             'report': r,
             'dates': dates,
             'SUMMARY_URI': settings.SUMMARY_URI,
-            'recent': recent_oopses,
+            'recent': recent,
             }
         return render_to_response("report.html", dictionary=data)