Don't clear branch cache

This keeps the branch cache continuously populated.  When part of
the cache is invalidated by a branch creation/deletion event, instead
of clearing the cache, this updates the data.

That can be slightly more efficient in that it may reduce the number
of ZK transactions, and it may also help us when adding layout loading
to zuul-web, in that those connections will not need to make any remote
calls to the code review system.

Change-Id: I0ad3ea0ecab50ae5261f9510e34ffb27ca83ac1e

2 files changed, 67 insertions, 20 deletions

diff --git a/zuul/connection/__init__.py b/zuul/connection/__init__.py
index 9aa4a9845..5a69dbb55 100644
--- a/zuul/connection/__init__.py
+++ b/zuul/connection/__init__.py
@@ -184,17 +184,42 @@ class ZKBranchCacheMixin:
                 # know if branch protection has been disabled before deletion
                 # of the branch.
                 # FIXME(tobiash): Find a way to handle that case
-                self._branch_cache.clearProjectCache(project.name)
+                self.updateProjectBranches(project)
             elif event.branch_created:
                 # In GitHub, a new branch never can be protected
                 # because that needs to be configured after it has
                 # been created.  Other drivers could optimize this,
                 # but for the moment, implement the lowest common
                 # denominator and clear the cache so that we query.
-                self._branch_cache.clearProjectCache(project.name)
+                self.updateProjectBranches(project)
             event.branch_cache_ltime = self._branch_cache.ltime
         return event
 
+    def updateProjectBranches(self, project):
+        """Update the branch cache for the project.
+
+        :param zuul.model.Project project:
+            The project for which the branches are returned.
+        """
+        # Figure out which queries we have a cache for
+        protected_branches = self._branch_cache.getProjectBranches(
+            project.name, True)
+        all_branches = self._branch_cache.getProjectBranches(
+            project.name, False)
+
+        # Update them if we have them
+        if protected_branches is not None:
+            protected_branches = self._fetchProjectBranches(
+                project, True)
+            self._branch_cache.setProjectBranches(
+                project.name, True, protected_branches)
+
+        if all_branches is not None:
+            all_branches = self._fetchProjectBranches(project, False)
+            self._branch_cache.setProjectBranches(
+                project.name, False, all_branches)
+        self.log.info("Got branches for %s" % project.name)
+
     def getProjectBranches(self, project, tenant, min_ltime=-1):
         """Get the branch names for the given project.
 
@@ -263,17 +288,17 @@ class ZKBranchCacheMixin:
             if not branches:
                 branches = []
 
-            clear = False
+            update = False
             if (event.branch in branches) and (not protected):
-                clear = True
+                update = True
             if (event.branch not in branches) and (protected):
-                clear = True
-            if clear:
-                self.log.debug(
-                    "Clearing protected branch cache for %s",
-                    project_name)
-                self._branch_cache.clearProtectedProjectCache(
-                    project_name)
+                update = True
+            if update:
+                self.log.info("Project %s branch %s protected state "
+                              "changed to %s",
+                              project_name, event.branch, protected)
+                self._branch_cache.setProtected(project_name, event.branch,
+                                                protected)
                 event.branch_cache_ltime = self._branch_cache.ltime
 
             event.branch_protected = protected
diff --git a/zuul/zk/branch_cache.py b/zuul/zk/branch_cache.py
index 032733450..bb861b512 100644
--- a/zuul/zk/branch_cache.py
+++ b/zuul/zk/branch_cache.py
@@ -163,18 +163,40 @@ class BranchCache:
                         remainder = branches
                     self.cache.remainder[project_name] = remainder
 
-    def clearProjectCache(self, project_name):
-        """Clear the connection cache for this project."""
-        with locked(self.wlock):
-            with self.cache.activeContext(self.zk_context):
-                self.cache.protected.pop(project_name, None)
-                self.cache.remainder.pop(project_name, None)
+    def setProtected(self, project_name, branch, protected):
+        """Correct the protection state of a branch.
+
+        This may be called if a branch has changed state without us
+        receiving an explicit event.
+        """
 
-    def clearProtectedProjectCache(self, project_name):
-        """Clear the protected branch list only for this project."""
         with locked(self.wlock):
             with self.cache.activeContext(self.zk_context):
-                self.cache.protected.pop(project_name, None)
+                protected_branches = self.cache.protected.get(project_name)
+                remainder_branches = self.cache.remainder.get(project_name)
+                if protected:
+                    if protected_branches is None:
+                        # We've never run a protected query, so we
+                        # should ignore this branch.
+                        return
+                    else:
+                        # We have run a protected query; if we have
+                        # also run an unprotected query, we need to
+                        # move the branch from remainder to protected.
+                        if remainder_branches and branch in remainder_branches:
+                            remainder_branches.remove(branch)
+                        if branch not in protected_branches:
+                            protected_branches.append(branch)
+                else:
+                    if protected_branches and branch in protected_branches:
+                        protected_branches.remove(branch)
+                    if remainder_branches is None:
+                        # We've never run an unprotected query, so we
+                        # should ignore this branch.
+                        return
+                    else:
+                        if branch not in remainder_branches:
+                            remainder_branches.append(branch)
 
     @property
     def ltime(self):