launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

John A Meinel has proposed merging lp:~jameinel/maas/nodes_perf into lp:maas.

Commit message:
Prefetch the mac_addresses for nodes that get displayed on the nodes/ page and the tags/ page.

The URLs we generate are linked against the mac address, so rather than making N queries, do one large prefetch request. Note that we have to change the syntax in the template, because prefetch will only affect an 'all()' query. However, macaddress_set.reverse can easily be mapped into macaddress_set.all reversed. (Does it even matter if we reverse the mac_addresses, are they stably sorted in the DB?)


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jameinel/maas/nodes_perf/+merge/128878

See commit message.
-- 
https://code.launchpad.net/~jameinel/maas/nodes_perf/+merge/128878
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jameinel/maas/nodes_perf into lp:maas.

=== modified file 'src/maasserver/templates/maasserver/nodes_listing.html'
--- src/maasserver/templates/maasserver/nodes_listing.html	2012-10-02 08:12:10 +0000
+++ src/maasserver/templates/maasserver/nodes_listing.html	2012-10-10 08:16:19 +0000
@@ -10,7 +10,7 @@
       <tr class="node {% cycle 'even' 'odd' %}">
         <td>
           <a href="{% url 'node-view' node.system_id %}">
-            {% for macaddress in node.macaddress_set.reverse %}
+            {% for macaddress in node.macaddress_set.all reversed %}
               {{ macaddress }}{% if not forloop.last %},{% endif %}
             {% endfor %}
           </a>

=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py	2012-10-08 17:00:30 +0000
+++ src/maasserver/views/nodes.py	2012-10-10 08:16:19 +0000
@@ -115,6 +115,9 @@
         nodes = Node.objects.get_nodes(
             user=self.request.user,
             perm=NODE_PERMISSION.VIEW).order_by('-created')
+        # The nodes are linked via their mac addresses, so we prefetch all of
+        # them, to do 1 large query rather than N small queries.
+        nodes = nodes.prefetch_related('macaddress_set')
         if self.query:
             try:
                 return constrain_nodes(nodes, _parse_constraints(self.query))

=== modified file 'src/maasserver/views/tags.py'
--- src/maasserver/views/tags.py	2012-10-01 08:22:46 +0000
+++ src/maasserver/views/tags.py	2012-10-10 08:16:19 +0000
@@ -36,6 +36,6 @@
 
     def get_context_data(self, **kwargs):
         context = super(TagView, self).get_context_data(**kwargs)
-        context['node_list'] = Tag.objects.get_nodes(self.kwargs['name'],
-                                                     self.request.user)
+        nodes = Tag.objects.get_nodes(self.kwargs['name'], self.request.user)
+        context['node_list'] = nodes.prefetch_related('macaddress_set')
         return context