diff options
author | Martin Polden <mpolden@mpolden.no> | 2019-06-11 13:30:51 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-06-11 13:30:51 +0200 |
commit | 1307945954b7ca541b640a30fb06e5a27e43fdf3 (patch) | |
tree | dc5e873edd8e44302ebf27d2547555ff735ea8ee | |
parent | bf0f7ced58968e26eafa632cb68cfb3757d4dd2f (diff) | |
parent | 70b15c19f0cf69fcae619edab1aa32dc88613e40 (diff) |
Merge pull request #9744 from vespa-engine/mpolden/remove-rotations-serialization
Remove serialization of ClusterSpec rotations
10 files changed, 18 insertions, 88 deletions
diff --git a/config-provisioning/abi-spec.json b/config-provisioning/abi-spec.json index e88947b3fdb..aa9b196c0e4 100644 --- a/config-provisioning/abi-spec.json +++ b/config-provisioning/abi-spec.json @@ -260,7 +260,6 @@ "public com.yahoo.component.Version vespaVersion()", "public java.util.Optional group()", "public boolean isExclusive()", - "public java.util.Set rotations()", "public com.yahoo.config.provision.ClusterSpec with(java.util.Optional)", "public com.yahoo.config.provision.ClusterSpec exclusive(boolean)", "public static com.yahoo.config.provision.ClusterSpec request(com.yahoo.config.provision.ClusterSpec$Type, com.yahoo.config.provision.ClusterSpec$Id, com.yahoo.component.Version, boolean)", diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java index c0099878b45..f041823bf04 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java @@ -3,14 +3,9 @@ package com.yahoo.config.provision; import com.yahoo.component.Version; -import java.util.Arrays; -import java.util.Collections; -import java.util.Set; -import java.util.stream.Collectors; - /** * A node's membership in a cluster. This is a value object. - * The format is "clusterType/clusterId/groupId/index[/exclusive][/retired][/rotationId,...]" + * The format is "clusterType/clusterId/groupId/index[/exclusive][/retired]" * * @author bratseth */ @@ -25,26 +20,23 @@ public class ClusterMembership { private ClusterMembership(String stringValue, Version vespaVersion) { String[] components = stringValue.split("/"); - if (components.length < 4 || components.length > 7) + if (components.length < 4) throw new RuntimeException("Could not parse '" + stringValue + "' to a cluster membership. " + - "Expected 'clusterType/clusterId/groupId/index[/retired][/exclusive][/rotationId,...]'"); + "Expected 'clusterType/clusterId/groupId/index[/retired][/exclusive]'"); boolean exclusive = false; - Set<RotationName> rotations = Collections.emptySet(); if (components.length > 4) { for (int i = 4; i < components.length; i++) { String component = components[i]; switch (component) { case "exclusive": exclusive = true; break; case "retired": retired = true; break; - default: rotations = rotationsFrom(component); break; } } } this.cluster = ClusterSpec.from(ClusterSpec.Type.valueOf(components[0]), ClusterSpec.Id.from(components[1]), - ClusterSpec.Group.from(Integer.valueOf(components[2])), vespaVersion, exclusive, - rotations); + ClusterSpec.Group.from(Integer.valueOf(components[2])), vespaVersion, exclusive); this.index = Integer.parseInt(components[3]); this.stringValue = toStringValue(); } @@ -62,8 +54,7 @@ public class ClusterMembership { (cluster.group().isPresent() ? "/" + cluster.group().get().index() : "") + "/" + index + ( cluster.isExclusive() ? "/exclusive" : "") + - ( retired ? "/retired" : "") + - ( !cluster.rotations().isEmpty() ? "/" + rotationsAsString(cluster.rotations()) : ""); + ( retired ? "/retired" : ""); } @@ -121,12 +112,4 @@ public class ClusterMembership { return new ClusterMembership(cluster, index, true); } - private static Set<RotationName> rotationsFrom(String s) { - return Arrays.stream(s.split(",")).map(RotationName::from).collect(Collectors.toUnmodifiableSet()); - } - - private static String rotationsAsString(Set<RotationName> rotations) { - return rotations.stream().map(RotationName::value).collect(Collectors.joining(",")); - } - } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java index 35ee538178a..8ed56b98705 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision; -import com.google.common.collect.ImmutableSortedSet; import com.yahoo.component.Version; import java.util.Objects; @@ -23,19 +22,13 @@ public final class ClusterSpec { private final Optional<Group> groupId; private final Version vespaVersion; private boolean exclusive; - private final Set<RotationName> rotations; - private ClusterSpec(Type type, Id id, Optional<Group> groupId, Version vespaVersion, boolean exclusive, - Set<RotationName> rotations) { - if (type != Type.container && !rotations.isEmpty()) { - throw new IllegalArgumentException("Rotations can only be declared for clusters of type " + Type.container); - } + private ClusterSpec(Type type, Id id, Optional<Group> groupId, Version vespaVersion, boolean exclusive) { this.type = type; this.id = id; this.groupId = groupId; this.vespaVersion = vespaVersion; this.exclusive = exclusive; - this.rotations = ImmutableSortedSet.copyOf(rotations); } /** Returns the cluster type */ @@ -57,35 +50,30 @@ public final class ClusterSpec { */ public boolean isExclusive() { return exclusive; } - /** Returns the rotations of which this cluster should be a member */ - public Set<RotationName> rotations() { - return rotations; - } - public ClusterSpec with(Optional<Group> newGroup) { - return new ClusterSpec(type, id, newGroup, vespaVersion, exclusive, rotations); + return new ClusterSpec(type, id, newGroup, vespaVersion, exclusive); } public ClusterSpec exclusive(boolean exclusive) { - return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, rotations); + return new ClusterSpec(type, id, groupId, vespaVersion, exclusive); } public static ClusterSpec request(Type type, Id id, Version vespaVersion, boolean exclusive) { - return new ClusterSpec(type, id, Optional.empty(), vespaVersion, exclusive, Set.of()); + return new ClusterSpec(type, id, Optional.empty(), vespaVersion, exclusive); } // TODO: Remove after June 2019 public static ClusterSpec request(Type type, Id id, Version vespaVersion, boolean exclusive, Set<RotationName> rotations) { - return new ClusterSpec(type, id, Optional.empty(), vespaVersion, exclusive, rotations); + return new ClusterSpec(type, id, Optional.empty(), vespaVersion, exclusive); } public static ClusterSpec from(Type type, Id id, Group groupId, Version vespaVersion, boolean exclusive) { - return new ClusterSpec(type, id, Optional.of(groupId), vespaVersion, exclusive, Set.of()); + return new ClusterSpec(type, id, Optional.of(groupId), vespaVersion, exclusive); } // TODO: Remove after June 2019 public static ClusterSpec from(Type type, Id id, Group groupId, Version vespaVersion, boolean exclusive, Set<RotationName> rotations) { - return new ClusterSpec(type, id, Optional.of(groupId), vespaVersion, exclusive, rotations); + return new ClusterSpec(type, id, Optional.of(groupId), vespaVersion, exclusive); } @Override diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java index 9bd0680b691..5eee55a1886 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java @@ -6,7 +6,6 @@ import com.yahoo.component.Vtag; import org.junit.Test; import java.util.Collections; -import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -37,26 +36,25 @@ public class ClusterMembershipTest { assertTrue(instance.cluster().isExclusive()); } + // TODO: Remove after June 2019. This ensures stale rotation data is handled { ClusterMembership instance = ClusterMembership.from("container/id1/4/37/rotation1,rotation2", Vtag.currentVersion); assertFalse(instance.retired()); assertFalse(instance.cluster().isExclusive()); - assertEquals(Set.of(RotationName.from("rotation1"), RotationName.from("rotation2")), instance.cluster().rotations()); } { ClusterMembership instance = ClusterMembership.from("container/id1/4/37/exclusive/rotation1,rotation2", Vtag.currentVersion); assertFalse(instance.retired()); assertTrue(instance.cluster().isExclusive()); - assertEquals(Set.of(RotationName.from("rotation1"), RotationName.from("rotation2")), instance.cluster().rotations()); } { ClusterMembership instance = ClusterMembership.from("container/id1/4/37/exclusive/retired/rotation1,rotation2", Vtag.currentVersion); assertTrue(instance.retired()); assertTrue(instance.cluster().isExclusive()); - assertEquals(Set.of(RotationName.from("rotation1"), RotationName.from("rotation2")), instance.cluster().rotations()); } + // end TODO } @Test @@ -101,7 +99,6 @@ public class ClusterMembershipTest { assertFalse(instance.cluster().group().isPresent()); assertEquals(3, instance.index()); assertEquals("container/id1/3", instance.stringValue()); - assertTrue(instance.cluster().rotations().isEmpty()); } private void assertContentService(ClusterMembership instance) { @@ -111,7 +108,6 @@ public class ClusterMembershipTest { assertEquals(37, instance.index()); assertFalse(instance.retired()); assertEquals("content/id1/37", instance.stringValue()); - assertTrue(instance.cluster().rotations().isEmpty()); } private void assertContentServiceWithGroup(ClusterMembership instance) { @@ -121,7 +117,6 @@ public class ClusterMembershipTest { assertEquals(37, instance.index()); assertFalse(instance.retired()); assertEquals("content/id1/4/37", instance.stringValue()); - assertTrue(instance.cluster().rotations().isEmpty()); } /** Serializing a spec without a group assigned works, but not deserialization */ @@ -131,7 +126,6 @@ public class ClusterMembershipTest { assertEquals(37, instance.index()); assertTrue(instance.retired()); assertEquals("content/id1/37/retired", instance.stringValue()); - assertTrue(instance.cluster().rotations().isEmpty()); } private void assertContentServiceWithGroupAndRetire(ClusterMembership instance) { @@ -141,7 +135,6 @@ public class ClusterMembershipTest { assertEquals(37, instance.index()); assertTrue(instance.retired()); assertEquals("content/id1/4/37/retired", instance.stringValue()); - assertTrue(instance.cluster().rotations().isEmpty()); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java index a6d311604fd..58c576d3f44 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java @@ -1,12 +1,9 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.lb; -import com.google.common.collect.ImmutableSortedSet; -import com.yahoo.config.provision.RotationName; import com.yahoo.vespa.hosted.provision.maintenance.LoadBalancerExpirer; import java.util.Objects; -import java.util.Set; /** * Represents a load balancer for an application's cluster. This is immutable. @@ -17,13 +14,11 @@ public class LoadBalancer { private final LoadBalancerId id; private final LoadBalancerInstance instance; - private final Set<RotationName> rotations; private final boolean inactive; - public LoadBalancer(LoadBalancerId id, LoadBalancerInstance instance, Set<RotationName> rotations, boolean inactive) { + public LoadBalancer(LoadBalancerId id, LoadBalancerInstance instance, boolean inactive) { this.id = Objects.requireNonNull(id, "id must be non-null"); this.instance = Objects.requireNonNull(instance, "instance must be non-null"); - this.rotations = ImmutableSortedSet.copyOf(Objects.requireNonNull(rotations, "rotations must be non-null")); this.inactive = inactive; } @@ -32,11 +27,6 @@ public class LoadBalancer { return id; } - /** The rotations of which this is a member */ - public Set<RotationName> rotations() { - return rotations; - } - /** The instance associated with this */ public LoadBalancerInstance instance() { return instance; @@ -52,7 +42,7 @@ public class LoadBalancer { /** Return a copy of this that is set inactive */ public LoadBalancer deactivate() { - return new LoadBalancer(id, instance, rotations, true); + return new LoadBalancer(id, instance, true); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java index 17f2d7364a6..a4b915a6128 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.provision.persistence; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RotationName; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; @@ -35,7 +34,6 @@ public class LoadBalancerSerializer { private static final String portsField = "ports"; private static final String networksField = "networks"; private static final String realsField = "reals"; - private static final String rotationsField = "rotations"; private static final String nameField = "name"; private static final String ipAddressField = "ipAddress"; private static final String portField = "port"; @@ -58,11 +56,6 @@ public class LoadBalancerSerializer { realObject.setString(ipAddressField, real.ipAddress()); realObject.setLong(portField, real.port()); }); - Cursor rotationArray = root.setArray(rotationsField); - loadBalancer.rotations().forEach(rotation -> { - Cursor rotationObject = rotationArray.addObject(); - rotationObject.setString(nameField, rotation.value()); - }); root.setBool(inactiveField, loadBalancer.inactive()); try { @@ -89,11 +82,6 @@ public class LoadBalancerSerializer { Set<String> networks = new LinkedHashSet<>(); object.field(networksField).traverse((ArrayTraverser) (i, network) -> networks.add(network.asString())); - Set<RotationName> rotations = new LinkedHashSet<>(); - object.field(rotationsField).traverse((ArrayTraverser) (i, rotation) -> { - rotations.add(RotationName.from(rotation.field(nameField).asString())); - }); - return new LoadBalancer(LoadBalancerId.fromSerializedForm(object.field(idField).asString()), new LoadBalancerInstance( HostName.from(object.field(hostnameField).asString()), @@ -102,7 +90,6 @@ public class LoadBalancerSerializer { networks, reals ), - rotations, object.field(inactiveField).asBool()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index f5f8ed53d2a..372dca84a53 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -57,7 +57,7 @@ public class LoadBalancerProvisioner { LoadBalancerInstance instance = create(application, kv.getKey().id(), kv.getValue()); // Load balancer is always re-activated here to avoid reallocation if an application/cluster is // deleted and then redeployed. - LoadBalancer loadBalancer = new LoadBalancer(id, instance, kv.getKey().rotations(), false); + LoadBalancer loadBalancer = new LoadBalancer(id, instance, false); loadBalancers.put(loadBalancer.id(), loadBalancer); db.writeLoadBalancer(loadBalancer); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java index 69e13f77a09..d31834567ab 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java @@ -76,11 +76,7 @@ public class LoadBalancersResponse extends HttpResponse { realObject.setLong("port", real.port()); }); - Cursor rotationArray = lbObject.setArray("rotations"); - lb.rotations().forEach(rotation -> { - Cursor rotationObject = rotationArray.addObject(); - rotationObject.setString("name", rotation.value()); - }); + lbObject.setArray("rotations"); // To avoid changing the API. This can be removed when clients stop expecting this lbObject.setBool("inactive", lb.inactive()); }); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java index 6de93c2ae65..460764b50db 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java @@ -5,7 +5,6 @@ import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RotationName; import com.yahoo.vespa.hosted.provision.lb.DnsZone; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId; @@ -39,8 +38,6 @@ public class LoadBalancerSerializerTest { new Real(HostName.from("real-2"), "127.0.0.2", 4080))), - ImmutableSet.of(RotationName.from("eu-cluster"), - RotationName.from("us-cluster")), false); LoadBalancer serialized = LoadBalancerSerializer.fromJson(LoadBalancerSerializer.toJson(loadBalancer)); @@ -49,7 +46,6 @@ public class LoadBalancerSerializerTest { assertEquals(loadBalancer.instance().dnsZone(), serialized.instance().dnsZone()); assertEquals(loadBalancer.instance().ports(), serialized.instance().ports()); assertEquals(loadBalancer.instance().networks(), serialized.instance().networks()); - assertEquals(loadBalancer.rotations(), serialized.rotations()); assertEquals(loadBalancer.inactive(), serialized.inactive()); assertEquals(loadBalancer.instance().reals(), serialized.instance().reals()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java index 58c0b3ed9cc..f97460713a5 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java @@ -61,7 +61,6 @@ public class LoadBalancerProvisionerTest { assertEquals(4080, get(loadBalancers.get().get(0).instance().reals(), 0).port()); assertEquals("127.0.0.2", get(loadBalancers.get().get(0).instance().reals(), 1).ipAddress()); assertEquals(4080, get(loadBalancers.get().get(0).instance().reals(), 1).port()); - assertEquals(rotationsCluster1, loadBalancers.get().get(0).rotations()); // A container is failed Supplier<List<Node>> containers = () -> tester.getNodes(app1).type(ClusterSpec.Type.container).asList(); @@ -105,7 +104,6 @@ public class LoadBalancerProvisionerTest { .map(Real::hostname) .sorted() .collect(Collectors.toList()); - assertEquals(rotationsCluster2, loadBalancers.get().get(1).rotations()); assertEquals(activeContainers, reals); // Application is removed and load balancer is deactivated |