Merge lp:~trapnine/maas/slow_node_delete_fix_1581219 into lp:maas/trunk

Proposed by Jeffrey C Jones on 2016-06-10
Status: Merged
Approved by: Jeffrey C Jones on 2016-06-11
Approved revision: 5102
Merged at revision: 5104
Proposed branch: lp:~trapnine/maas/slow_node_delete_fix_1581219
Merge into: lp:maas/trunk
Diff against target: 203 lines (+50/-70)
6 files modified
src/maasserver/migrations/builtin/maasserver/0064_remove_unneeded_event_triggers.py (+29/-0)
src/maasserver/triggers/tests/test_websocket.py (+0/-2)
src/maasserver/triggers/tests/test_websocket_listener.py (+0/-36)
src/maasserver/triggers/websocket.py (+0/-10)
src/maasserver/websockets/handlers/event.py (+3/-9)
src/maasserver/websockets/handlers/tests/test_event.py (+18/-13)
To merge this branch: bzr merge lp:~trapnine/maas/slow_node_delete_fix_1581219
Reviewer Review Type Date Requested Status
Blake Rouse 2016-06-10 Approve on 2016-06-10
Review via email: mp+297100@code.launchpad.net

Commit message

Remove unnecessary and slow event update/delete triggers.

Description of the change

Remove unnecessary and slow event update/delete triggers.

To post a comment you must log in.
Blake Rouse (blake-rouse) wrote :

Looks good.

review: Approve
MAAS Lander (maas-lander) wrote :
Download full text (1.3 MiB)

The attempt to merge lp:~trapnine/maas/slow_node_delete_fix_1581219 into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease
Hit:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:4 http://security.ubuntu.com/ubuntu xenial-security InRelease [94.5 kB]
Fetched 94.5 kB in 0s (219 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind 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 libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt 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...
apache2 is already the newest version (2.4.18-2ubuntu3).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
build-essential is already the newest version (12.1ubuntu2).
bzr is already the newest version (2.7.0-2ubuntu1).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
isc-dhcp-common is already the newest version (4.3.3-5ubuntu12).
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).
pxelinux is already the newest version (3:6.03+df...

5102. By Jeffrey C Jones on 2016-06-11

make format

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'src/maasserver/migrations/builtin/maasserver/0064_remove_unneeded_event_triggers.py'
2--- src/maasserver/migrations/builtin/maasserver/0064_remove_unneeded_event_triggers.py 1970-01-01 00:00:00 +0000
3+++ src/maasserver/migrations/builtin/maasserver/0064_remove_unneeded_event_triggers.py 2016-06-11 00:53:15 +0000
4@@ -0,0 +1,29 @@
5+# -*- coding: utf-8 -*-
6+
7+from contextlib import closing
8+
9+from django.db import (
10+ connection,
11+ migrations,
12+)
13+
14+
15+def remove_unneeded_event_triggers(apps, schema_editor):
16+ with closing(connection.cursor()) as cursor:
17+ cursor.execute(
18+ "DROP TRIGGER IF EXISTS event_event_update_notify ON "
19+ "maasserver_event;"
20+ "DROP TRIGGER IF EXISTS event_event_delete_notify ON "
21+ "maasserver_event;"
22+ )
23+
24+
25+class Migration(migrations.Migration):
26+
27+ dependencies = [
28+ ('maasserver', '0063_remove_orphaned_bmcs_and_ips'),
29+ ]
30+
31+ operations = [
32+ migrations.RunPython(remove_unneeded_event_triggers),
33+ ]
34
35=== modified file 'src/maasserver/triggers/tests/test_websocket.py'
36--- src/maasserver/triggers/tests/test_websocket.py 2016-06-03 20:55:49 +0000
37+++ src/maasserver/triggers/tests/test_websocket.py 2016-06-11 00:53:15 +0000
38@@ -46,8 +46,6 @@
39 "auth_user_user_update_notify",
40 "auth_user_user_delete_notify",
41 "event_event_create_notify",
42- "event_event_update_notify",
43- "event_event_delete_notify",
44 "event_event_create_machine_device_notify",
45 "interface_ip_addresses_nd_sipaddress_link_notify",
46 "interface_ip_addresses_nd_sipaddress_unlink_notify",
47
48=== modified file 'src/maasserver/triggers/tests/test_websocket_listener.py'
49--- src/maasserver/triggers/tests/test_websocket_listener.py 2016-06-02 16:05:11 +0000
50+++ src/maasserver/triggers/tests/test_websocket_listener.py 2016-06-11 00:53:15 +0000
51@@ -557,42 +557,6 @@
52 finally:
53 yield listener.stopService()
54
55- @wait_for_reactor
56- @inlineCallbacks
57- def test__calls_handler_on_update_notification(self):
58- yield deferToDatabase(register_websocket_triggers)
59- listener = self.make_listener_without_delay()
60- dv = DeferredValue()
61- listener.register("event", lambda *args: dv.set(args))
62- event = yield deferToDatabase(self.create_event)
63-
64- yield listener.startService()
65- try:
66- yield deferToDatabase(
67- self.update_event,
68- event.id,
69- {'description': factory.make_name('description')})
70- yield dv.get(timeout=2)
71- self.assertEqual(('update', '%s' % event.id), dv.value)
72- finally:
73- yield listener.stopService()
74-
75- @wait_for_reactor
76- @inlineCallbacks
77- def test__calls_handler_on_delete_notification(self):
78- yield deferToDatabase(register_websocket_triggers)
79- listener = self.make_listener_without_delay()
80- dv = DeferredValue()
81- listener.register("event", lambda *args: dv.set(args))
82- event = yield deferToDatabase(self.create_event)
83- yield listener.startService()
84- try:
85- yield deferToDatabase(self.delete_event, event.id)
86- yield dv.get(timeout=2)
87- self.assertEqual(('delete', '%s' % event.id), dv.value)
88- finally:
89- yield listener.stopService()
90-
91
92 class TestNodeEventListener(
93 MAASTransactionServerTestCase, TransactionalHelpersMixin):
94
95=== modified file 'src/maasserver/triggers/websocket.py'
96--- src/maasserver/triggers/websocket.py 2016-06-03 20:04:57 +0000
97+++ src/maasserver/triggers/websocket.py 2016-06-11 00:53:15 +0000
98@@ -1266,18 +1266,8 @@
99 register_procedure(
100 render_notification_procedure(
101 'event_create_notify', 'event_create', 'NEW.id'))
102- register_procedure(
103- render_notification_procedure(
104- 'event_update_notify', 'event_update', 'NEW.id'))
105- register_procedure(
106- render_notification_procedure(
107- 'event_delete_notify', 'event_delete', 'OLD.id'))
108 register_trigger(
109 "maasserver_event", "event_create_notify", "insert")
110- register_trigger(
111- "maasserver_event", "event_update_notify", "update")
112- register_trigger(
113- "maasserver_event", "event_delete_notify", "delete")
114
115 # Events table, update to linked node.
116 register_procedure(EVENT_NODE_NOTIFY % (
117
118=== modified file 'src/maasserver/websockets/handlers/event.py'
119--- src/maasserver/websockets/handlers/event.py 2015-12-01 18:12:59 +0000
120+++ src/maasserver/websockets/handlers/event.py 2016-06-11 00:53:15 +0000
121@@ -106,15 +106,9 @@
122
123 def on_listen(self, channel, action, pk):
124 """Called by the protocol when a channel notification occurs."""
125- pk = self._meta.pk_type(pk)
126- if action == "delete":
127- # Possible to get a delete for an event that is currently, not
128- # being viewed by the user because it belongs to a differnet node.
129- # Since there is no way to get the deleted event, we just send the
130- # primary key, it will only be removed if the client has an event
131- # with that id.
132- return (self._meta.handler_name, action, pk)
133-
134+ # Only care about create everything else is ignored.
135+ if action != "create":
136+ return None
137 obj = self.listen(channel, action, pk)
138 if obj is None:
139 return None
140
141=== modified file 'src/maasserver/websockets/handlers/tests/test_event.py'
142--- src/maasserver/websockets/handlers/tests/test_event.py 2016-05-12 19:07:37 +0000
143+++ src/maasserver/websockets/handlers/tests/test_event.py 2016-06-11 00:53:15 +0000
144@@ -20,7 +20,10 @@
145 EventHandler,
146 )
147 from maasserver.websockets.handlers.timestampedmodel import dehydrate_datetime
148-from maastesting.matchers import MockCalledOnceWith
149+from maastesting.matchers import (
150+ MockCalledOnceWith,
151+ MockNotCalled,
152+)
153 from testtools.matchers import (
154 Equals,
155 Is,
156@@ -198,17 +201,28 @@
157 self.expectThat(handler.clear({"node_id": node.id}), Is(None))
158 self.expectThat(handler.cache["node_ids"], Equals([]))
159
160- def test_on_listen_calls_listen(self):
161+ def test_on_listen_calls_listen_for_create(self):
162 user = factory.make_User()
163 handler = EventHandler(user, {})
164 mock_listen = self.patch(handler, "listen")
165 mock_listen.return_value = None
166 pk = random.randint(1, 1000)
167- handler.on_listen(sentinel.channel, sentinel.action, pk)
168+ handler.on_listen(sentinel.channel, "create", pk)
169 self.assertThat(
170 mock_listen,
171 MockCalledOnceWith(
172- sentinel.channel, sentinel.action, pk))
173+ sentinel.channel, "create", pk))
174+
175+ def test_on_listen_doesnt_call_listen_for_non_create(self):
176+ user = factory.make_User()
177+ handler = EventHandler(user, {})
178+ mock_listen = self.patch(handler, "listen")
179+ mock_listen.return_value = None
180+ pk = random.randint(1, 1000)
181+ action = factory.make_string()
182+ if action != "create":
183+ handler.on_listen(sentinel.channel, action, pk)
184+ self.assertThat(mock_listen, MockNotCalled())
185
186 def test_on_listen_returns_None_if_listen_returns_None(self):
187 user = factory.make_User()
188@@ -219,15 +233,6 @@
189 handler.on_listen(
190 sentinel.channel, sentinel.action, random.randint(1, 1000)))
191
192- def test_on_listen_delete_returns_handler_name_and_pk(self):
193- user = factory.make_User()
194- pk = random.randint(1, 1000)
195- handler = EventHandler(user, {})
196- self.assertEqual(
197- (handler._meta.handler_name, "delete", pk),
198- handler.on_listen(
199- sentinel.channel, "delete", pk))
200-
201 def test_on_listen_returns_None_if_event_node_id_not_in_cache(self):
202 user = factory.make_User()
203 handler = EventHandler(user, {})