From 7c0a02a79e3007c1dee4be7a98902dc1f1858023 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 17 Dec 2018 11:17:16 +0100 Subject: Add feature flag for cache invalidation method, default old --- .../vespa/hosted/provision/NodeRepository.java | 4 +-- .../yahoo/vespa/hosted/provision/flag/FlagId.java | 5 ++- .../provision/persistence/CuratorDatabase.java | 10 ++++++ .../persistence/CuratorDatabaseClient.java | 39 ++++++++++++++-------- .../provision/restapi/v2/responses/flags1.json | 7 ++++ .../provision/restapi/v2/responses/flags2.json | 6 ++++ 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 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 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 addNodesInState(List 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 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 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 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 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 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": [] } ] } -- cgit v1.2.3