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

[Blake Rouse]

Be defensive in the postgresql listener when a system notification is received for a none existed handler or missing listener channel.

[Gavin Panella (allenap)]

Looks good, but I don't understand how it fixes the bug.

review: Approve

[Blake Rouse (blake-rouse)]

I believe this fixes the bug because the failure was causing continuous failures to handle that notification which was grabbing lots of connections to the database.

It floods the logs on the attached bug. This is the only reason I can see him getting that strange error from postgresql.

=== modified file 'src/maasserver/listener.py'
--- src/maasserver/listener.py	2016-10-24 14:02:17 +0000
+++ src/maasserver/listener.py	2016-11-10 06:48:22 +0000
@@ -160,8 +160,20 @@
                     if self.isSystemChannel(notify.channel):
                         # System level message; pass it to the registered
                         # handler immediately.
-                        handler = self.listeners[notify.channel][0]
-                        handler(notify.channel, notify.payload)
+                        if notify.channel in self.listeners:
+                            # Be defensive in that if a handler does not exist
+                            # for this channel then the channel should be
+                            # unregisted and removed from listeners.
+                            if len(self.listeners[notify.channel]) > 0:
+                                handler = self.listeners[notify.channel][0]
+                                handler(notify.channel, notify.payload)
+                            else:
+                                self.unregisterChannel(notify.channel)
+                                del self.listeners[notify.channel]
+                        else:
+                            # Unregister the channel since no listener is
+                            # registered for this channel.
+                            self.unregisterChannel(notify.channel)
                     else:
                         # Place non-system messages into the queue to be
                         # processed.

=== modified file 'src/maasserver/tests/test_listener.py'
--- src/maasserver/tests/test_listener.py	2016-10-24 14:02:17 +0000
+++ src/maasserver/tests/test_listener.py	2016-11-10 06:48:22 +0000
@@ -110,6 +110,39 @@
 
     @wait_for_reactor
     @inlineCallbacks
+    def test__handles_missing_system_handler_on_notification(self):
+        listener = PostgresListenerService()
+        # Change notifications to a frozenset. This makes sure that
+        # the system message does not go into the queue. Instead if should
+        # call the handler directly in `doRead`.
+        listener.notifications = frozenset()
+        yield listener.startService()
+        yield deferToDatabase(listener.registerChannel, "sys_test")
+        listener.listeners["sys_test"] = []
+        try:
+            yield deferToDatabase(self.send_notification, "sys_test", 1)
+            self.assertFalse("sys_test" in listener.listeners)
+        finally:
+            yield listener.stopService()
+
+    @wait_for_reactor
+    @inlineCallbacks
+    def test__handles_missing_notify_system_listener_on_notification(self):
+        listener = PostgresListenerService()
+        # Change notifications to a frozenset. This makes sure that
+        # the system message does not go into the queue. Instead if should
+        # call the handler directly in `doRead`.
+        listener.notifications = frozenset()
+        yield listener.startService()
+        yield deferToDatabase(listener.registerChannel, "sys_test")
+        try:
+            yield deferToDatabase(self.send_notification, "sys_test", 1)
+            self.assertFalse("sys_test" in listener.listeners)
+        finally:
+            yield listener.stopService()
+
+    @wait_for_reactor
+    @inlineCallbacks
     def test__calls_handler_on_notification(self):
         listener = PostgresListenerService()
         dv = DeferredValue()