diff options
| author | Håkon Hallingstad <hakon@oath.com> | 2017-09-26 09:01:39 +0200 |
|---|---|---|
| committer | Håkon Hallingstad <hakon@oath.com> | 2017-09-26 09:01:39 +0200 |
| commit | 971e4232e6f7440df3aa53a1dce54c8739074473 (patch) | |
| tree | d87f33276e834cc8dcb5ba7c2e40473cae12ead6 | |
| parent | 112e023607f8a20a006dadfa07f29b36c3040860 (diff) | |
| parent | eba935248e99605a42eaccbdebfd53dea7e983ef (diff) | |
Merge remote-tracking branch 'origin/master' into hakon/move-supermodel-to-config-model-api
49 files changed, 553 insertions, 338 deletions
diff --git a/bootstrap.sh b/bootstrap.sh index 126359b3951..075da74b7c7 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -45,7 +45,7 @@ $top/dist/getversion.pl -M $top > $top/dist/vtag.map # The 'full' mode also builds modules needed by C++ tests. # must install parent pom first: -echo "Downloading all dependencies. This make take a few of minutes with an empty Maven cache." +echo "Downloading all dependencies. This may take a few minutes with an empty Maven cache." mvn_install -N # and build plugins first: diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index 32a9158dbcf..77029360593 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -217,6 +217,7 @@ public final class ContainerCluster addSimpleComponent(AccessLog.class); // TODO better modelling addSimpleComponent(ThreadPoolProvider.class); + addSimpleComponent(com.yahoo.concurrent.classlock.ClassLocking.class); addSimpleComponent("com.yahoo.jdisc.http.filter.SecurityFilterInvoker"); addSimpleComponent(SIMPLE_LINGUISTICS_PROVIDER); addSimpleComponent("com.yahoo.container.jdisc.SslKeyStoreFactoryProvider"); diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java index eff18d02293..47bb7052c97 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java @@ -2,6 +2,8 @@ package com.yahoo.config.subscription.impl; import java.text.SimpleDateFormat; +import java.time.Duration; +import java.time.Instant; import java.util.TimeZone; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledThreadPoolExecutor; @@ -39,9 +41,9 @@ public class JRTConfigRequester implements RequestWaiter { private int fatalFailures = 0; // independent of transientFailures private int transientFailures = 0; // independent of fatalFailures private final ScheduledThreadPoolExecutor scheduler = new ScheduledThreadPoolExecutor(1, new JRTSourceThreadFactory()); - private long suspendWarned; - private long noApplicationWarned; - private static final long delayBetweenWarnings = 60000; //ms + private Instant suspendWarningLogged; + private Instant noApplicationWarningLogged; + private static final Duration delayBetweenWarnings = Duration.ofSeconds(60); private final ConnectionPool connectionPool; static final float randomFraction = 0.2f; /* Time to be added to server timeout to create client timeout. This is the time allowed for the server to respond after serverTimeout has elapsed. */ @@ -146,12 +148,11 @@ public class JRTConfigRequester implements RequestWaiter { break; case ErrorCode.APPLICATION_NOT_LOADED: case ErrorCode.UNKNOWN_VESPA_VERSION: - final long now = System.currentTimeMillis(); - if (noApplicationWarned < (now - delayBetweenWarnings)) { + if (noApplicationWarningLogged.isBefore(Instant.now().minus(delayBetweenWarnings))) { log.log(LogLevel.WARNING, "Request callback failed: " + ErrorCode.getName(jrtReq.errorCode()) + ". Connection spec: " + connection.getAddress() + ", error message: " + jrtReq.errorMessage()); - noApplicationWarned = now; + noApplicationWarningLogged = Instant.now(); } break; default: @@ -197,12 +198,11 @@ public class JRTConfigRequester implements RequestWaiter { JRTConfigSubscription<ConfigInstance> sub, long delay, Connection connection) { - long now = System.currentTimeMillis(); transientFailures++; - if (suspendWarned < (now - delayBetweenWarnings)) { + if (suspendWarningLogged.isBefore(Instant.now().minus(delayBetweenWarnings))) { log.log(LogLevel.INFO, "Connection to " + connection.getAddress() + " failed or timed out, clients will keep existing config, will keep trying."); - suspendWarned = now; + suspendWarningLogged = Instant.now(); } if (sub.getState() != ConfigSubscription.State.OPEN) return; scheduleNextRequest(jrtReq, sub, delay, calculateErrorTimeout()); @@ -240,7 +240,8 @@ public class JRTConfigRequester implements RequestWaiter { // Reset counters pertaining to error handling here fatalFailures = 0; transientFailures = 0; - suspendWarned = 0; + suspendWarningLogged = Instant.MIN; + noApplicationWarningLogged = Instant.MIN; connection.setSuccess(); sub.setLastCallBackOKTS(System.currentTimeMillis()); if (jrtReq.hasUpdatedGeneration()) { @@ -293,7 +294,10 @@ public class JRTConfigRequester implements RequestWaiter { } public void close() { - suspendWarned = System.currentTimeMillis(); // Avoid printing warnings after this + // Fake that we have logged to avoid printing warnings after this + suspendWarningLogged = Instant.now(); + noApplicationWarningLogged = Instant.now(); + connectionPool.close(); scheduler.shutdown(); } diff --git a/container-dependencies-enforcer/pom.xml b/container-dependencies-enforcer/pom.xml new file mode 100644 index 00000000000..6aff62248d0 --- /dev/null +++ b/container-dependencies-enforcer/pom.xml @@ -0,0 +1,126 @@ +<?xml version="1.0"?> +<!-- Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> +<project xmlns="http://maven.apache.org/POM/4.0.0" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 + http://maven.apache.org/xsd/maven-4.0.0.xsd"> + <modelVersion>4.0.0</modelVersion> + <groupId>com.yahoo.vespa</groupId> + <artifactId>container-dependencies-enforcer</artifactId> + <version>6-SNAPSHOT</version> + <packaging>pom</packaging> + + <dependencies> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>application</artifactId> + <version>${project.version}</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>container-dev</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + </dependencies> + + <build> + <plugins> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-enforcer-plugin</artifactId> + <version>3.0.0-M1</version> + <executions> + <execution> + <!-- To allow running 'mvn enforcer:enforce' from the command line --> + <id>default-cli</id> + <goals> + <goal>enforce</goal> + </goals> + <configuration> + <rules> + <bannedDependencies> + <excludes> + <!-- Only allow explicitly listed deps in provided scope --> + <exclude>*:*:*:jar:provided:*</exclude> + </excludes> + <includes> + <include>com.yahoo.vespa</include> + <include>aopalliance:aopalliance:1.0:jar:provided</include> + <include>com.fasterxml.jackson.core:jackson-annotations:2.8.3:jar:provided</include> + <include>com.fasterxml.jackson.core:jackson-core:2.8.3:jar:provided</include> + <include>com.fasterxml.jackson.core:jackson-databind:2.8.3:jar:provided</include> + <include>com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.8.3:jar:provided</include> + <include>com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:2.5.4:jar:provided</include> + <include>com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:2.5.4:jar:provided</include> + <include>com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.5.4:jar:provided</include> + <include>com.google.code.findbugs:annotations:1.3.9:jar:provided</include> + <include>com.google.code.findbugs:jsr305:1.3.9:jar:provided</include> + <include>com.google.guava:guava:18.0:jar:provided</include> + <include>com.google.inject.extensions:guice-assistedinject:3.0:jar:provided</include> + <include>com.google.inject.extensions:guice-multibindings:3.0:jar:provided</include> + <include>com.google.inject:guice:3.0:jar:provided:no_aop</include> + <include>commons-codec:commons-codec:1.4:jar:provided</include> + <include>commons-daemon:commons-daemon:1.0.3:jar:provided</include> + <include>commons-logging:commons-logging:1.1.1:jar:provided</include> + <include>javax.annotation:javax.annotation-api:1.2:jar:provided</include> + <include>javax.inject:javax.inject:1:jar:provided</include> + <include>javax.servlet:javax.servlet-api:3.1.0:jar:provided</include> + <include>javax.validation:validation-api:1.1.0.Final:jar:provided</include> + <include>javax.ws.rs:javax.ws.rs-api:2.0.1:jar:provided</include> + <include>net.jcip:jcip-annotations:1.0:jar:provided</include> + <include>net.jpountz.lz4:lz4:1.3.0:jar:provided</include> + <include>org.apache.felix:org.apache.felix.framework:4.2.1:jar:provided</include> + <include>org.apache.felix:org.apache.felix.log:1.0.1:jar:provided</include> + <include>org.apache.felix:org.apache.felix.main:4.2.1:jar:provided</include> + <include>org.apache.httpcomponents:httpclient:4.3.6:jar:provided</include> + <include>org.apache.httpcomponents:httpcore:4.3.3:jar:provided</include> + <include>org.eclipse.jetty:jetty-http:9.4.6.v20170531:jar:provided</include> + <include>org.eclipse.jetty:jetty-io:9.4.6.v20170531:jar:provided</include> + <include>org.eclipse.jetty:jetty-util:9.4.6.v20170531:jar:provided</include> + <include>org.glassfish.hk2.external:aopalliance-repackaged:2.5.0-b05:jar:provided</include> + <include>org.glassfish.hk2.external:javax.inject:2.5.0-b05:jar:provided</include> + <include>org.glassfish.hk2:hk2-api:2.5.0-b05:jar:provided</include> + <include>org.glassfish.hk2:hk2-locator:2.5.0-b05:jar:provided</include> + <include>org.glassfish.hk2:hk2-utils:2.5.0-b05:jar:provided</include> + <include>org.glassfish.hk2:osgi-resource-locator:1.0.1:jar:provided</include> + <include>org.glassfish.jersey.bundles.repackaged:jersey-guava:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.containers:jersey-container-servlet-core:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.containers:jersey-container-servlet:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.core:jersey-client:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.core:jersey-common:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.core:jersey-server:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.ext:jersey-entity-filtering:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.ext:jersey-proxy-client:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.media:jersey-media-jaxb:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.media:jersey-media-json-jackson:2.23.2:jar:provided</include> + <include>org.glassfish.jersey.media:jersey-media-multipart:2.23.2:jar:provided</include> + <include>org.javassist:javassist:3.20.0-GA:jar:provided</include> + <include>org.json:json:20090211:jar:provided</include> + <include>org.jvnet.mimepull:mimepull:1.9.6:jar:provided</include> + <include>org.scala-lang.modules:scala-parser-combinators_2.11:1.0.1:jar:provided</include> + <include>org.slf4j:jcl-over-slf4j:1.7.5:jar:provided</include> + <include>org.slf4j:log4j-over-slf4j:1.7.5:jar:provided</include> + <include>org.slf4j:slf4j-api:1.7.5:jar:provided</include> + <include>org.slf4j:slf4j-jdk14:1.7.5:jar:provided</include> + <include>xml-apis:xml-apis:1.4.01:jar:provided</include> + + <!-- TODO: Remove the deps below when all environments use maven >= 3.3 --> + <include>org.scala-lang:scala-library:2.11.4:jar:provided</include> + <include>commons-lang:commons-lang:2.6:jar:provided</include> + <include>org.apache.commons:commons-exec:1.3:jar:provided</include> + <include>org.antlr:antlr-runtime:3.5.2:jar:provided</include> + <include>log4j:log4j:1.2.16:jar:provided</include> + <include>commons-collections:commons-collections:3.2.1:jar:provided</include> + </includes> + </bannedDependencies> + </rules> + <fail>true</fail> + </configuration> + </execution> + </executions> + </plugin> + </plugins> + </build> +</project> diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java index fa7a48c85c2..5af4523b579 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java @@ -5,15 +5,12 @@ import com.google.common.collect.ImmutableList; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.RegionName; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; import java.time.Instant; import java.util.Comparator; import java.util.List; -import java.util.Optional; import java.util.stream.Stream; /** @@ -130,12 +127,7 @@ public class ApplicationList { return listOf(list.stream().filter(a -> !hasRunningJob(a, change))); } - /** Returns the subset of applications which deploys to given environment and region */ - public ApplicationList deploysTo(Environment environment, RegionName region) { - return listOf(list.stream().filter(a -> a.deploymentSpec().includes(environment, Optional.of(region)))); - } - - // ----------------------------------- Internal helpers + // ----------------------------------- Internal helpers private static boolean isUpgradingTo(Version version, Application application) { if ( ! (application.deploying().isPresent()) ) return false; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java index 6c57c9423ff..2c048bfa3ce 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.JobStatus; import java.time.Duration; @@ -15,6 +16,7 @@ import java.util.Optional; * Attempts redeployment of failed jobs and deployments. * * @author bratseth + * @author mpolden */ public class FailureRedeployer extends Maintainer { @@ -26,7 +28,9 @@ public class FailureRedeployer extends Maintainer { @Override public void maintain() { - List<Application> applications = controller().applications().asList(); + List<Application> applications = ApplicationList.from(controller().applications().asList()) + .notPullRequest() + .asList(); retryFailingJobs(applications); retryStuckJobs(applications); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index 22d0a56c367..a4d50d0c150 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -2,10 +2,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; -import com.yahoo.component.Vtag; import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.RegionName; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.application.ApplicationList; @@ -14,7 +11,6 @@ import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import com.yahoo.yolean.Exceptions; import java.time.Duration; -import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; @@ -38,19 +34,13 @@ public class Upgrader extends Maintainer { public void maintain() { VespaVersion target = controller().versionStatus().version(controller().systemVersion()); if (target == null) return; // we don't have information about the current system version at this time - - // TODO: Remove corp-prod special casing when corp-prod and main are upgraded at the same time - if (Vtag.currentVersion.isAfter(target.versionNumber())) { - upgrade(applications().deploysTo(Environment.prod, RegionName.from("corp-us-east-1")).with(UpgradePolicy.canary), - Vtag.currentVersion); - } switch (target.confidence()) { case broken: ApplicationList toCancel = applications().upgradingTo(target.versionNumber()) .without(UpgradePolicy.canary); if (toCancel.isEmpty()) break; - log.info("Version " + target.versionNumber() + " is broken, cancelling all upgrades"); + log.info("Version " + target.versionNumber() + " is broken, cancelling upgrades of non-canaries"); cancelUpgradesOf(toCancel); break; case low: diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java index 38ddd8a4a1b..fd7a3605766 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java @@ -296,4 +296,30 @@ public class FailureRedeployerTest { assertFalse("Change deployed", tester.application(application.id()).deploying().isPresent()); } + @Test + public void ignoresPullRequestInstances() throws Exception { + DeploymentTester tester = new DeploymentTester(); + tester.controllerTester().getZoneRegistryMock().setSystem(SystemName.cd); + + // Current system version, matches version in test data + Version version = Version.fromString("6.42.1"); + tester.configServerClientMock().setDefaultConfigServerVersion(version); + tester.updateVersionStatus(version); + assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); + + // Load test data data + ApplicationSerializer serializer = new ApplicationSerializer(); + byte[] json = Files.readAllBytes(Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/pr-instance-with-dead-locked-job.json")); + Slime slime = SlimeUtils.jsonToSlime(json); + Application application = serializer.fromSlime(slime); + + try (Lock lock = tester.controller().applications().lock(application.id())) { + tester.controller().applications().store(application, lock); + } + + // Failure redeployer does not restart deployment + tester.failureRedeployer().maintain(); + assertTrue("No jobs scheduled", tester.buildSystem().jobs().isEmpty()); + } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index f7af3ba7a3f..3046a89efe6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -2,11 +2,9 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; -import com.yahoo.component.Vtag; import com.yahoo.config.provision.Environment; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; @@ -362,39 +360,4 @@ public class UpgraderTest { assertEquals(VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence()); } - // TODO: Remove when corp-prod special casing is no longer needed - @Test - public void upgradesCanariesToControllerVersion() { - DeploymentTester tester = new DeploymentTester(); - ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .upgradePolicy("canary") - .environment(Environment.prod) - .region("corp-us-east-1") - .build(); - - Version version = Version.fromString("5.0"); // Lower version than controller (6.10) - tester.updateVersionStatus(version); - - // Application is on 5.0 - Application app = tester.createApplication("app1", "tenant1", 1, 11L); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionCorpUsEast1); - - // Canary in prod.corp-us-east-1 is upgraded to controller version - tester.upgrader().maintain(); - assertEquals("Upgrade started", 1, tester.buildSystem().jobs().size()); - assertEquals(Vtag.currentVersion, ((Change.VersionChange) tester.application(app.id()).deploying().get()).version()); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionCorpUsEast1); - - // System is upgraded to newer version, no upgrade triggered for canary as version is lower than controller - version = Version.fromString("5.1"); - tester.updateVersionStatus(version); - tester.upgrader().maintain(); - assertTrue("No more jobs triggered", tester.buildSystem().jobs().isEmpty()); - } - } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/pr-instance-with-dead-locked-job.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/pr-instance-with-dead-locked-job.json new file mode 100644 index 00000000000..32d34edd576 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/pr-instance-with-dead-locked-job.json @@ -0,0 +1,23 @@ +{ + "id": "tenant1:app1:default-pr1", + "deploymentSpecField": "<deployment version='1.0'>\n <test />\n <staging />\n <prod global-service-id=\"default\">\n <region active=\"true\">us-central-1</region>\n </prod>\n</deployment>\n", + "validationOverrides": "<deployment version='1.0'/>", + "deployments": [], + "deploymentJobs": { + "projectId": 0, + "jobStatus": [ + { + "jobType": "system-test", + "lastTriggered": { + "version": "6.42.1", + "revision": { + "applicationPackageHash": "dead" + }, + "upgrade": false, + "at": 1499075576005 + } + } + ] + }, + "outstandingChangeField": false +} diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/RunSystemTests.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/RunSystemTests.java index 915b3b53867..9613470a735 100644 --- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/RunSystemTests.java +++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/RunSystemTests.java @@ -155,7 +155,7 @@ public class RunSystemTests { logger.info("Pulling " + vespaBaseImage.asString() + " (This may take a while)"); while (docker.pullImageAsyncIfNeeded(vespaBaseImage)) { Thread.sleep(5000); - }; + } } Path systestBuildDirectory = pathToVespaRepoInHost.resolve("docker-api/src/test/resources/systest/"); diff --git a/docker/run/run-vespa-internal.sh b/docker/run/run-vespa-internal.sh index 074c1887394..dce0d4dff97 100755 --- a/docker/run/run-vespa-internal.sh +++ b/docker/run/run-vespa-internal.sh @@ -17,6 +17,8 @@ chown -R vespa:vespa /opt/vespa export VESPA_CONFIG_SERVERS=$(hostname) /opt/vespa/bin/vespa-start-configserver +# Give config server some time to come up before starting services +sleep 5 /opt/vespa/bin/vespa-start-services # Print log forever diff --git a/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCSendV2.java b/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCSendV2.java index 8cc0b73ae30..c6de8c8628f 100644 --- a/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCSendV2.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/RPCSendV2.java @@ -34,7 +34,7 @@ public class RPCSendV2 extends RPCSend { private final static String METHOD_NAME = "mbus.slime"; private final static String METHOD_PARAMS = "bixbix"; private final static String METHOD_RETURN = "bixbix"; - private final Compressor compressor = new Compressor(CompressionType.LZ4, 3, 90, 1024); + private final Compressor compressor = new Compressor(CompressionType.LZ4, 3, 0.90, 1024); @Override protected String getReturnSpec() { return METHOD_RETURN; } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java index cf70963eee1..6bc3b6d2a46 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java @@ -60,6 +60,11 @@ public interface NodeAdmin { Map<String, Object> debugInfo(); /** + * Start node-admin schedulers. + */ + void start(); + + /** * Stop the NodeAgent. Will not delete the storage or stop the container. */ void stop(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java index f227a166034..ddb04f1249d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java @@ -47,6 +47,7 @@ public class NodeAdminImpl implements NodeAdmin { private final DockerOperations dockerOperations; private final Function<String, NodeAgent> nodeAgentFactory; private final StorageMaintainer storageMaintainer; + private final AclMaintainer aclMaintainer; private final Clock clock; private boolean previousWantFrozen; @@ -67,6 +68,7 @@ public class NodeAdminImpl implements NodeAdmin { this.dockerOperations = dockerOperations; this.nodeAgentFactory = nodeAgentFactory; this.storageMaintainer = storageMaintainer; + this.aclMaintainer = aclMaintainer; this.clock = clock; this.previousWantFrozen = true; @@ -76,18 +78,6 @@ public class NodeAdminImpl implements NodeAdmin { Dimensions dimensions = new Dimensions.Builder().add("role", "docker").build(); this.numberOfContainersInLoadImageState = metricReceiver.declareGauge(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "nodes.image.loading"); this.numberOfUnhandledExceptionsInNodeAgent = metricReceiver.declareCounter(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "nodes.unhandled_exceptions"); - - metricsScheduler.scheduleAtFixedRate(() -> { - try { - nodeAgents.values().forEach(NodeAgent::updateContainerNodeMetrics); - } catch (Throwable e) { - logger.warning("Metric fetcher scheduler failed", e); - } - }, 0, 55, TimeUnit.SECONDS); - - aclScheduler.scheduleWithFixedDelay(() -> { - if (!isFrozen()) aclMaintainer.run(); - }, 30, 60, TimeUnit.SECONDS); } @Override @@ -179,6 +169,21 @@ public class NodeAdminImpl implements NodeAdmin { } @Override + public void start() { + metricsScheduler.scheduleAtFixedRate(() -> { + try { + nodeAgents.values().forEach(NodeAgent::updateContainerNodeMetrics); + } catch (Throwable e) { + logger.warning("Metric fetcher scheduler failed", e); + } + }, 0, 55, TimeUnit.SECONDS); + + aclScheduler.scheduleWithFixedDelay(() -> { + if (!isFrozen()) aclMaintainer.run(); + }, 30, 60, TimeUnit.SECONDS); + } + + @Override public void stop() { metricsScheduler.shutdown(); aclScheduler.shutdown(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java index d1f23b13e6c..6bfa34388ef 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java @@ -2,6 +2,9 @@ package com.yahoo.vespa.hosted.node.admin.nodeadmin; import com.yahoo.concurrent.ThreadFactoryFactory; +import com.yahoo.concurrent.classlock.ClassLock; +import com.yahoo.concurrent.classlock.ClassLocking; +import com.yahoo.concurrent.classlock.LockInterruptException; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec; import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; @@ -19,6 +22,7 @@ import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -48,16 +52,17 @@ public class NodeAdminStateUpdater { private final Logger log = Logger.getLogger(NodeAdminStateUpdater.class.getName()); private final ScheduledExecutorService specVerifierScheduler = Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory("specverifier")); - private Thread loopThread; + private final Thread loopThread; private final NodeRepository nodeRepository; private final Orchestrator orchestrator; - private final StorageMaintainer storageMaintainer; private final NodeAdmin nodeAdmin; private final Clock clock; private final String dockerHostHostName; private final Duration nodeAdminConvergeStateInterval; + private final ClassLocking classLocking; + private Optional<ClassLock> classLock; private Instant lastTick; public NodeAdminStateUpdater( @@ -67,15 +72,41 @@ public class NodeAdminStateUpdater { NodeAdmin nodeAdmin, String dockerHostHostName, Clock clock, - Duration nodeAdminConvergeStateInterval) { + Duration nodeAdminConvergeStateInterval, + ClassLocking classLocking) { + log.info(objectToString() + ": Creating object"); this.nodeRepository = nodeRepository; this.orchestrator = orchestrator; - this.storageMaintainer = storageMaintainer; this.nodeAdmin = nodeAdmin; this.dockerHostHostName = dockerHostHostName; this.clock = clock; this.nodeAdminConvergeStateInterval = nodeAdminConvergeStateInterval; + this.classLocking = classLocking; this.lastTick = clock.instant(); + + this.loopThread = new Thread(() -> { + log.info(objectToString() + ": Acquiring lock"); + try { + classLock = Optional.of(classLocking.lockWhile(NodeAdminStateUpdater.class, () -> !terminated.get())); + } catch (LockInterruptException e) { + classLock = Optional.empty(); + return; + } + + log.info(objectToString() + ": Starting threads and schedulers"); + nodeAdmin.start(); + specVerifierScheduler.scheduleWithFixedDelay(() -> + updateHardwareDivergence(storageMaintainer), 5, 60, TimeUnit.MINUTES); + + while (! terminated.get()) { + tick(); + } + }); + this.loopThread.setName("tick-NodeAdminStateUpdater"); + } + + private String objectToString() { + return this.getClass().getSimpleName() + "@" + Integer.toString(System.identityHashCode(this)); } public enum State { RESUMED, SUSPENDED_NODE_ADMIN, SUSPENDED} @@ -257,28 +288,20 @@ public class NodeAdminStateUpdater { } public void start() { - if (loopThread != null) { - throw new RuntimeException("Can not restart NodeAdminStateUpdater"); - } - - loopThread = new Thread(() -> { - while (! terminated.get()) tick(); - }); - loopThread.setName("tick-NodeAdminStateUpdater"); loopThread.start(); - - specVerifierScheduler.scheduleWithFixedDelay(() -> - updateHardwareDivergence(storageMaintainer), 5, 60, TimeUnit.MINUTES); } public void stop() { - specVerifierScheduler.shutdown(); + log.info(objectToString() + ": Stop called"); if (!terminated.compareAndSet(false, true)) { throw new RuntimeException("Can not re-stop a node agent."); } + classLocking.interrupt(); + // First we need to stop NodeAdminStateUpdater thread to make sure no new NodeAgents are spawned signalWorkToBeDone(); + specVerifierScheduler.shutdown(); do { try { @@ -291,5 +314,11 @@ public class NodeAdminStateUpdater { // Finally, stop NodeAdmin and all the NodeAgents nodeAdmin.stop(); + + classLock.ifPresent(lock -> { + log.info(objectToString() + ": Releasing lock"); + lock.close(); + }); + log.info(objectToString() + ": Stop complete"); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 77348a9dc45..453012e9791 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -81,7 +81,7 @@ public class NodeAgentImpl implements NodeAgent { private int numberOfUnhandledException = 0; private Instant lastConverge; - private Thread loopThread; + private final Thread loopThread; private final ScheduledExecutorService filebeatRestarter = Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory("filebeatrestarter")); @@ -131,6 +131,11 @@ public class NodeAgentImpl implements NodeAgent { this.clock = clock; this.timeBetweenEachConverge = timeBetweenEachConverge; this.lastConverge = clock.instant(); + + this.loopThread = new Thread(() -> { + while (!terminated.get()) tick(); + }); + this.loopThread.setName("tick-" + hostname); } @Override @@ -178,14 +183,6 @@ public class NodeAgentImpl implements NodeAgent { logger.info(message); addDebugMessage(message); - if (loopThread != null) { - throw new RuntimeException("Can not restart a node agent."); - } - - loopThread = new Thread(() -> { - while (!terminated.get()) tick(); - }); - loopThread.setName("tick-" + hostname); loopThread.start(); serviceRestarter = service -> { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProvider.java deleted file mode 100644 index 93a77a13bf9..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProvider.java +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.provider; - -import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater; - -/** - * Class for setting up instances of classes; enables testing. - * - * @author dybis - */ -public interface ComponentsProvider { - NodeAdminStateUpdater getNodeAdminStateUpdater(); -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java index a18325672be..109dbab924c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java @@ -2,7 +2,8 @@ package com.yahoo.vespa.hosted.node.admin.provider; import com.google.inject.Inject; -import com.yahoo.component.AbstractComponent; +import com.yahoo.concurrent.classlock.ClassLocking; +import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.net.HostName; import com.yahoo.system.ProcessExecuter; @@ -27,7 +28,6 @@ import com.yahoo.vespa.hosted.node.admin.util.Environment; import java.time.Clock; import java.time.Duration; import java.util.function.Function; -import java.util.logging.Logger; import static com.yahoo.vespa.defaults.Defaults.getDefaults; @@ -36,19 +36,16 @@ import static com.yahoo.vespa.defaults.Defaults.getDefaults; * * @author dybis */ -public class ComponentsProviderImpl extends AbstractComponent implements ComponentsProvider { +public class NodeAdminProvider implements Provider<NodeAdminStateUpdater> { private static final int WEB_SERVICE_PORT = getDefaults().vespaWebServicePort(); private static final Duration NODE_AGENT_SCAN_INTERVAL = Duration.ofSeconds(30); private static final Duration NODE_ADMIN_CONVERGE_STATE_INTERVAL = Duration.ofSeconds(30); - private final Logger log = Logger.getLogger(ComponentsProviderImpl.class.getName()); private final NodeAdminStateUpdater nodeAdminStateUpdater; @Inject - public ComponentsProviderImpl(Docker docker, MetricReceiverWrapper metricReceiver) { - log.info(objectToString() + ": Creating object"); - + public NodeAdminProvider(Docker docker, MetricReceiverWrapper metricReceiver, ClassLocking classLocking) { Clock clock = Clock.systemUTC(); String dockerHostHostName = HostName.getLocalhost(); ProcessExecuter processExecuter = new ProcessExecuter(); @@ -69,24 +66,18 @@ public class ComponentsProviderImpl extends AbstractComponent implements Compone metricReceiver, clock); nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepository, orchestrator, storageMaintainer, nodeAdmin, - dockerHostHostName, clock, NODE_ADMIN_CONVERGE_STATE_INTERVAL); + dockerHostHostName, clock, NODE_ADMIN_CONVERGE_STATE_INTERVAL, classLocking); + nodeAdminStateUpdater.start(); } @Override - public NodeAdminStateUpdater getNodeAdminStateUpdater() { + public NodeAdminStateUpdater get() { return nodeAdminStateUpdater; } @Override public void deconstruct() { - log.info(objectToString() + ": Stop called"); nodeAdminStateUpdater.stop(); - log.info(objectToString() + ": Stop complete"); - } - - - private String objectToString() { - return this.getClass().getSimpleName() + "@" + Integer.toString(System.identityHashCode(this)); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/restapi/RestApiHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/restapi/RestApiHandler.java index 73c544c8c80..42b36d95374 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/restapi/RestApiHandler.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/restapi/RestApiHandler.java @@ -10,7 +10,6 @@ import com.yahoo.container.logging.AccessLog; import com.yahoo.vespa.hosted.dockerapi.metrics.DimensionMetrics; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater; -import com.yahoo.vespa.hosted.node.admin.provider.ComponentsProvider; import javax.inject.Inject; import javax.ws.rs.core.MediaType; @@ -39,10 +38,10 @@ public class RestApiHandler extends LoggingRequestHandler{ @Inject public RestApiHandler(Executor executor, AccessLog accessLog, - ComponentsProvider componentsProvider, + NodeAdminStateUpdater nodeAdminStateUpdater, MetricReceiverWrapper metricReceiverWrapper) { super(executor, accessLog); - this.refresher = componentsProvider.getNodeAdminStateUpdater(); + this.refresher = nodeAdminStateUpdater; this.metricReceiverWrapper = metricReceiverWrapper; } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java deleted file mode 100644 index 522ad07a558..00000000000 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.integrationTests; - -import com.yahoo.metrics.simple.MetricReceiver; -import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; -import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; -import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; -import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; -import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdmin; -import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminImpl; -import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl; -import com.yahoo.vespa.hosted.node.admin.noderepository.NodeRepository; -import com.yahoo.vespa.hosted.node.admin.orchestrator.Orchestrator; -import com.yahoo.vespa.hosted.node.admin.provider.ComponentsProvider; -import com.yahoo.vespa.hosted.node.admin.util.Environment; - -import java.time.Clock; -import java.time.Duration; -import java.util.function.Function; - -import static org.mockito.Mockito.mock; - -/** - * For setting up test with mocks. - * - * @author dybis - */ -public class ComponentsProviderWithMocks implements ComponentsProvider { - private static final Duration NODE_AGENT_SCAN_INTERVAL = Duration.ofMillis(100); - private static final Duration NODE_ADMIN_CONVERGE_STATE_INTERVAL = Duration.ofMillis(5); - - static final NodeRepository nodeRepositoryMock = mock(NodeRepository.class); - static final Orchestrator orchestratorMock = mock(Orchestrator.class); - static final DockerOperations dockerOperationsMock = mock(DockerOperations.class); - - private final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class); - private final AclMaintainer aclMaintainer = mock(AclMaintainer.class); - private final Environment environment = new Environment.Builder().build(); - private final MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation); - private final Function<String, NodeAgent> nodeAgentFactory = - (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock, dockerOperationsMock, - storageMaintainer, aclMaintainer, environment, Clock.systemUTC(), NODE_AGENT_SCAN_INTERVAL); - private final NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperationsMock, nodeAgentFactory, storageMaintainer, aclMaintainer, mr, Clock.systemUTC()); - private final NodeAdminStateUpdater nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepositoryMock, - orchestratorMock, storageMaintainer, nodeAdmin, "localhost.test.yahoo.com", Clock.systemUTC(), NODE_ADMIN_CONVERGE_STATE_INTERVAL); - - public ComponentsProviderWithMocks() { - nodeAdminStateUpdater.start(); - } - - @Override - public NodeAdminStateUpdater getNodeAdminStateUpdater() { - return nodeAdminStateUpdater; - } -} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java index ab752bbe4c0..b4a5b552738 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.integrationTests; +import com.yahoo.concurrent.classlock.ClassLocking; import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.vespa.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; @@ -66,7 +67,7 @@ public class DockerTester implements AutoCloseable { orchestratorMock, dockerOperations, storageMaintainer, aclMaintainer, environment, clock, NODE_AGENT_SCAN_INTERVAL); nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, storageMaintainer, aclMaintainer, mr, Clock.systemUTC()); nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepositoryMock, orchestratorMock, storageMaintainer, - nodeAdmin, "basehostname", clock, NODE_ADMIN_CONVERGE_STATE_INTERVAL); + nodeAdmin, "basehostname", clock, NODE_ADMIN_CONVERGE_STATE_INTERVAL, new ClassLocking()); nodeAdminStateUpdater.start(); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RunInContainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RunInContainerTest.java index 036a53a9654..23d55bd947c 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RunInContainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/RunInContainerTest.java @@ -3,11 +3,25 @@ package com.yahoo.vespa.hosted.node.admin.integrationTests; import com.yahoo.application.Networking; import com.yahoo.application.container.JDisc; +import com.yahoo.concurrent.classlock.ClassLocking; +import com.yahoo.container.di.componentgraph.Provider; +import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.DockerImage; +import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec; +import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; +import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer; +import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; +import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdmin; +import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminImpl; +import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater; +import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent; +import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl; +import com.yahoo.vespa.hosted.node.admin.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.orchestrator.Orchestrator; import com.yahoo.vespa.hosted.node.admin.orchestrator.OrchestratorException; +import com.yahoo.vespa.hosted.node.admin.util.Environment; import com.yahoo.vespa.hosted.provision.Node; import org.apache.commons.io.IOUtils; import org.apache.http.HttpEntity; @@ -26,15 +40,19 @@ import java.io.IOException; import java.io.StringWriter; import java.net.ServerSocket; import java.nio.charset.StandardCharsets; +import java.time.Clock; +import java.time.Duration; import java.time.Instant; import java.util.Arrays; import java.util.Collections; +import java.util.function.Function; import java.util.logging.Logger; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; /** @@ -42,7 +60,11 @@ import static org.mockito.Mockito.when; */ public class RunInContainerTest { private final Logger logger = Logger.getLogger("RunInContainerTest"); - private final Orchestrator orchestrator = ComponentsProviderWithMocks.orchestratorMock; + + private static final NodeRepository nodeRepositoryMock = mock(NodeRepository.class); + private static final Orchestrator orchestratorMock = mock(Orchestrator.class); + private static final DockerOperations dockerOperationsMock = mock(DockerOperations.class); + private final String parentHostname = "localhost.test.yahoo.com"; private JDisc container; private int port; @@ -58,7 +80,7 @@ public class RunInContainerTest { // To test the initial NodeAdminStateUpdater convergence towards RESUME, orchestrator should // deny permission to resume for parent host, otherwise it'll converge to RESUME before REST // handler comes up - doThrow(new RuntimeException()).when(orchestrator).resume(parentHostname); + doThrow(new RuntimeException()).when(orchestratorMock).resume(parentHostname); port = findRandomOpenPort(); System.out.println("PORT IS " + port); logger.info("PORT IS " + port); @@ -117,22 +139,22 @@ public class RunInContainerTest { @Ignore @Test public void testGetContainersToRunAPi() throws IOException, InterruptedException { - doThrow(new OrchestratorException("Cannot suspend because...")).when(orchestrator).suspend(parentHostname); - when(ComponentsProviderWithMocks.nodeRepositoryMock.getContainersToRun(eq(parentHostname))).thenReturn(Collections.emptyList()); + doThrow(new OrchestratorException("Cannot suspend because...")).when(orchestratorMock).suspend(parentHostname); + when(nodeRepositoryMock.getContainersToRun(eq(parentHostname))).thenReturn(Collections.emptyList()); waitForJdiscContainerToServe(); assertTrue("The initial resume command should fail because it needs to converge first", verifyWithRetries("resume", false)); - doNothing().when(orchestrator).resume(parentHostname); + doNothing().when(orchestratorMock).resume(parentHostname); assertTrue(verifyWithRetries("resume", true)); doThrow(new OrchestratorException("Cannot suspend because...")) - .when(orchestrator).suspend(parentHostname, Collections.singletonList(parentHostname)); + .when(orchestratorMock).suspend(parentHostname, Collections.singletonList(parentHostname)); assertTrue("Should fail because orchestrator does not allow node-admin to suspend", verifyWithRetries("suspend/node-admin", false)); // Orchestrator changes its mind, allows node-admin to suspend - doNothing().when(orchestrator).suspend(parentHostname, Collections.singletonList(parentHostname)); + doNothing().when(orchestratorMock).suspend(parentHostname, Collections.singletonList(parentHostname)); assertTrue(verifyWithRetries("suspend/node-admin", true)); // Lets try to suspend everything now, should be trivial as we have no active containers to stop services at @@ -144,7 +166,7 @@ public class RunInContainerTest { assertTrue(verifyWithRetries("resume", true)); // Lets try the same, but with an active container running on this host - when(ComponentsProviderWithMocks.nodeRepositoryMock.getContainersToRun(eq(parentHostname))).thenReturn( + when(nodeRepositoryMock.getContainersToRun(eq(parentHostname))).thenReturn( Collections.singletonList(new ContainerNodeSpec.Builder() .hostname("host1.test.yahoo.com") .wantedDockerImage(new DockerImage("dockerImage")) @@ -152,7 +174,7 @@ public class RunInContainerTest { .nodeType("tenant") .nodeFlavor("docker") .build())); - doThrow(new OrchestratorException("Cannot suspend because...")).when(orchestrator) + doThrow(new OrchestratorException("Cannot suspend because...")).when(orchestratorMock) .suspend("localhost.test.yahoo.com", Arrays.asList("host1.test.yahoo.com", parentHostname)); // Initially we are denied to suspend because we have to freeze all the node-agents @@ -160,16 +182,16 @@ public class RunInContainerTest { // At this point they should be frozen, but Orchestrator doesn't allow to suspend either the container or the node-admin assertTrue(verifyWithRetries("suspend/node-admin", false)); - doNothing().when(orchestrator) + doNothing().when(orchestratorMock) .suspend("localhost.test.yahoo.com", Arrays.asList("host1.test.yahoo.com", parentHostname)); // Orchestrator successfully suspended everything assertTrue(verifyWithRetries("suspend/node-admin", true)); // Allow stopping services in active nodes - doNothing().when(ComponentsProviderWithMocks.dockerOperationsMock) + doNothing().when(dockerOperationsMock) .trySuspendNode(eq(new ContainerName("host1"))); - doNothing().when(ComponentsProviderWithMocks.dockerOperationsMock) + doNothing().when(dockerOperationsMock) .stopServicesOnNode(eq(new ContainerName("host1"))); assertTrue(verifyWithRetries("suspend", false)); @@ -191,11 +213,49 @@ public class RunInContainerTest { " <handler id=\"com.yahoo.vespa.hosted.node.admin.restapi.RestApiHandler\" bundle=\"node-admin\">\n" + " <binding>http://*/rest/*</binding>\n" + " </handler>\n" + - " <component id=\"node-admin\" class=\"com.yahoo.vespa.hosted.node.admin.integrationTests.ComponentsProviderWithMocks\" bundle=\"node-admin\"/>\n" + + " <component id=\"metric-receiver\" class=\"com.yahoo.vespa.hosted.node.admin.integrationTests.RunInContainerTest$MetricReceiverWrapperMock\" bundle=\"node-admin\"/>\n" + + " <component id=\"node-admin\" class=\"com.yahoo.vespa.hosted.node.admin.integrationTests.RunInContainerTest$NodeAdminProviderWithMocks\" bundle=\"node-admin\"/>\n" + " <http>" + " <server id=\'myServer\' port=\'" + port + "\' />" + " </http>" + " </jdisc>\n" + "</services>\n"; } + + + public static class MetricReceiverWrapperMock extends MetricReceiverWrapper { + public MetricReceiverWrapperMock() { + super(MetricReceiver.nullImplementation); + } + } + + public class NodeAdminProviderWithMocks implements Provider<NodeAdminStateUpdater> { + private final Duration NODE_AGENT_SCAN_INTERVAL = Duration.ofMillis(100); + private final Duration NODE_ADMIN_CONVERGE_STATE_INTERVAL = Duration.ofMillis(5); + + private final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class); + private final AclMaintainer aclMaintainer = mock(AclMaintainer.class); + private final Environment environment = new Environment.Builder().build(); + private final MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation); + private final Function<String, NodeAgent> nodeAgentFactory = + (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock, dockerOperationsMock, + storageMaintainer, aclMaintainer, environment, Clock.systemUTC(), NODE_AGENT_SCAN_INTERVAL); + private final NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperationsMock, nodeAgentFactory, storageMaintainer, aclMaintainer, mr, Clock.systemUTC()); + private final NodeAdminStateUpdater nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepositoryMock, + orchestratorMock, storageMaintainer, nodeAdmin, "localhost.test.yahoo.com", Clock.systemUTC(), NODE_ADMIN_CONVERGE_STATE_INTERVAL, new ClassLocking()); + + public NodeAdminProviderWithMocks() { + nodeAdminStateUpdater.start(); + } + + @Override + public NodeAdminStateUpdater get() { + return nodeAdminStateUpdater; + } + + @Override + public void deconstruct() { + nodeAdminStateUpdater.stop(); + } + } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java index e1501cf59fe..ce062702a3b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java @@ -45,7 +45,7 @@ public class NodeAdminStateUpdaterTest { private final Duration convergeStateInterval = Duration.ofSeconds(30); private final NodeAdminStateUpdater refresher = spy(new NodeAdminStateUpdater( - nodeRepository, orchestrator, storageMaintainer, nodeAdmin, parentHostname, clock, convergeStateInterval)); + nodeRepository, orchestrator, storageMaintainer, nodeAdmin, parentHostname, clock, convergeStateInterval, null)); @Test @@ -941,6 +941,7 @@ <module>container</module> <module>container-core</module> <module>container-accesslogging</module> + <module>container-dependencies-enforcer</module> <module>container-dev</module> <module>container-di</module> <module>container-disc</module> diff --git a/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp b/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp index 2a9885c9648..38d6483f21a 100644 --- a/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp +++ b/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp @@ -234,7 +234,7 @@ struct WeightedSetFactory : SparseVectorFactory { terms.push_back(childFactory.createChild(i, limit)); weights.push_back(default_weight); } - return WeightedSetTermSearch::create(terms, tfmd, weights); + return WeightedSetTermSearch::create(terms, tfmd, weights, MatchData::UP(nullptr)); } }; diff --git a/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp b/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp index 003c9935716..78195f19427 100644 --- a/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp +++ b/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp @@ -7,7 +7,9 @@ #include <vespa/searchlib/query/tree/simplequery.h> #include <vespa/searchlib/queryeval/field_spec.h> #include <vespa/searchlib/queryeval/blueprint.h> +#include <vespa/searchlib/queryeval/weighted_set_term_blueprint.h> #include <vespa/searchlib/queryeval/fake_result.h> +#include <vespa/searchlib/queryeval/emptysearch.h> #include <vespa/searchlib/queryeval/fake_searchable.h> #include <vespa/searchlib/queryeval/fake_requestcontext.h> #include <vespa/searchlib/test/weightedchildrenverifiers.h> @@ -121,7 +123,7 @@ struct MockFixture { mock = new MockSearch(initial); children.push_back(mock); weights.push_back(1); - search.reset(WeightedSetTermSearch::create(children, tfmd, weights)); + search.reset(WeightedSetTermSearch::create(children, tfmd, weights, MatchData::UP(nullptr))); } }; @@ -192,7 +194,7 @@ TEST_F("test Eager Matching Child", MockFixture(5)) { class IteratorChildrenVerifier : public search::test::IteratorChildrenVerifier { private: SearchIterator::UP create(const std::vector<SearchIterator*> &children) const override { - return SearchIterator::UP(WeightedSetTermSearch::create(children, _tfmd, _weights)); + return SearchIterator::UP(WeightedSetTermSearch::create(children, _tfmd, _weights, MatchData::UP(nullptr))); } }; @@ -213,4 +215,45 @@ TEST("verify search iterator conformance with document weight iterator children" verifier.verify(); } +struct VerifyMatchData { + struct MyBlueprint : search::queryeval::SimpleLeafBlueprint { + VerifyMatchData &vmd; + MyBlueprint(VerifyMatchData &vmd_in, FieldSpec spec_in) + : SimpleLeafBlueprint(spec_in), vmd(vmd_in) {} + SearchIterator::UP createLeafSearch(const fef::TermFieldMatchDataArray &tfmda, bool) const override { + EXPECT_EQUAL(tfmda.size(), 1u); + EXPECT_TRUE(tfmda[0] != nullptr); + if (vmd.child_tfmd == nullptr) { + vmd.child_tfmd = tfmda[0]; + } else { + EXPECT_EQUAL(vmd.child_tfmd, tfmda[0]); + } + ++vmd.child_cnt; + return std::make_unique<EmptySearch>(); + } + }; + size_t child_cnt = 0; + TermFieldMatchData *child_tfmd = nullptr; + search::queryeval::Blueprint::UP create(const FieldSpec &spec) { + return std::make_unique<MyBlueprint>(*this, spec); + } +}; + +TEST("require that children get a common (yet separate) term field match data") { + VerifyMatchData vmd; + MatchDataLayout layout; + auto top_handle = layout.allocTermField(42); + FieldSpec top_spec("foo", 42, top_handle); + WeightedSetTermBlueprint blueprint(top_spec); + for (size_t i = 0; i < 5; ++i) { + blueprint.addTerm(vmd.create(blueprint.getNextChildField(top_spec)), 1); + } + auto match_data = layout.createMatchData(); + auto search = blueprint.createSearch(*match_data, true); + auto top_tfmd = match_data->resolveTermField(top_handle); + EXPECT_EQUAL(vmd.child_cnt, 5u); + EXPECT_TRUE(vmd.child_tfmd != nullptr); + EXPECT_NOT_EQUAL(top_tfmd, vmd.child_tfmd); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_weighted_set_blueprint.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_weighted_set_blueprint.cpp index e6c9e9c0590..3ff7db5a184 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_weighted_set_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_weighted_set_blueprint.cpp @@ -6,6 +6,7 @@ #include <vespa/searchlib/query/queryterm.h> #include <vespa/searchlib/common/bitvector.h> #include <vespa/vespalib/stllike/hash_map.h> +#include <vespa/searchlib/fef/matchdatalayout.h> namespace search { @@ -157,11 +158,15 @@ AttributeWeightedSetBlueprint::createLeafSearch(const fef::TermFieldMatchDataArr assert(tfmda.size() == 1); fef::TermFieldMatchData &tfmd = *tfmda[0]; if (strict) { // use generic weighted set search + fef::MatchDataLayout layout; + auto handle = layout.allocTermField(tfmd.getFieldId()); + auto match_data = layout.createMatchData(); + auto child_tfmd = match_data->resolveTermField(handle); std::vector<queryeval::SearchIterator*> children(_contexts.size()); for (size_t i = 0; i < _contexts.size(); ++i) { - children[i] = _contexts[i]->createIterator(&tfmd, true).release(); + children[i] = _contexts[i]->createIterator(child_tfmd, true).release(); } - return queryeval::SearchIterator::UP(queryeval::WeightedSetTermSearch::create(children, tfmd, _weights)); + return queryeval::SearchIterator::UP(queryeval::WeightedSetTermSearch::create(children, tfmd, _weights, std::move(match_data))); } else { // use attribute filter optimization bool isSingleValue = !_attr.hasMultiValue(); bool isString = (_attr.isStringType() && _attr.hasEnum()); diff --git a/searchlib/src/vespa/searchlib/queryeval/iterator_pack.cpp b/searchlib/src/vespa/searchlib/queryeval/iterator_pack.cpp index e50fe57ac41..1f5858c1100 100644 --- a/searchlib/src/vespa/searchlib/queryeval/iterator_pack.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/iterator_pack.cpp @@ -37,12 +37,11 @@ SearchIteratorPack::SearchIteratorPack(const std::vector<SearchIterator*> &child for (auto child: children) { _children.emplace_back(child); } - assert((_children.size() == _childMatch.size()) || - (_childMatch.empty() && (_md.get() == nullptr))); + assert((_children.size() == _childMatch.size()) || _childMatch.empty()); } -SearchIteratorPack::SearchIteratorPack(const std::vector<SearchIterator*> &children) - : SearchIteratorPack(children, std::vector<fef::TermFieldMatchData*>(), MatchDataUP()) +SearchIteratorPack::SearchIteratorPack(const std::vector<SearchIterator*> &children, MatchDataUP md) + : SearchIteratorPack(children, std::vector<fef::TermFieldMatchData*>(), MatchDataUP(std::move(md))) { } std::unique_ptr<BitVector> diff --git a/searchlib/src/vespa/searchlib/queryeval/iterator_pack.h b/searchlib/src/vespa/searchlib/queryeval/iterator_pack.h index 20a3a57f34a..58c774e0903 100644 --- a/searchlib/src/vespa/searchlib/queryeval/iterator_pack.h +++ b/searchlib/src/vespa/searchlib/queryeval/iterator_pack.h @@ -29,7 +29,7 @@ public: const std::vector<fef::TermFieldMatchData*> &childMatch, MatchDataUP md); - explicit SearchIteratorPack(const std::vector<SearchIterator*> &children); + SearchIteratorPack(const std::vector<SearchIterator*> &children, MatchDataUP md); uint32_t get_docid(uint32_t ref) const { return _children[ref]->getDocId(); diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp index 0c9af1d3e25..fc68c48a247 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp @@ -9,6 +9,8 @@ namespace search::queryeval { WeightedSetTermBlueprint::WeightedSetTermBlueprint(const FieldSpec &field) : ComplexLeafBlueprint(field), _estimate(), + _layout(), + _children_field(field.getName(), field.getFieldId(), _layout.allocTermField(field.getFieldId()), false), _weights(), _terms() { @@ -40,18 +42,18 @@ WeightedSetTermBlueprint::addTerm(Blueprint::UP term, int32_t weight) term.release(); } + SearchIterator::UP -WeightedSetTermBlueprint::createSearch(search::fef::MatchData &md, bool) const +WeightedSetTermBlueprint::createLeafSearch(const search::fef::TermFieldMatchDataArray &tfmda, + bool) const { - const State &state = getState(); - assert(state.numFields() == 1); - search::fef::TermFieldMatchData &tfmd = *state.field(0).resolve(md); - + assert(tfmda.size() == 1); + fef::MatchData::UP md = _layout.createMatchData(); std::vector<SearchIterator*> children(_terms.size()); for (size_t i = 0; i < _terms.size(); ++i) { - children[i] = _terms[i]->createSearch(md, true).release(); + children[i] = _terms[i]->createSearch(*md, true).release(); } - return SearchIterator::UP(WeightedSetTermSearch::create(children, tfmd, _weights)); + return SearchIterator::UP(WeightedSetTermSearch::create(children, *tfmda[0], _weights, std::move(md))); } void @@ -71,10 +73,4 @@ WeightedSetTermBlueprint::visitMembers(vespalib::ObjectVisitor &visitor) const visit(visitor, "_terms", _terms); } -SearchIterator::UP -WeightedSetTermBlueprint::createLeafSearch(const search::fef::TermFieldMatchDataArray &, bool) const -{ - abort(); -} - } diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.h index 1afa0f9f2b2..b81d6c6f9e9 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.h @@ -15,6 +15,8 @@ namespace queryeval { class WeightedSetTermBlueprint : public ComplexLeafBlueprint { HitEstimate _estimate; + fef::MatchDataLayout _layout; + FieldSpec _children_field; std::vector<int32_t> _weights; std::vector<Blueprint*> _terms; @@ -27,18 +29,16 @@ public: // used by create visitor // matches signature in dot product blueprint for common blueprint - // building code. Hands out its own field spec to children. NOTE: - // this is only ok since children will never be unpacked. - FieldSpec getNextChildField(const FieldSpec &outer) { return outer; } + // building code. Hands out the same field spec to all children. + FieldSpec getNextChildField(const FieldSpec &) { return _children_field; } // used by create visitor void addTerm(Blueprint::UP term, int32_t weight); - SearchIteratorUP createSearch(search::fef::MatchData &md, bool strict) const override; + SearchIteratorUP createLeafSearch(const search::fef::TermFieldMatchDataArray &tfmda, bool strict) const override; void visitMembers(vespalib::ObjectVisitor &visitor) const override; private: - SearchIteratorUP createLeafSearch(const search::fef::TermFieldMatchDataArray &, bool) const override; void fetchPostings(bool strict) override; }; diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp index aae7c60bd80..2801f1c5e0c 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp @@ -134,15 +134,16 @@ public: SearchIterator * WeightedSetTermSearch::create(const std::vector<SearchIterator*> &children, TermFieldMatchData &tmd, - const std::vector<int32_t> &weights) + const std::vector<int32_t> &weights, + fef::MatchData::UP match_data) { typedef WeightedSetTermSearchImpl<vespalib::LeftArrayHeap, SearchIteratorPack> ArrayHeapImpl; typedef WeightedSetTermSearchImpl<vespalib::LeftHeap, SearchIteratorPack> HeapImpl; if (children.size() < 128) { - return new ArrayHeapImpl(tmd, weights, SearchIteratorPack(children)); + return new ArrayHeapImpl(tmd, weights, SearchIteratorPack(children, std::move(match_data))); } - return new HeapImpl(tmd, weights, SearchIteratorPack(children)); + return new HeapImpl(tmd, weights, SearchIteratorPack(children, std::move(match_data))); } //----------------------------------------------------------------------------- diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h index de7131bd0a9..397ac0caf2e 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h @@ -29,7 +29,8 @@ protected: public: static SearchIterator* create(const std::vector<SearchIterator*> &children, search::fef::TermFieldMatchData &tmd, - const std::vector<int32_t> &weights); + const std::vector<int32_t> &weights, + fef::MatchData::UP match_data); static SearchIterator::UP create(search::fef::TermFieldMatchData &tmd, const std::vector<int32_t> &weights, diff --git a/storageapi/src/vespa/storageapi/mbusprot/storagecommand.cpp b/storageapi/src/vespa/storageapi/mbusprot/storagecommand.cpp index 08fecfab666..8aa44a0d7bf 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/storagecommand.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/storagecommand.cpp @@ -1,13 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "storagecommand.h" -namespace storage { -namespace mbusprot { +namespace storage::mbusprot { StorageCommand::StorageCommand(const storage::api::StorageCommand::SP& cmd) : mbus::Message(), _cmd(cmd) { } -} // mbusprot -} // storage +} diff --git a/storageapi/src/vespa/storageapi/mbusprot/storagecommand.h b/storageapi/src/vespa/storageapi/mbusprot/storagecommand.h index bf5174f8f62..651336b39e5 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/storagecommand.h +++ b/storageapi/src/vespa/storageapi/mbusprot/storagecommand.h @@ -6,8 +6,7 @@ #include <vespa/messagebus/message.h> #include <vespa/storageapi/messageapi/storagecommand.h> -namespace storage { -namespace mbusprot { +namespace storage::mbusprot { class StorageCommand : public mbus::Message, public StorageMessage { public: @@ -30,5 +29,4 @@ private: api::StorageCommand::SP _cmd; }; -} // mbusprot -} // storage +} diff --git a/storageapi/src/vespa/storageapi/mbusprot/storagemessage.h b/storageapi/src/vespa/storageapi/mbusprot/storagemessage.h index 551388345de..c63271a8956 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/storagemessage.h +++ b/storageapi/src/vespa/storageapi/mbusprot/storagemessage.h @@ -3,8 +3,7 @@ #include <vespa/storageapi/messageapi/storagemessage.h> -namespace storage { -namespace mbusprot { +namespace storage::mbusprot { class StorageMessage { public: @@ -17,6 +16,5 @@ public: }; -} // protocol -} // storage +} diff --git a/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.h b/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.h index f8e698e1dc3..699f1c4c239 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.h +++ b/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.h @@ -4,8 +4,7 @@ #include "protocolserialization5_2.h" #include <vespa/messagebus/iprotocol.h> -namespace storage { -namespace mbusprot { +namespace storage::mbusprot { class StorageProtocol : public mbus::IProtocol { @@ -28,5 +27,4 @@ private: ProtocolSerialization5_2 _serializer5_2; }; -} // mbusprot -} // storage +} diff --git a/storageapi/src/vespa/storageapi/mbusprot/storagereply.cpp b/storageapi/src/vespa/storageapi/mbusprot/storagereply.cpp index 0c2fa022948..7a21a1bbb27 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/storagereply.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/storagereply.cpp @@ -7,23 +7,23 @@ using vespalib::alloc::Alloc; using vespalib::IllegalStateException; -namespace storage { -namespace mbusprot { +namespace storage::mbusprot { -StorageReply::StorageReply(const mbus::BlobRef& data, - const ProtocolSerialization& serializer) +StorageReply::StorageReply(mbus::BlobRef data, const ProtocolSerialization& serializer) : _serializer(&serializer), - _buffer(Alloc::alloc(data.size())), + _sz(data.size()), + _buffer(Alloc::alloc(_sz)), _mbusType(0), _reply() { - memcpy(_buffer.get(), data.data(), _buffer.size()); - document::ByteBuffer buf(data.data(), data.size()); + memcpy(_buffer.get(), data.data(), _sz); + document::ByteBuffer buf(data.data(), _sz); buf.getIntNetwork(reinterpret_cast<int32_t&>(_mbusType)); } StorageReply::StorageReply(const api::StorageReply::SP& reply) : _serializer(0), + _sz(0), _buffer(), _mbusType(reply->getType().getId()), _reply(reply) @@ -48,10 +48,9 @@ StorageReply::deserialize() const if (cmd == 0) { throw IllegalStateException("Storage reply get message did not return a storage command", VESPA_STRLOC); } - mbus::BlobRef blobRef(static_cast<char *>(_buffer.get()), _buffer.size()); + mbus::BlobRef blobRef(static_cast<char *>(_buffer.get()), _sz); _reply = _serializer->decodeReply(blobRef, *cmd->getCommand())->getReply(); Alloc().swap(_buffer); } -} // mbusprot -} // storage +} diff --git a/storageapi/src/vespa/storageapi/mbusprot/storagereply.h b/storageapi/src/vespa/storageapi/mbusprot/storagereply.h index 0000c98f7db..2fd6be2dc14 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/storagereply.h +++ b/storageapi/src/vespa/storageapi/mbusprot/storagereply.h @@ -6,11 +6,11 @@ #include <vespa/messagebus/reply.h> #include <vespa/storageapi/messageapi/storagereply.h> -namespace storage { -namespace mbusprot { +namespace storage::mbusprot { class StorageReply : public mbus::Reply, public StorageMessage { const ProtocolSerialization* _serializer; + size_t _sz; mutable vespalib::alloc::Alloc _buffer; uint32_t _mbusType; mutable api::StorageReply::SP _reply; @@ -18,7 +18,7 @@ class StorageReply : public mbus::Reply, public StorageMessage { public: typedef std::unique_ptr<StorageReply> UP; - StorageReply(const mbus::BlobRef& data, const ProtocolSerialization&); + StorageReply(mbus::BlobRef data, const ProtocolSerialization&); StorageReply(const api::StorageReply::SP& reply); ~StorageReply(); @@ -40,9 +40,7 @@ public: } private: - void deserialize() const; }; -} // mbusprot -} // storage +} diff --git a/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp b/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp index 7c113228a35..0028de9924a 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp @@ -5,8 +5,7 @@ #include <vespa/vespalib/util/exceptions.h> #include <ostream> -namespace storage { -namespace api { +namespace storage::api { StorageCommand::StorageCommand(const StorageCommand& other) : StorageMessage(other, generateMsgId()), @@ -49,5 +48,4 @@ StorageCommand::createCopyToForward(const document::BucketId&, uint64_t) const VESPA_STRLOC); } -} // api -} // storage +} diff --git a/storageapi/src/vespa/storageapi/messageapi/storagecommand.h b/storageapi/src/vespa/storageapi/messageapi/storagecommand.h index 73412807d66..5667c5c10f4 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagecommand.h +++ b/storageapi/src/vespa/storageapi/messageapi/storagecommand.h @@ -14,8 +14,7 @@ #include "storagemessage.h" -namespace storage { -namespace api { +namespace storage::api { class StorageReply; @@ -65,6 +64,4 @@ public: }; -} // api -} // storage - +} diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp index be384f253c0..d9eecf36a03 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp @@ -381,4 +381,3 @@ StorageMessage::getSummary() const { } } - diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h index 5b74d88fc68..20697c4be72 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h +++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h @@ -68,9 +68,7 @@ public: \ return std::unique_ptr<storage::api::StorageReply>(new reply(*this)); \ } -namespace storage { - -namespace api { +namespace storage::api { /** * @class MessageType @@ -404,8 +402,7 @@ public: * This method is overloaded in subclasses and will call the correct * method in the MessageHandler interface. */ - virtual bool callHandler(MessageHandler&, - const StorageMessage::SP&) const = 0; + virtual bool callHandler(MessageHandler&, const StorageMessage::SP&) const = 0; const documentapi::LoadType& getLoadType() const { return _loadType; } void setLoadType(const documentapi::LoadType& type) { _loadType = type; } @@ -427,6 +424,4 @@ public: virtual bool hasSingleBucketId() const { return false; } }; -} // api -} // storage - +} diff --git a/vdstestlib/src/vespa/vdstestlib/cppunit/macros.h b/vdstestlib/src/vespa/vdstestlib/cppunit/macros.h index 5f7ce892723..fdde94bbbd6 100644 --- a/vdstestlib/src/vespa/vdstestlib/cppunit/macros.h +++ b/vdstestlib/src/vespa/vdstestlib/cppunit/macros.h @@ -4,6 +4,7 @@ */ #pragma once #include <cppunit/extensions/HelperMacros.h> +#include <vespa/vespalib/test/insertion_operators.h> // Wrapper for CPPUNIT_ASSERT_EQUAL_MESSAGE to prevent it from evaluating @@ -147,49 +148,6 @@ // Create output operator for containers. // Needed so we can use CPPUNIT_ASSERT_EQUAL with them. -// TODO: Remove these functions from the std namespace. -namespace std { - template<typename T> - inline std::ostream& operator<<(std::ostream& out, const std::vector<T>& v) - { - out << "std::vector(" << v.size() << ") {"; - for (uint32_t i=0, n=v.size(); i<n; ++i) { - out << "\n " << v[i]; - } - if (v.size() > 0) out << "\n"; - return out << "}"; - } - template<typename T> - inline std::ostream& operator<<(std::ostream& out, const std::set<T>& v) - { - out << "std::set(" << v.size() << ") {"; - for (typename std::set<T>::const_iterator it = v.begin(); it != v.end(); - ++it) - { - out << "\n " << *it; - } - if (v.size() > 0) out << "\n"; - return out << "}"; - } - template<typename S, typename T> - inline std::ostream& operator<<(std::ostream& out, const std::map<S, T>& m) - { - out << "std::map(" << m.size() << ") {"; - for (typename std::map<S, T>::const_iterator it = m.begin(); - it != m.end(); ++it) - { - out << "\n " << *it; - } - if (m.size() > 0) out << "\n"; - return out << "}"; - } - template<typename S, typename T> - inline std::ostream& operator<<(std::ostream& out, const std::pair<S, T>& p) - { - return out << "std::pair(" << p.first << ", " << p.second << ")"; - } -} - template<typename S, typename T> std::ostream& operator<<(std::ostream& out, const std::unordered_map<S, T>& umap) diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/classlock/ClassLock.java b/vespajlib/src/main/java/com/yahoo/concurrent/classlock/ClassLock.java new file mode 100644 index 00000000000..2a3c70d31d2 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/concurrent/classlock/ClassLock.java @@ -0,0 +1,19 @@ +package com.yahoo.concurrent.classlock; + +/** + * @author valerijf + */ +public class ClassLock implements AutoCloseable { + private final Class<?> clazz; + private final ClassLocking classLocking; + + ClassLock(ClassLocking classLocking, Class<?> clazz) { + this.classLocking = classLocking; + this.clazz = clazz; + } + + @Override + public void close() { + classLocking.unlock(clazz, this); + } +} diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/classlock/ClassLocking.java b/vespajlib/src/main/java/com/yahoo/concurrent/classlock/ClassLocking.java new file mode 100644 index 00000000000..e387d259f26 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/concurrent/classlock/ClassLocking.java @@ -0,0 +1,53 @@ +package com.yahoo.concurrent.classlock; + +import java.util.HashMap; +import java.util.Map; +import java.util.function.BooleanSupplier; + +/** + * @author valerijf + */ +public class ClassLocking { + private final Map<Class<?>, ClassLock> classLocks = new HashMap<>(); + private final Object monitor = new Object(); + + public ClassLock lock(Class<?> clazz) { + return lockWhile(clazz, () -> true); + } + + public ClassLock lockWhile(Class<?> clazz, BooleanSupplier interruptCondition) { + synchronized (monitor) { + while (classLocks.containsKey(clazz)) { + try { + monitor.wait(); + } catch (InterruptedException ignored) { + } + + if (!interruptCondition.getAsBoolean()) { + throw new LockInterruptException(); + } + } + + ClassLock classLock = new ClassLock(this, clazz); + classLocks.put(clazz, classLock); + return classLock; + } + } + + void unlock(Class<?> clazz, ClassLock classLock) { + synchronized (monitor) { + if (classLock.equals(classLocks.get(clazz))) { + classLocks.remove(clazz); + monitor.notifyAll(); + } else { + throw new IllegalArgumentException("Lock has already been released"); + } + } + } + + public void interrupt() { + synchronized (monitor) { + monitor.notifyAll(); + } + } +} diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/classlock/LockInterruptException.java b/vespajlib/src/main/java/com/yahoo/concurrent/classlock/LockInterruptException.java new file mode 100644 index 00000000000..b2ae4166564 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/concurrent/classlock/LockInterruptException.java @@ -0,0 +1,8 @@ +package com.yahoo.concurrent.classlock; + +/** + * @author valerijf + */ +@SuppressWarnings("serial") +public class LockInterruptException extends RuntimeException { +} diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/classlock/package-info.java b/vespajlib/src/main/java/com/yahoo/concurrent/classlock/package-info.java new file mode 100644 index 00000000000..010af471f03 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/concurrent/classlock/package-info.java @@ -0,0 +1,5 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.concurrent.classlock; + +import com.yahoo.osgi.annotation.ExportPackage; |