diff options
author | Jon Bratseth <bratseth@oath.com> | 2021-06-18 10:18:58 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-06-18 10:18:58 +0200 |
commit | 535c296b9c51a1c8b94a3afbc8c38bed57c95cc2 (patch) | |
tree | 644859d3eb4d3a0bdc888c1b8e4544e39e0947ab /controller-server | |
parent | 0aa18d7fd68f42b778652f9e9e75034515b84b82 (diff) | |
parent | c8a5556d50ac960f48c25067fd0ffd6ee64994fd (diff) |
Merge pull request #18312 from vespa-engine/mpolden/faster-tests
Minor performance improvements to controller-server tests
Diffstat (limited to 'controller-server')
6 files changed, 85 insertions, 59 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java index 2322b251fe0..4f01df21430 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ZipStreamReader.java @@ -1,15 +1,14 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.application; -import com.google.common.collect.ImmutableList; - import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.UncheckedIOException; import java.nio.file.Path; -import java.util.Arrays; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.function.Predicate; import java.util.zip.ZipEntry; @@ -21,19 +20,19 @@ import java.util.zip.ZipOutputStream; */ public class ZipStreamReader { - private final ImmutableList<ZipEntryWithContent> entries; + private final List<ZipEntryWithContent> entries = new ArrayList<>(); private final int maxEntrySizeInBytes; public ZipStreamReader(InputStream input, Predicate<String> entryNameMatcher, int maxEntrySizeInBytes) { this.maxEntrySizeInBytes = maxEntrySizeInBytes; try (ZipInputStream zipInput = new ZipInputStream(input)) { - ImmutableList.Builder<ZipEntryWithContent> builder = new ImmutableList.Builder<>(); ZipEntry zipEntry; + while (null != (zipEntry = zipInput.getNextEntry())) { if (!entryNameMatcher.test(requireName(zipEntry.getName()))) continue; - builder.add(new ZipEntryWithContent(zipEntry, readContent(zipInput))); + entries.add(new ZipEntryWithContent(zipEntry, readContent(zipInput))); } - entries = builder.build(); + } catch (IOException e) { throw new UncheckedIOException("IO error reading zip content", e); } @@ -79,10 +78,10 @@ public class ZipStreamReader { } } - public List<ZipEntryWithContent> entries() { return entries; } + public List<ZipEntryWithContent> entries() { return Collections.unmodifiableList(entries); } private static String requireName(String name) { - if (Arrays.asList(name.split("/")).contains("..") || + if (List.of(name.split("/")).contains("..") || !trimTrailingSlash(name).equals(Path.of(name).normalize().toString())) { throw new IllegalArgumentException("Unexpected non-normalized path found in zip content: '" + name + "'"); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index fc7a99eb2f0..78f688f545b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -27,6 +27,7 @@ import java.util.Date; import java.util.List; import java.util.OptionalInt; import java.util.StringJoiner; +import java.util.zip.Deflater; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; @@ -273,27 +274,27 @@ public class ApplicationPackageBuilder { } ByteArrayOutputStream zip = new ByteArrayOutputStream(); try (ZipOutputStream out = new ZipOutputStream(zip)) { - out.putNextEntry(new ZipEntry(dir + "deployment.xml")); - out.write(deploymentSpec()); - out.closeEntry(); - out.putNextEntry(new ZipEntry(dir + "validation-overrides.xml")); - out.write(validationOverrides()); - out.closeEntry(); - out.putNextEntry(new ZipEntry(dir + "search-definitions/test.sd")); - out.write(searchDefinition()); - out.closeEntry(); - out.putNextEntry(new ZipEntry(dir + "build-meta.json")); - out.write(buildMeta(compileVersion)); - out.closeEntry(); - out.putNextEntry(new ZipEntry(dir + "security/clients.pem")); - out.write(X509CertificateUtils.toPem(trustedCertificates).getBytes(UTF_8)); - out.closeEntry(); + out.setLevel(Deflater.NO_COMPRESSION); // This is for testing purposes so we skip compression for performance + writeZipEntry(out, dir + "deployment.xml", deploymentSpec()); + writeZipEntry(out, dir + "validation-overrides.xml", validationOverrides()); + writeZipEntry(out, dir + "search-definitions/test.sd", searchDefinition()); + writeZipEntry(out, dir + "build-meta.json", buildMeta(compileVersion)); + if (!trustedCertificates.isEmpty()) { + writeZipEntry(out, dir + "security/clients.pem", X509CertificateUtils.toPem(trustedCertificates).getBytes(UTF_8)); + } } catch (IOException e) { throw new UncheckedIOException(e); } return new ApplicationPackage(zip.toByteArray()); } + private void writeZipEntry(ZipOutputStream out, String name, byte[] content) throws IOException { + ZipEntry entry = new ZipEntry(name); + out.putNextEntry(entry); + out.write(content); + out.closeEntry(); + } + private static String asIso8601Date(Instant instant) { return new SimpleDateFormat("yyyy-MM-dd").format(Date.from(instant)); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index 4203051965b..098282e4e89 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -64,7 +64,6 @@ import java.util.Set; import java.util.UUID; import java.util.logging.Level; import java.util.stream.Collectors; -import java.util.stream.IntStream; import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow; import static com.yahoo.config.provision.NodeResources.StorageType.remote; @@ -168,18 +167,18 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer public void addNodes(List<ZoneId> zones, List<SystemApplication> applications) { for (ZoneId zone : zones) { for (SystemApplication application : applications) { - List<Node> nodes = IntStream.rangeClosed(1, 3) - .mapToObj(i -> new Node.Builder() - .hostname(HostName.from("node-" + i + "-" + application.id().application() - .value() + "-" + zone.value())) - .state(Node.State.active) - .type(application.nodeType()) - .owner(application.id()) - .currentVersion(initialVersion).wantedVersion(initialVersion) - .currentOsVersion(Version.emptyVersion).wantedOsVersion(Version.emptyVersion) - .build()) - .collect(Collectors.toList()); - nodeRepository().putNodes(zone, nodes); + for (int i = 1; i <= 3; i++) { + Node node = new Node.Builder() + .hostname(HostName.from("node-" + i + "-" + application.id().application() + .value() + "-" + zone.value())) + .state(Node.State.active) + .type(application.nodeType()) + .owner(application.id()) + .currentVersion(initialVersion).wantedVersion(initialVersion) + .currentOsVersion(Version.emptyVersion).wantedOsVersion(Version.emptyVersion) + .build(); + nodeRepository().putNode(zone, node); + } convergeServices(application.id(), zone); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java index afb56f10c38..4079591730d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java @@ -59,9 +59,14 @@ public class NodeRepositoryMock implements NodeRepository { /** Add or update given nodes in zone */ public void putNodes(ZoneId zone, List<Node> nodes) { - nodeRepository.putIfAbsent(zone, new HashMap<>()); - nodeRepository.get(zone).putAll(nodes.stream().collect(Collectors.toMap(Node::hostname, - Function.identity()))); + Map<HostName, Node> zoneNodes = nodeRepository.computeIfAbsent(zone, (k) -> new HashMap<>()); + for (var node : nodes) { + zoneNodes.put(node.hostname(), node); + } + } + + public void putNode(ZoneId zone, Node node) { + nodeRepository.computeIfAbsent(zone, (k) -> new HashMap<>()).put(node.hostname(), node); } public void putApplication(ZoneId zone, Application application) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index 047a4461f7c..79b564eee52 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -66,9 +66,9 @@ public class RoutingPoliciesTest { private static final ZoneId zone3 = ZoneId.from("prod", "aws-us-east-1a"); private static final ZoneId zone4 = ZoneId.from("prod", "aws-us-east-1b"); - private final ApplicationPackage applicationPackage = applicationPackageBuilder().region(zone1.region()) - .region(zone2.region()) - .build(); + private static final ApplicationPackage applicationPackage = applicationPackageBuilder().region(zone1.region()) + .region(zone2.region()) + .build(); @Test public void global_routing_policies() { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index 77ce86f1664..4dd283cf5d7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -127,11 +127,7 @@ public class VersionStatusTest { @Test public void testVersionStatusAfterApplicationUpdates() { DeploymentTester tester = new DeploymentTester(); - ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .upgradePolicy("default") - .region("us-west-1") - .region("us-east-3") - .build(); + ApplicationPackage applicationPackage = applicationPackage("default"); Version version1 = new Version("6.2"); Version version2 = new Version("6.3"); @@ -216,10 +212,9 @@ public class VersionStatusTest { Version version0 = new Version("6.2"); tester.controllerTester().upgradeSystem(version0); tester.upgrader().maintain(); - var builder = new ApplicationPackageBuilder().region("us-west-1").region("us-east-3"); // Setup applications - all running on version0 - ApplicationPackage canaryPolicy = builder.upgradePolicy("canary").build(); + ApplicationPackage canaryPolicy = applicationPackage("canary"); var canary0 = tester.newDeploymentContext("tenant1", "canary0", "default") .submit(canaryPolicy) .deploy(); @@ -230,7 +225,7 @@ public class VersionStatusTest { .submit(canaryPolicy) .deploy(); - ApplicationPackage defaultPolicy = builder.upgradePolicy("default").build(); + ApplicationPackage defaultPolicy = applicationPackage("default"); var default0 = tester.newDeploymentContext("tenant1", "default0", "default") .submit(defaultPolicy) .deploy(); @@ -262,7 +257,7 @@ public class VersionStatusTest { .submit(defaultPolicy) .deploy(); - ApplicationPackage conservativePolicy = builder.upgradePolicy("conservative").build(); + ApplicationPackage conservativePolicy = applicationPackage("conservative"); var conservative0 = tester.newDeploymentContext("tenant1", "conservative0", "default") .submit(conservativePolicy) .deploy(); @@ -388,10 +383,10 @@ public class VersionStatusTest { Version version0 = new Version("6.2"); tester.controllerTester().upgradeSystem(version0); tester.upgrader().maintain(); - var appPackage = new ApplicationPackageBuilder().region("us-west-1").region("us-east-3").upgradePolicy("canary"); + var appPackage = applicationPackage("canary"); var canary0 = tester.newDeploymentContext("tenant1", "canary0", "default") - .submit(appPackage.build()) + .submit(appPackage) .deploy(); assertEquals("All applications running on this version: High", @@ -537,13 +532,13 @@ public class VersionStatusTest { Version version0 = Version.fromString("7.1"); tester.controllerTester().upgradeSystem(version0); var canary0 = tester.newDeploymentContext("tenant1", "canary0", "default") - .submit(new ApplicationPackageBuilder().upgradePolicy("canary").region("us-west-1").build()) + .submit(applicationPackage("canary")) .deploy(); var canary1 = tester.newDeploymentContext("tenant1", "canary1", "default") - .submit(new ApplicationPackageBuilder().upgradePolicy("canary").region("us-west-1").build()) + .submit(applicationPackage("canary")) .deploy(); var default0 = tester.newDeploymentContext("tenant1", "default0", "default") - .submit(new ApplicationPackageBuilder().upgradePolicy("default").region("us-west-1").build()) + .submit(applicationPackage("default")) .deploy(); tester.controllerTester().computeVersionStatus(); assertSame(Confidence.high, tester.controller().readVersionStatus().version(version0).confidence()); @@ -609,12 +604,11 @@ public class VersionStatusTest { public void testStatusIncludesIncompleteUpgrades() { var tester = new DeploymentTester().atMondayMorning(); var version0 = Version.fromString("7.1"); - var applicationPackage = new ApplicationPackageBuilder().region("us-west-1").build(); // Application deploys on initial version tester.controllerTester().upgradeSystem(version0); var context = tester.newDeploymentContext("tenant1", "default0", "default"); - context.submit(applicationPackage).deploy(); + context.submit(applicationPackage("default")).deploy(); // System is upgraded and application starts upgrading to next version var version1 = Version.fromString("7.2"); @@ -688,4 +682,32 @@ public class VersionStatusTest { .orElseThrow(() -> new IllegalArgumentException("Expected to find version: " + version)); } + private static final ApplicationPackage canaryApplicationPackage = + new ApplicationPackageBuilder().upgradePolicy("canary") + .region("us-west-1") + .region("us-east-3") + .build(); + + private static final ApplicationPackage defaultApplicationPackage = + new ApplicationPackageBuilder().upgradePolicy("default") + .region("us-west-1") + .region("us-east-3") + .build(); + + private static final ApplicationPackage conservativeApplicationPackage = + new ApplicationPackageBuilder().upgradePolicy("conservative") + .region("us-west-1") + .region("us-east-3") + .build(); + + /** Returns empty prebuilt applications for efficiency */ + private ApplicationPackage applicationPackage(String upgradePolicy) { + switch (upgradePolicy) { + case "canary" : return canaryApplicationPackage; + case "default" : return defaultApplicationPackage; + case "conservative" : return conservativeApplicationPackage; + default : throw new IllegalArgumentException("No upgrade policy '" + upgradePolicy + "'"); + } + } + } |