Metadata: reread client list from database

This fixes two related bugs: One causes Metadata to use an out-of-date
cached list of clients when a client is deleted or added with
bcfg2-admin; the other causes child worker processes to use an
out-of-date cached list of clients when a client is added with a Bcfg2
run when the multiprocessing core is in use.

3 files changed, 41 insertions, 5 deletions

diff --git a/src/lib/Bcfg2/Server/MultiprocessingCore.py b/src/lib/Bcfg2/Server/MultiprocessingCore.py
index 6d41bbcbb..2cb3adae3 100644
--- a/src/lib/Bcfg2/Server/MultiprocessingCore.py
+++ b/src/lib/Bcfg2/Server/MultiprocessingCore.py
@@ -303,6 +303,7 @@ class ChildCore(BaseCore):
     @exposed
     def GetConfig(self, client):
         """ Render the configuration for a client """
+        self.metadata.update_client_list()
         self.logger.debug("%s: Building configuration for %s" %
                           (self.name, client))
         return lxml.etree.tostring(self.BuildConfiguration(client))
diff --git a/src/lib/Bcfg2/Server/Plugin/interfaces.py b/src/lib/Bcfg2/Server/Plugin/interfaces.py
index 33f6d338c..07717a710 100644
--- a/src/lib/Bcfg2/Server/Plugin/interfaces.py
+++ b/src/lib/Bcfg2/Server/Plugin/interfaces.py
@@ -213,6 +213,10 @@ class Metadata(object):
         """
         raise NotImplementedError
 
+    def update_client_list(self):
+        """ Re-read the cached list of clients """
+        raise NotImplementedError
+
 
 class Connector(object):
     """ Connector plugins augment client metadata instances with
diff --git a/src/lib/Bcfg2/Server/Plugins/Metadata.py b/src/lib/Bcfg2/Server/Plugins/Metadata.py
index f734c98d0..d6febcff6 100644
--- a/src/lib/Bcfg2/Server/Plugins/Metadata.py
+++ b/src/lib/Bcfg2/Server/Plugins/Metadata.py
@@ -668,7 +668,7 @@ class Metadata(Bcfg2.Server.Plugin.Metadata,
             except MetadataClientModel.DoesNotExist:
                 client = MetadataClientModel(hostname=client_name)
                 client.save()
-            self.clients = self.list_clients()
+            self.update_client_list()
             return client
         else:
             try:
@@ -721,7 +721,15 @@ class Metadata(Bcfg2.Server.Plugin.Metadata,
                                       attribs, alias=True)
 
     def list_clients(self):
-        """ List all clients in client database """
+        """ List all clients in client database.
+
+        Making ``self.clients`` a property and reading the client list
+        dynamically from the database on every call to
+        ``self.clients`` can result in very high rates of database
+        reads, so we cache the ``list_clients()`` results to reduce
+        the database load.  When the database is in use, the client
+        list is reread periodically with
+        :func:`Bcfg2.Server.Plugins.Metadata.update_client_list`. """
         if self._use_db:
             return set([c.hostname for c in MetadataClientModel.objects.all()])
         else:
@@ -772,7 +780,7 @@ class Metadata(Bcfg2.Server.Plugin.Metadata,
                 self.logger.warning(msg)
                 raise Bcfg2.Server.Plugin.MetadataConsistencyError(msg)
             client.delete()
-            self.clients = self.list_clients()
+            self.update_client_list()
         else:
             return self._remove_xdata(self.clients_xml, "Client", client_name)
 
@@ -841,8 +849,7 @@ class Metadata(Bcfg2.Server.Plugin.Metadata,
             except KeyError:
                 self.clientgroups[clname] = [profile]
         self.states['clients.xml'] = True
-        if self._use_db:
-            self.clients = self.list_clients()
+        self.update_client_list()
 
     def _get_condition(self, element):
         """ Return a predicate that returns True if a client meets
@@ -1434,6 +1441,30 @@ class Metadata(Bcfg2.Server.Plugin.Metadata,
         return True
     # pylint: enable=R0911,R0912
 
+    def update_client_list(self):
+        """ Re-read the client list from the database (if the database is in
+        use) """
+        if self._use_db:
+            self.logger.debug("Metadata: Re-reading client list from database")
+            old = set(self.clients)
+            self.clients = self.list_clients()
+            new = set(self.clients)
+            added = new - old
+            removed = old - new
+            self.logger.debug("Metadata: Added %s clients: %s" %
+                              (len(added), added))
+            self.logger.debug("Metadata: Removed %s clients: %s" %
+                              (len(removed), removed))
+            # we could do this with set.symmetric_difference(), but we
+            # want detailed numbers of added/removed clients for
+            # logging
+            for client in added.union(removed):
+                self.expire_cache(client)
+
+    def start_client_run(self, metadata):
+        """ Hook to reread client list if the database is in use """
+        self.update_client_list()
+
     def end_statistics(self, metadata):
         """ Hook to toggle clients in bootstrap mode """
         if self.auth.get(metadata.hostname,