Add feature flag for cache invalidation method, default old

6 files changed, 54 insertions, 17 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
index 8710b47b914..8b66b7f3fea 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
@@ -81,7 +81,6 @@ public class NodeRepository extends AbstractComponent {
     private final NameResolver nameResolver;
     private final DockerImage dockerImage;
     private final OsVersions osVersions;
-    private final Flags flags;
 
     /**
      * Creates a node repository from a zookeeper provider.
@@ -105,7 +104,6 @@ public class NodeRepository extends AbstractComponent {
         this.nameResolver = nameResolver;
         this.dockerImage = dockerImage;
         this.osVersions = new OsVersions(this.db);
-        this.flags = new Flags(this.db);
 
         // read and write all nodes to make sure they are stored in the latest version of the serialized format
         for (Node.State state : Node.State.values())
@@ -126,7 +124,7 @@ public class NodeRepository extends AbstractComponent {
 
     /** Returns feature flags of this node repository */
     public Flags flags() {
-        return flags;
+        return db.flags();
     }
 
     // ---------------- Query API ----------------------------------------------------------------
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/flag/FlagId.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/flag/FlagId.java
index 1b798c14588..f0714111cea 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/flag/FlagId.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/flag/FlagId.java
@@ -11,7 +11,10 @@ import java.util.Arrays;
 public enum FlagId {
 
     /** Indicates whether a exclusive load balancer should be provisioned */
-    exclusiveLoadBalancer("exclusive-load-balancer");
+    exclusiveLoadBalancer("exclusive-load-balancer"),
+
+    /** Temporary. Indicates whether to use the new cache generation counting, or the old one (with a known bug) */
+    newCacheCounting("new-cache-counting");
 
     private final String serializedValue;
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java
index 2ceffc54dd5..dd1b58dee8e 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java
@@ -93,6 +93,16 @@ public class CuratorDatabase {
         return curatorTransaction;
     }
 
+    // TODO jvenstad: remove.
+    /** Kept for now to be able to revert to old caching behaviour. */
+    CuratorTransaction newEagerCuratorTransactionIn(NestedTransaction transaction) {
+        // Add a counting transaction first, to make sure we always invalidate the current state on any transaction commit
+        transaction.add(new EagerCountingCuratorTransaction(changeGenerationCounter), CuratorTransaction.class);
+        CuratorTransaction curatorTransaction = new CuratorTransaction(curator);
+        transaction.add(curatorTransaction);
+        return curatorTransaction;
+    }
+
     /** Creates a path in curator and all its parents as necessary. If the path already exists this does nothing. */
     void create(Path path) {
         curator.create(path);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
index 959bc5101bc..9bcfdd8b494 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
@@ -20,6 +20,7 @@ import com.yahoo.vespa.curator.transaction.CuratorTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.flag.Flag;
 import com.yahoo.vespa.hosted.provision.flag.FlagId;
+import com.yahoo.vespa.hosted.provision.flag.Flags;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId;
 import com.yahoo.vespa.hosted.provision.node.Agent;
@@ -68,19 +69,17 @@ public class CuratorDatabaseClient {
     private final CuratorDatabase curatorDatabase;
     private final Clock clock;
     private final Zone zone;
+    private final Flags flags;
 
     public CuratorDatabaseClient(NodeFlavors flavors, Curator curator, Clock clock, Zone zone, boolean useCache) {
         this.nodeSerializer = new NodeSerializer(flavors);
         this.zone = zone;
+        this.flags = new Flags(this);
         this.curatorDatabase = new CuratorDatabase(curator, root, useCache);
         this.clock = clock;
         initZK();
     }
 
-    public List<HostName> cluster() {
-        return curatorDatabase.cluster();
-    }
-
     private void initZK() {
         curatorDatabase.create(root);
         for (Node.State state : Node.State.values())
@@ -92,12 +91,20 @@ public class CuratorDatabaseClient {
         curatorDatabase.create(flagsRoot);
     }
 
+    public List<HostName> cluster() {
+        return curatorDatabase.cluster();
+    }
+
+    public Flags flags() {
+        return flags;
+    }
+
     /**
      * Adds a set of nodes. Rollbacks/fails transaction if any node is not in the expected state.
      */
     public List<Node> addNodesInState(List<Node> nodes, Node.State expectedState) {
         NestedTransaction transaction = new NestedTransaction();
-        CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
+        CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction);
         for (Node node : nodes) {
             if (node.state() != expectedState)
                 throw new IllegalArgumentException(node + " is not in the " + node.state() + " state");
@@ -132,7 +139,7 @@ public class CuratorDatabaseClient {
 
         for (Node node : nodes) {
             Path path = toPath(node.state(), node.hostname());
-            CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
+            CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction);
             curatorTransaction.add(CuratorOperations.delete(path.getAbsolute()));
         }
 
@@ -202,7 +209,7 @@ public class CuratorDatabaseClient {
 
         List<Node> writtenNodes = new ArrayList<>(nodes.size());
 
-        CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
+        CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction);
         for (Node node : nodes) {
             Node newNode = new Node(node.openStackId(), node.ipAddresses(), node.ipAddressPool().asSet(), node.hostname(),
                                     node.parentHostname(), node.flavor(),
@@ -361,7 +368,7 @@ public class CuratorDatabaseClient {
 
     public void writeInactiveJobs(Set<String> inactiveJobs) {
         NestedTransaction transaction = new NestedTransaction();
-        CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
+        CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction);
         curatorTransaction.add(CuratorOperations.setData(inactiveJobsPath().getAbsolute(),
                                                          stringSetSerializer.toJson(inactiveJobs)));
         transaction.commit();
@@ -381,7 +388,7 @@ public class CuratorDatabaseClient {
 
     public void writeInfrastructureVersions(Map<NodeType, Version> infrastructureVersions) {
         NestedTransaction transaction = new NestedTransaction();
-        CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
+        CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction);
         curatorTransaction.add(CuratorOperations.setData(infrastructureVersionsPath().getAbsolute(),
                                                          NodeTypeVersionsSerializer.toJson(infrastructureVersions)));
         transaction.commit();
@@ -401,7 +408,7 @@ public class CuratorDatabaseClient {
 
     public void writeOsVersions(Map<NodeType, Version> versions) {
         NestedTransaction transaction = new NestedTransaction();
-        CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
+        CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction);
         curatorTransaction.add(CuratorOperations.setData(osVersionsPath().getAbsolute(),
                                                          NodeTypeVersionsSerializer.toJson(versions)));
         transaction.commit();
@@ -442,7 +449,7 @@ public class CuratorDatabaseClient {
     }
 
     public void writeLoadBalancers(Collection<LoadBalancer> loadBalancers, NestedTransaction transaction) {
-        CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
+        CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction);
         loadBalancers.forEach(loadBalancer -> {
             curatorTransaction.add(createOrSet(loadBalancerPath(loadBalancer.id()),
                                                LoadBalancerSerializer.toJson(loadBalancer)));
@@ -451,7 +458,7 @@ public class CuratorDatabaseClient {
 
     public void removeLoadBalancer(LoadBalancer loadBalancer) {
         NestedTransaction transaction = new NestedTransaction();
-        CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
+        CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction);
         curatorTransaction.add(CuratorOperations.delete(loadBalancerPath(loadBalancer.id()).getAbsolute()));
         transaction.commit();
     }
@@ -467,7 +474,7 @@ public class CuratorDatabaseClient {
     public void writeFlag(Flag flag) {
         Path path = flagPath(flag.id());
         NestedTransaction transaction = new NestedTransaction();
-        CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
+        CuratorTransaction curatorTransaction = newCuratorTransactionIn(transaction);
         curatorTransaction.add(createOrSet(path, FlagSerializer.toJson(flag)));
         transaction.commit();
     }
@@ -491,4 +498,10 @@ public class CuratorDatabaseClient {
         return CuratorOperations.create(path.getAbsolute(), data);
     }
 
+    private CuratorTransaction newCuratorTransactionIn(NestedTransaction transaction) {
+        return flags.get(FlagId.newCacheCounting).isEnabled()
+                ? curatorDatabase.newCuratorTransactionIn(transaction)
+                : curatorDatabase.newEagerCuratorTransactionIn(transaction);
+    }
+
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags1.json
index 8fd09b4a274..f27545f6094 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags1.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags1.json
@@ -5,6 +5,13 @@
       "enabled": false,
       "enabledHostnames": [],
       "enabledApplications": []
+    },
+    {
+      "id": "new-cache-counting",
+      "enabled": false,
+      "enabledHostnames": [],
+      "enabledApplications": []
     }
+
   ]
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags2.json
index 78de52e4e85..a0e9954bec4 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags2.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/flags2.json
@@ -9,6 +9,12 @@
       "enabledApplications": [
         "foo:bar:default"
       ]
+    },
+    {
+      "id": "new-cache-counting",
+      "enabled": false,
+      "enabledHostnames": [],
+      "enabledApplications": []
     }
   ]
 }