Merge lp:~blake-rouse/maas/fix-interfaces-read into lp:~maas-committers/maas/trunk

[Blake Rouse]

Prefetch interface data for GET interfaces.

This make the number of queries for interfaces read to always be the same. Doesn't matter the number of interfaces on the machine the number of queries will be the same.

make_complex_interfaces with read was 100+ queries, second time it was 200+ queries, now its around 50 always (no matter the number of interfaces).

[Mike Pontillo (mpontillo)]

Very nice!

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~blake-rouse/maas/fix-interfaces-read into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com/ubuntu xenial-security InRelease [102 kB]
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [102 kB]
Get:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease [102 kB]
Fetched 306 kB in 0s (605 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind avahi-utils bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common isc-dhcp-server libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libnss-wrapper libpq-dev make nodejs-legacy npm postgresql psmisc pxelinux python3-all python3-apt python3-attr python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~rc+dfsg-1ubuntu2).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
psmisc is already the newest version (22.21-2.1build1).
pxelinux is already the newest version (3:6.03+dfsg-11ubuntu1).
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr is already the newest version (0.7.18-1).
python-net...

=== modified file 'src/maasserver/api/interfaces.py'
--- src/maasserver/api/interfaces.py	2017-03-29 13:34:01 +0000
+++ src/maasserver/api/interfaces.py	2017-04-20 22:42:24 +0000
@@ -124,7 +124,26 @@
         """
         node = Node.objects.get_node_or_404(
             system_id, request.user, NODE_PERMISSION.VIEW)
-        return node.interface_set.all()
+        queryset = node.interface_set.all()
+        prefetch_related_fields = [
+            'node',
+            'vlan__primary_rack',
+            'vlan__secondary_rack',
+            'vlan__fabric__vlan_set',
+            'vlan__space',
+            'parents',
+            'ip_addresses__subnet',
+            # Prefetch 3 levels deep, anything more will require extra queries.
+            'children_relationships__child__vlan',
+            ('children_relationships__child__'
+             'children_relationships__child__vlan'),
+            ('children_relationships__child__'
+             'children_relationships__child__'
+             'children_relationships__child__vlan'),
+        ]
+        for prefetch in prefetch_related_fields:
+            queryset = queryset.prefetch_related(prefetch)
+        return queryset
 
     @operation(idempotent=False)
     def create_physical(self, request, system_id):

=== modified file 'src/maasserver/api/nodes.py'
--- src/maasserver/api/nodes.py	2017-04-17 17:25:47 +0000
+++ src/maasserver/api/nodes.py	2017-04-20 22:42:24 +0000
@@ -352,14 +352,20 @@
             'boot_interface__parents',
             ('boot_interface__children_relationships__child__'
              'children_relationships__child'),
+            'interface_set__node',
             'interface_set__vlan__primary_rack',
             'interface_set__vlan__secondary_rack',
             'interface_set__vlan__fabric__vlan_set',
             'interface_set__vlan__space',
             'interface_set__parents',
             'interface_set__ip_addresses__subnet',
-            ('interface_set__children_relationships__child__'
-             'children_relationships__child'),
+            # Prefetch 3 levels deep, anything more will require extra queries.
+            'interface_set__children_relationships__child__vlan',
+            ('interface_set__children_relationships__child__'
+             'children_relationships__child__vlan'),
+            ('interface_set__children_relationships__child__'
+             'children_relationships__child__'
+             'children_relationships__child__vlan'),
             'tags',
         ]
         for prefetch in prefetch_related_fields:

=== modified file 'src/maasserver/api/tests/test_interfaces.py'
--- src/maasserver/api/tests/test_interfaces.py	2017-01-28 00:51:47 +0000
+++ src/maasserver/api/tests/test_interfaces.py	2017-04-20 22:42:24 +0000
@@ -9,6 +9,7 @@
 import random
 
 from django.core.urlresolvers import reverse
+from maasserver import middleware
 from maasserver.enum import (
     INTERFACE_LINK_TYPE,
     INTERFACE_TYPE,
@@ -24,6 +25,7 @@
 from maasserver.testing.factory import factory
 from maasserver.utils.converters import json_load_bytes
 from maasserver.utils.orm import reload_object
+from maastesting.djangotestcase import count_queries
 from testtools.matchers import (
     Contains,
     ContainsDict,
@@ -135,6 +137,42 @@
         self.assertEqual(
             interface.id, json_load_bytes(response.content)[0]['id'])
 
+    def test_read_uses_constant_number_of_queries(self):
+        # Patch middleware so it does not affect query counting.
+        self.patch(
+            middleware.ExternalComponentsMiddleware,
+            '_check_rack_controller_connectivity')
+
+        node = factory.make_Node()
+        bond1, parents1, children1 = make_complex_interface(node)
+        uri = get_interfaces_uri(node)
+
+        num_queries1, response1 = count_queries(self.client.get, uri)
+
+        bond2, parents2, children2 = make_complex_interface(node)
+        num_queries2, response2 = count_queries(self.client.get, uri)
+
+        # Make sure the responses are ok as it's not useful to compare the
+        # number of queries if they are not.
+        parsed_result_1 = json_load_bytes(response1.content)
+        parsed_result_2 = json_load_bytes(response2.content)
+        self.assertEqual(
+            [
+                http.client.OK, http.client.OK,
+                len([bond1] + parents1 + children1),
+                len(
+                    [bond1, bond2] +
+                    parents1 + parents2 +
+                    children1 + children2),
+            ],
+            [
+                response1.status_code,
+                response2.status_code,
+                len(parsed_result_1),
+                len(parsed_result_2),
+            ])
+        self.assertEqual(num_queries1, num_queries2)
+
     def test_create_physical(self):
         self.become_admin()
         for status in (NODE_STATUS.READY, NODE_STATUS.BROKEN):