Merge ~bjornt/maas:bug-1835316-add-event-index into maas:master

[Björn Tillenius]

Add index to the event table to speed up machine listing.

The index speeds up getting the latest event for each node that is
displayed on the machine listing.

The added patch depends on 0187, which is the latest patch on 2.6, since this
patch is to be backported there.

[Alberto Donato (ack)]

LGTM

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bug-1835316-add-event-index lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6265/console
COMMIT: 637213a7fb9aac5cc07b13c0415bffdabf80bc71

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bug-1835316-add-event-index lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6266/console
COMMIT: c3182226de62752bbb9fde4b281ab4b06fb8c3df

review: Needs Fixing

[Blake Rouse (blake-rouse)]

I read your email, still a little confused on the merging of migrations. Want to understand that completely first. Overall the change of adding the index looks good.

review: Needs Information

[Blake Rouse (blake-rouse)]

That means the backport will not be a strait cherry-pick, is it possible to split this over 2 branches or will it not land in master?

[Björn Tillenius (bjornt)]

> That means the backport will not be a strait cherry-pick, is it possible to
> split this over 2 branches or will it not land in master?

No, it's not possible to split it over two commits. Without that merge patch,
Django will complain that there are more than one leaf patch.

On the other hand, backporting of DB patches have never been straight
cherry-picks, since they did re-ordering of db patches that didn't exist in
the backported version. So from that point of view, it's no worse than before.

[Blake Rouse (blake-rouse)]

Yeah I thought that was the case. Looks good!

review: Approve

diff --git a/src/maasserver/migrations/maasserver/0194_machine_listing_event_index.py b/src/maasserver/migrations/maasserver/0194_machine_listing_event_index.py
 new file mode 100644
index 0000000..b82a8ea
--- /dev/null
+++ b/src/maasserver/migrations/maasserver/0194_machine_listing_event_index.py
@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.11 on 2019-08-28 15:07
+from __future__ import unicode_literals
+
+from django.db import (
+    migrations,
+    models,
+)
+
+
+class Migration(migrations.Migration):
+
+    # This depends on the latest (at time of writing) patch in 2.6,
+    # since this patch is to be backported there.
+    dependencies = [
+        ('maasserver', '0192_event_node_no_set_null'),
+    ]
+
+    operations = [
+        migrations.AddIndex(
+            model_name='event',
+            index=models.Index(fields=['node', '-created', '-id'], name='maasserver__node_id_e4a8dd_idx'),
+        ),
+    ]
diff --git a/src/maasserver/migrations/maasserver/0195_merge_20190902_1357.py b/src/maasserver/migrations/maasserver/0195_merge_20190902_1357.py
 new file mode 100644
index 0000000..97af45a
--- /dev/null
+++ b/src/maasserver/migrations/maasserver/0195_merge_20190902_1357.py
@@ -0,0 +1,16 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.11 on 2019-09-02 13:57
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('maasserver', '0193_merge_maasserver_0191_1092'),
+        ('maasserver', '0194_machine_listing_event_index'),
+    ]
+
+    operations = [
+    ]
diff --git a/src/maasserver/models/event.py b/src/maasserver/models/event.py
index 3d0b169..fe58ff8 100644
--- a/src/maasserver/models/event.py
+++ b/src/maasserver/models/event.py
@@ -13,6 +13,7 @@ from django.db.models import (
     CharField,
     DO_NOTHING,
     ForeignKey,
+    Index,
     IntegerField,
     Manager,
     PROTECT,
@@ -146,6 +147,11 @@ class Event(CleanSave, TimestampedModel):
         index_together = (
             ("node", "id"),
         )
+        indexes = [
+            # Needed to get the latest event for each node on the
+            # machine listing page.
+            Index(fields=['node', '-created', '-id']),
+        ]
 
     @property
     def endpoint_name(self):
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index 191b963..de1b513 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -1982,7 +1982,7 @@ class TestNode(MAASServerTestCase):
         self.patch(Node, "_set_status")
         with post_commit_hooks:
             node.abort_disk_erasing(owner)
-        events = Event.objects.filter(node=node)
+        events = Event.objects.filter(node=node).order_by('id')
         self.assertEqual(
             events[0].type.name, EVENT_TYPES.REQUEST_NODE_ABORT_ERASE_DISK)
         self.assertEqual(
@@ -2113,7 +2113,7 @@ class TestNode(MAASServerTestCase):
         self.patch(Node, "_stop").return_value = None
         with post_commit_hooks:
             node.abort_deploying(admin)
-        events = Event.objects.filter(node=node)
+        events = Event.objects.filter(node=node).order_by('id')
         self.assertEqual(
             events[0].type.name, EVENT_TYPES.REQUEST_NODE_ABORT_DEPLOYMENT)
         self.assertEqual(
@@ -2170,7 +2170,7 @@ class TestNode(MAASServerTestCase):
         self.patch(Node, "_stop").return_value = None
         with post_commit_hooks:
             node.abort_testing(admin)
-        events = Event.objects.filter(node=node)
+        events = Event.objects.filter(node=node).order_by('id')
         self.assertEqual(
             events[0].type.name, EVENT_TYPES.REQUEST_NODE_ABORT_TESTING)
         self.assertEqual(
@@ -2339,7 +2339,7 @@ class TestNode(MAASServerTestCase):
         node.power_state = POWER_STATE.OFF
         with post_commit_hooks:
             node.release()
-        events = Event.objects.filter(node=node)
+        events = Event.objects.filter(node=node).order_by('id')
         self.expectThat(events[1].type.name, Equals(EVENT_TYPES.RELEASING))
         self.expectThat(events[2].type.name, Equals(EVENT_TYPES.RELEASED))
         self.expectThat(node._stop, MockNotCalled())
@@ -2542,7 +2542,7 @@ class TestNode(MAASServerTestCase):
         self.patch(node, '_set_status')
         with post_commit_hooks:
             node.release(owner)
-        events = Event.objects.filter(node=node)
+        events = Event.objects.filter(node=node).order_by('id')
         self.assertEqual(events[0].type.name, EVENT_TYPES.REQUEST_NODE_RELEASE)
         self.assertEqual(events[1].type.name, EVENT_TYPES.RELEASING)
 
@@ -3079,7 +3079,7 @@ class TestNode(MAASServerTestCase):
         node.start_commissioning(admin)
         post_commit_hooks.reset()  # Ignore these for now.
         node = reload_object(node)
-        events = Event.objects.filter(node=node)
+        events = Event.objects.filter(node=node).order_by('id')
         self.assertEqual(
             events[0].type.name, EVENT_TYPES.REQUEST_NODE_START_COMMISSIONING)
         self.assertEqual(events[1].type.name, EVENT_TYPES.COMMISSIONING)
@@ -3155,7 +3155,7 @@ class TestNode(MAASServerTestCase):
         self.patch(Node, "_stop").return_value = None
         with post_commit_hooks:
             node.abort_commissioning(admin)
-        events = Event.objects.filter(node=node)
+        events = Event.objects.filter(node=node).order_by('id')
         self.assertEqual(
             events[0].type.name, EVENT_TYPES.REQUEST_NODE_ABORT_COMMISSIONING)
         self.assertEqual(
@@ -3289,7 +3289,7 @@ class TestNode(MAASServerTestCase):
         self.patch(node, '_start').return_value = None
         admin = factory.make_admin()
         node.start_testing(admin, testing_scripts=[script.name])
-        events = Event.objects.filter(node=node)
+        events = Event.objects.filter(node=node).order_by('id')
         post_commit_hooks.reset()  # Ignore these for now.
         node = reload_object(node)
         self.assertEqual(
@@ -4640,7 +4640,7 @@ class TestNode(MAASServerTestCase):
         node.start_rescue_mode(admin)
         post_commit_hooks.reset()  # Ignore these for now.
         node = reload_object(node)
-        events = Event.objects.filter(node=node)
+        events = Event.objects.filter(node=node).order_by('id')
         self.assertEqual(
             events[0].type.name, EVENT_TYPES.REQUEST_NODE_START_RESCUE_MODE)
         self.assertEqual(events[1].type.name, EVENT_TYPES.ENTERING_RESCUE_MODE)
diff --git a/src/maasserver/rpc/tests/test_boot.py b/src/maasserver/rpc/tests/test_boot.py
index a0bfd50..859b752 100644
--- a/src/maasserver/rpc/tests/test_boot.py
+++ b/src/maasserver/rpc/tests/test_boot.py
@@ -769,7 +769,7 @@ class TestGetConfig(MAASServerTestCase):
         for purpose, description in purposes:
             node = self.make_node()
             event_log_pxe_request(node, purpose)
-            events = Event.objects.filter(node=node)
+            events = Event.objects.filter(node=node).order_by('id')
             self.assertEqual(description, events[0].description)
             self.assertEqual(events[1].type.description, EVENT_DETAILS[
                 EVENT_TYPES.PERFORMING_PXE_BOOT].description)