Merge lp:~gmb/maas/fix-bug-1402237-1.7 into lp:maas/1.7

Proposed by Graham Binns
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 3332
Proposed branch: lp:~gmb/maas/fix-bug-1402237-1.7
Merge into: lp:maas/1.7
Diff against target: 69 lines (+29/-8)
2 files modified
src/maasserver/views/nodes.py (+19/-8)
src/maasserver/views/tests/test_nodes.py (+10/-0)
To merge this branch: bzr merge lp:~gmb/maas/fix-bug-1402237-1.7
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve
Review via email: mp+244986@code.launchpad.net

Commit message

When fetching the non-debug events for a node, fetch the non-debug EventTypes first and then query the Event table using 'WHERE type_id IN <non-debug-EventTypes>'.

Previously, this query was the default Django-generated one, which did an INNER JOIN between Event and EventType. The new version is significantly faster, because there's an index missing on EventType.level which makes things slow at scale.

To post a comment you must log in.
Revision history for this message
Raphaël Badin (rvb) wrote :

Looks good. Couple of remarks below but nothing major.

[0]

Typo in the commit message: fecthing

[1]

Could you say in the commit message why we're using this trick instead of adding the index… and that the index is being added to trunk…?

review: Approve
Revision history for this message
Graham Binns (gmb) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py 2014-12-10 16:28:52 +0000
+++ src/maasserver/views/nodes.py 2014-12-18 13:51:39 +0000
@@ -81,7 +81,10 @@
81 Tag,81 Tag,
82 )82 )
83from maasserver.models.config import Config83from maasserver.models.config import Config
84from maasserver.models.event import Event84from maasserver.models.event import (
85 Event,
86 EventType,
87 )
85from maasserver.models.nodeprobeddetails import get_single_probed_details88from maasserver.models.nodeprobeddetails import get_single_probed_details
86from maasserver.node_action import ACTIONS_DICT89from maasserver.node_action import ACTIONS_DICT
87from maasserver.node_constraint_filter_forms import (90from maasserver.node_constraint_filter_forms import (
@@ -238,17 +241,25 @@
238 # Add event information to the generated node dictionary. We exclude241 # Add event information to the generated node dictionary. We exclude
239 # debug after we calculate the count, so we show the correct total242 # debug after we calculate the count, so we show the correct total
240 # number of events.243 # number of events.
241 events = Event.objects.filter(node=node)244 node_events = Event.objects.filter(node=node)
242 total_num_events = events.count()245 total_num_events = node_events.count()
243 events = events.exclude(type__level=logging.DEBUG).order_by('-id')246
244 events_count = events.count()247 # We fetch the IDs of the EventTypes that are non-DEBUG here
248 # because in MAAS 1.7 the EventType.level field is missing an
249 # index. That makes querying for Events whose EventType has a
250 # non-DEBUG level (using an INNER JOIN) slow at scale. Doing it
251 # this way speeds things up considerably.
252 non_debug_event_type_ids = EventType.objects.exclude(
253 level=logging.DEBUG).values_list('id', flat=True)
254 non_debug_events = node_events.filter(
255 type__id__in=non_debug_event_type_ids).order_by('-id')
245 if event_log_count > 0:256 if event_log_count > 0:
246 # Limit the number of events.257 # Limit the number of events.
247 events = events.all()[:event_log_count]258 events = non_debug_events.all()[:event_log_count]
248 events_count = len(events)259 displayed_events_count = len(events)
249 node_dict['events'] = dict(260 node_dict['events'] = dict(
250 total=total_num_events,261 total=total_num_events,
251 count=events_count,262 count=displayed_events_count,
252 events=[event_to_dict(event) for event in events],263 events=[event_to_dict(event) for event in events],
253 more_url=reverse('node-event-list-view', args=[node.system_id]),264 more_url=reverse('node-event-list-view', args=[node.system_id]),
254 )265 )
255266
=== modified file 'src/maasserver/views/tests/test_nodes.py'
--- src/maasserver/views/tests/test_nodes.py 2014-12-15 10:04:18 +0000
+++ src/maasserver/views/tests/test_nodes.py 2014-12-18 13:51:39 +0000
@@ -164,6 +164,16 @@
164 dict_node['events']['more_url'],164 dict_node['events']['more_url'],
165 Equals(reverse('node-event-list-view', args=[node.system_id])))165 Equals(reverse('node-event-list-view', args=[node.system_id])))
166166
167 def test_node_to_dict_excludes_debug_events(self):
168 node = factory.make_Node()
169 debug_type = factory.make_EventType(level=logging.DEBUG)
170 debug_event = factory.make_Event(node, debug_type)
171
172 node_dict = node_to_dict(node, event_log_count=2)
173 self.assertThat(
174 [event_dict['id'] for event_dict in node_dict['events']['events']],
175 Not(Contains(debug_event.id)))
176
167 def test_event_to_dict_keys(self):177 def test_event_to_dict_keys(self):
168 event = factory.make_Event()178 event = factory.make_Event()
169 self.assertThat(179 self.assertThat(

Subscribers

People subscribed via source and target branches

to all changes: