Fix SemaphoreHandler leak

A recent change pushed semaphore handling into zookeeper. To do this a
new ZooKeeperBase class called SemaphoreHandler was written. Previously
the ZookeeperBase class expected instances of it to be few in number and
long lived, but semaphore handlers are replaced every time a new layout
is created. Unfortunately this expectation causes problems because
references to onConnect and onDisconnect methods in the SemaphoreHanlder
are held by the ZooKeeper Client. This causes the SemaphoreHandlers to
not be garbage collectable when they are replaced.

To make this worse the SemaphoreHandlers also keep a reference to tenant
layouts. Since the SemaphoreHandler cannot be GC'd the layout cannot be
either and we create a big enough to notice over time memory leak.

We fix this by changing the ZooKeeper client component hierarchy
slightly to introduce the idea that some components need state handling
on connection state changes and others do not. For the ones that do not
like SemaphoreHandler we can simply avoid registering any methods with
the client. The class previously didn't make use of this so no change in
behavior there.

Credit to corvus for doing the debugging and tracking this down. I'm
just trying to get a head start on a mergeable change.

Change-Id: I198769f3b7f3efe3c9d9321e0c12c0771dea0281

zuul/zk/__init__.py

@@ -169,13 +169,11 @@ class ZooKeeperClient(object):
         )
 
 
-class ZooKeeperBase(metaclass=ABCMeta):
-    """Base class for components that need to interact with Zookeeper."""
+class ZooKeeperSimpleBase(metaclass=ABCMeta):
+    """Base class for stateless Zookeeper interaction."""
 
     def __init__(self, client: ZooKeeperClient):
         self.client = client
-        self.client.on_connect_listeners.append(self._onConnect)
-        self.client.on_disconnect_listeners.append(self._onDisconnect)
 
     @property
     def kazoo_client(self) -> KazooClient:
@@ -183,6 +181,15 @@ class ZooKeeperBase(metaclass=ABCMeta):
             raise NoClientException()
         return self.client.client
 
+
+class ZooKeeperBase(ZooKeeperSimpleBase):
+    """Base class for registering state handling methods with ZooKeeper."""
+
+    def __init__(self, client: ZooKeeperClient):
+        super().__init__(client)
+        self.client.on_connect_listeners.append(self._onConnect)
+        self.client.on_disconnect_listeners.append(self._onDisconnect)
+
     def _onConnect(self):
         pass
 

zuul/zk/semaphore.py

@@ -19,7 +19,7 @@ from urllib.parse import quote_plus
 from kazoo.exceptions import BadVersionError, NoNodeError
 
 from zuul.lib.logutil import get_annotated_logger
-from zuul.zk import ZooKeeperBase
+from zuul.zk import ZooKeeperSimpleBase
 
 
 def holdersFromData(data):
@@ -32,7 +32,7 @@ def holdersToData(holders):
     return json.dumps(holders).encode("utf8")
 
 
-class SemaphoreHandler(ZooKeeperBase):
+class SemaphoreHandler(ZooKeeperSimpleBase):
     log = logging.getLogger("zuul.zk.SemaphoreHandler")
 
     semaphore_root = "/zuul/semaphores"