diff options
88 files changed, 417 insertions, 1386 deletions
diff --git a/configdefinitions/src/vespa/zookeeper-server.def b/configdefinitions/src/vespa/zookeeper-server.def index 723246daab0..b8ff6494764 100644 --- a/configdefinitions/src/vespa/zookeeper-server.def +++ b/configdefinitions/src/vespa/zookeeper-server.def @@ -42,3 +42,8 @@ server[].id int server[].hostname string server[].quorumPort int default=2182 server[].electionPort int default=2183 + +# Needed when upgrading from ZooKeeper 3.4 to 3.5, see https://issues.apache.org/jira/browse/ZOOKEEPER-3056, +# and in general where there is a zookeeper ensemble running that has had few transactions. +# TODO: Consider setting this to false by default (and override where appropriate) +trustEmptySnapshot bool default=true diff --git a/container-dependency-versions/pom.xml b/container-dependency-versions/pom.xml index 44ee98ae521..8093fa41e43 100644 --- a/container-dependency-versions/pom.xml +++ b/container-dependency-versions/pom.xml @@ -429,6 +429,7 @@ <phase>verify</phase> <goals> <goal>display-property-updates</goal> + <goal>property-updates-report</goal> </goals> </execution> </executions> diff --git a/container-search/src/main/antlr4/com/yahoo/search/yql/yqlplus.g4 b/container-search/src/main/antlr4/com/yahoo/search/yql/yqlplus.g4 index 2e8aac43a65..639ff8255d2 100644 --- a/container-search/src/main/antlr4/com/yahoo/search/yql/yqlplus.g4 +++ b/container-search/src/main/antlr4/com/yahoo/search/yql/yqlplus.g4 @@ -298,7 +298,7 @@ ident keyword_as_ident : SELECT | TABLE | DELETE | INTO | VALUES | LIMIT | OFFSET | WHERE | 'order' | 'by' | DESC | MERGE | LEFT | JOIN | ON | OUTPUT | COUNT | BEGIN | END | APPLY | TYPE_BYTE | TYPE_INT16 | TYPE_INT32 | TYPE_INT64 | TYPE_BOOLEAN | TYPE_TIMESTAMP | TYPE_DOUBLE | TYPE_STRING | TYPE_ARRAY | TYPE_MAP - | VIEW | CREATE | IMPORT | PROGRAM | NEXT | PAGED | SOURCES | SET | MATCHES + | VIEW | CREATE | IMPORT | PROGRAM | NEXT | PAGED | SOURCES | SET | MATCHES | LIKE ; //program : params? (import_statement SEMI)* (ddl SEMI)* (statement SEMI)* EOF diff --git a/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java index 78868f37718..7b3327c04af 100644 --- a/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java @@ -35,9 +35,6 @@ import static org.junit.Assume.assumeTrue; */ public class QueryTestCase { - /** - * Basic test - */ @Test public void testSimpleQueryParsing () { Query q = newQuery("/search?query=foobar&offset=10&hits=20"); @@ -46,9 +43,6 @@ public class QueryTestCase { assertEquals(20,q.getHits()); } - /** - * Not quite so basic - */ @Test public void testAdvancedQueryParsing () { Query q = newQuery("/search?query=fOObar and kanoo&offset=10&hits=20&filter=-foo +bar&type=adv&suggestonly=true"); @@ -58,9 +52,6 @@ public class QueryTestCase { assertEquals(true, q.properties().getBoolean("suggestonly", false)); } - /** - * Not quite so basic - */ @Test public void testAnyQueryParsing () { Query q = newQuery("/search?query=foobar and kanoo&offset=10&hits=10&type=any&suggestonly=true&filter=-fast.type:offensive&encoding=latin1"); @@ -71,9 +62,6 @@ public class QueryTestCase { assertEquals("latin1",q.getModel().getEncoding()); } - /** - * A long string - */ @Test public void testLongQueryParsing() { Query q = newQuery("/p13n?query=news" @@ -108,18 +96,12 @@ public class QueryTestCase { } } - /** - * Test UTF-8 decoding - */ @Test public void testUtf8Decoding() { Query q = new Query("/?query=beyonc%C3%A9"); assertEquals("beyonc\u00e9",((WordItem) q.getModel().getQueryTree().getRoot()).getWord()); } - /** - * Check sortspec "parsing" is correct. - */ @Test public void testSortSpec() { Query q = newQuery("?query=test&sortspec=+a -b c +[rank]"); @@ -321,7 +303,7 @@ public class QueryTestCase { /** Test a vertical specific patch, see Tokenizer */ @Test @Ignore - public void testOtherharactersParsing() { + public void testOtherCharactersParsing() { Query query=newQuery(com.yahoo.search.test.QueryTestCase.httpEncode("?query=\u3007\u2e80\u2eff\u2ed0")); assertEquals(Character.UnicodeBlock.CJK_SYMBOLS_AND_PUNCTUATION, Character.UnicodeBlock.of(query.getModel().getQueryTree().getRoot().toString().charAt(0))); diff --git a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java index 00a17f963c6..1d25c2301e2 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java @@ -980,6 +980,12 @@ public class YqlParserTestCase { assertUrlQuery("urlfield", yql, false, false, false); } + @Test + public void testReservedWordInSource() { + parse("select * from sources like where text contains \"test\";"); + // success: parsed without exception + } + private void assertUrlQuery(String field, Query query, boolean startAnchor, boolean endAnchor, boolean endAnchorIsDefault) { boolean startAnchorIsDefault = false; // Always diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 7c80593dee8..ba9f8e809c8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -149,7 +149,6 @@ public class ApplicationController { // Update serialization format of all applications Once.after(Duration.ofMinutes(1), () -> { - curator.clearInstanceRoot(); Instant start = clock.instant(); int count = 0; for (Application application : curator.readApplications()) { @@ -743,7 +742,6 @@ public class ApplicationController { application.get().require(applicationId.instance()).deployments().keySet().stream().map(ZoneId::toString) .sorted().collect(Collectors.joining(", "))); - curator.removeApplication(applicationId); applicationStore.removeAll(applicationId); applicationStore.removeAll(TesterId.of(applicationId)); @@ -756,6 +754,7 @@ public class ApplicationController { controller.nameServiceForwarder().removeRecords(Record.Type.CNAME, RecordName.from(name), Priority.normal); }); }); + curator.storeWithoutInstance(application.without(applicationId.instance()).get(), applicationId); log.info("Deleted " + application); }); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java index c9ec355a3b9..3f1bebfed48 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java @@ -292,34 +292,6 @@ public class Instance { return metrics; } - /** Returns activity for this */ - public ApplicationActivity activity() { - return ApplicationActivity.from(deployments.values()); - } - - /** - * Returns the oldest platform version this has deployed in a permanent zone (not test or staging). - * - * This is unfortunately quite similar to {@link ApplicationController#oldestInstalledPlatform(TenantAndApplicationId)}, - * but this checks only what the controller has deployed to the production zones, while that checks the node repository - * to see what's actually installed on each node. Thus, this is the right choice for, e.g., target Vespa versions for - * new deployments, while that is the right choice for version to compile against. - */ - public Optional<Version> oldestDeployedPlatform() { - return productionDeployments().values().stream() - .map(Deployment::version) - .min(Comparator.naturalOrder()); - } - - /** - * Returns the oldest application version this has deployed in a permanent zone (not test or staging). - */ - public Optional<ApplicationVersion> oldestDeployedApplication() { - return productionDeployments().values().stream() - .map(Deployment::applicationVersion) - .min(Comparator.naturalOrder()); - } - /** Returns all rotations assigned to this */ public List<AssignedRotation> rotations() { return rotations; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index 9aba8921860..b8912a848fc 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -114,7 +114,7 @@ public class LockedApplication { projectId, internal, instances); } - private LockedApplication without(InstanceName instance) { + public LockedApplication without(InstanceName instance) { var instances = new HashMap<>(this.instances); instances.remove(instance); return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 2680160b1cb..76a146fb1e6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -15,7 +15,6 @@ import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; -import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; @@ -77,7 +76,6 @@ public class CuratorDb { private static final Path lockRoot = root.append("locks"); private static final Path tenantRoot = root.append("tenants"); private static final Path applicationRoot = root.append("applications"); - private static final Path instanceRoot = root.append("instances"); private static final Path jobRoot = root.append("jobs"); private static final Path controllerRoot = root.append("controllers"); private static final Path routingPoliciesRoot = root.append("routingPolicies"); @@ -143,10 +141,6 @@ public class CuratorDb { return lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); } - public Lock lock(ApplicationId id) { - return lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); - } - public Lock lockForDeployment(ApplicationId id, ZoneId zone) { return lock(lockPath(id, zone), deployLockTimeout); } @@ -350,8 +344,7 @@ public class CuratorDb { } public Optional<Application> readApplication(TenantAndApplicationId application) { - List<Instance> instances = readInstances(id -> TenantAndApplicationId.from(id).equals(application)); - return Application.aggregate(instances); + return readSlime(applicationPath(application)).map(applicationSerializer::fromSlime); } public List<Application> readApplications() { @@ -362,45 +355,28 @@ public class CuratorDb { return readApplications(application -> application.tenant().equals(name)); } - private Stream<TenantAndApplicationId> readTenantAndApplicationIds() { - return readInstanceIds().map(TenantAndApplicationId::from).distinct(); - } - private List<Application> readApplications(Predicate<TenantAndApplicationId> applicationFilter) { - return readTenantAndApplicationIds().filter(applicationFilter) - .sorted() - .map(this::readApplication) - .flatMap(Optional::stream) - .collect(Collectors.toUnmodifiableList()); - } - - private Optional<Instance> readInstance(ApplicationId application) { - return readSlime(oldApplicationPath(application)).map(instanceSerializer::fromSlime); + return readApplicationIds().filter(applicationFilter) + .sorted() + .map(this::readApplication) + .flatMap(Optional::stream) + .collect(Collectors.toUnmodifiableList()); } - private Stream<ApplicationId> readInstanceIds() { + // TODO jonmv: Clear out old instance data here + private Stream<TenantAndApplicationId> readApplicationIds() { return curator.getChildren(applicationRoot).stream() - .filter(id -> id.split(":").length == 3) - .map(ApplicationId::fromSerializedForm); + .filter(id -> id.split(":").length == 2) + .map(TenantAndApplicationId::fromSerialized); } - private List<Instance> readInstances(Predicate<ApplicationId> applicationFilter) { - return readInstanceIds().filter(applicationFilter) - .sorted() - .map(this::readInstance) - .flatMap(Optional::stream) - .collect(Collectors.toUnmodifiableList()); - } - - public void removeApplication(ApplicationId id) { - // WARNING: This is part of a multi-step data move operation, so don't touch!!! - curator.delete(oldApplicationPath(id)); - if (readApplication(TenantAndApplicationId.from(id)).isEmpty()) - curator.delete(applicationPath(TenantAndApplicationId.from(id))); - } - - public void clearInstanceRoot() { - curator.delete(instanceRoot); + // TODO jonmv: Refactor when instance split operation is done + public void storeWithoutInstance(Application application, ApplicationId instanceId) { + curator.delete(oldApplicationPath(instanceId)); + if (application.instances().isEmpty()) + curator.delete(applicationPath(application.id())); + else + writeApplication(application); } /** @@ -422,12 +398,14 @@ public class CuratorDb { * Write application with instances to application path DONE * Write all instances of an application to old application path DONE * Remove everything under instance root DONE + * Stop locking applications on instance level DONE * - * Read Application with instances from application path (with filter) - * Stop locking applications on instance level + * Read Application with instances from application path (with filter) DONE * * Stop writing Instance to old application path * Remove unused parts of Instance (Used only for legacy serialization) + * Store new production application packages under non-instance path + * Read production packages from non-instance path, with fallback */ // -------------- Job Runs ------------------------------------------------ @@ -575,19 +553,19 @@ public class CuratorDb { .append(instance.instance().value()); } - private Path lockPath(ApplicationId application, ZoneId zone) { - return lockPath(application) + private Path lockPath(ApplicationId instance, ZoneId zone) { + return lockPath(instance) .append(zone.environment().value()) .append(zone.region().value()); } - private Path lockPath(ApplicationId application, JobType type) { - return lockPath(application) + private Path lockPath(ApplicationId instance, JobType type) { + return lockPath(instance) .append(type.jobName()); } - private Path lockPath(ApplicationId application, JobType type, Step step) { - return lockPath(application, type) + private Path lockPath(ApplicationId instance, JobType type, Step step) { + return lockPath(instance, type) .append(step.name()); } @@ -657,10 +635,6 @@ public class CuratorDb { return applicationRoot.append(application.serializedForm()); } - private static Path instancePath(ApplicationId id) { - return instanceRoot.append(id.serializedForm()); - } - private static Path runsPath(ApplicationId id, JobType type) { return jobRoot.append(id.serializedForm()).append(type.jobName()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 90a4ecdef9a..ab92e38ee4b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -184,7 +184,7 @@ class JobControllerApiHandlerHelper { lastPlatformObject.setString("pending", application.change().isEmpty() ? "Waiting for upgrade slot" - : "Waiting for " + instance.change() + " to complete"); + : "Waiting for " + application.change() + " to complete"); } private static void lastApplicationToSlime(Cursor lastApplicationObject, Application application, Instance instance, Change change, DeploymentSteps steps, Controller controller) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 5d82225c9d5..90d257d754d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -141,8 +141,6 @@ public class ControllerTest { // Simulate restart tester.restartController(); - applications = tester.controller().applications(); - assertNotNull(tester.controller().tenants().get(TenantName.from("tenant1"))); assertNotNull(tester.defaultInstance(app1.id())); assertEquals(4, tester.defaultInstance(app1.id()).deploymentJobs().jobStatus().size()); @@ -152,7 +150,7 @@ public class ControllerTest { // system and staging test job - succeeding tester.jobCompletion(component).application(app1).nextBuildNumber().uploadArtifact(applicationPackage).submit(); - applicationVersion = tester.defaultInstance("app1").change().application().get(); + applicationVersion = tester.application(app1.id()).change().application().get(); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, systemTest); assertStatus(JobStatus.initial(systemTest) .withTriggering(version1, applicationVersion, Optional.of(tester.defaultInstance(app1.id()).deployments().get(productionUsWest1.zone(main))), "", tester.clock().instant().truncatedTo(MILLIS)) @@ -853,18 +851,19 @@ public class ControllerTest { MockCuratorDb newDb = new MockCuratorDb(); OldMockCuratorDb oldDb = new OldMockCuratorDb(newDb.curator()); - oldDb.writeInstance(old1); - oldDb.writeInstance(old2); + oldDb.writeApplication(Application.aggregate(List.of(old1, old2)).orElseThrow()); - Application application = newDb.readApplication(TenantAndApplicationId.from("t1", "a1")).orElseThrow(); + Application application = oldDb.readApplication(TenantAndApplicationId.from("t1", "a1")).orElseThrow(); Instance new1 = application.legacy(InstanceName.defaultName()); String new1Serialized = new String(JsonFormat.toJsonBytes(instanceSerializer.toSlime(new1)), UTF_8); assertEquals(old1Serialized, new1Serialized); LockedApplication locked = new LockedApplication(application, newDb.lock(application.id())); - newDb.writeApplication(locked.with(new ApplicationMetrics(8, 9)).get()); - Instance mod1 = oldDb.readInstance(old1.id()).orElseThrow(); - Instance mod2 = oldDb.readInstance(old2.id()).orElseThrow(); + oldDb.writeApplication(locked.with(new ApplicationMetrics(8, 9)).get()); + + Application newApp = newDb.readApplication(application.id()).orElseThrow(); + Instance mod1 = newApp.legacy(old1.name()); + Instance mod2 = newApp.legacy(old2.name()); old1 = old1.with(new ApplicationMetrics(8, 9)); old1Serialized = new String(JsonFormat.toJsonBytes(instanceSerializer.toSlime(old1)), UTF_8); @@ -878,12 +877,14 @@ public class ControllerTest { assertEquals(old2.deployments(), mod2.deployments()); assertEquals(old2.deploymentJobs().jobStatus(), mod2.deploymentJobs().jobStatus()); - newDb.removeApplication(old1.id()); + application = new LockedApplication(application, newDb.lock(application.id())).without(old1.name()).get(); + newDb.storeWithoutInstance(application, old1.id()); assertEquals(1, newDb.readApplication(application.id()).orElseThrow().instances().size()); - newDb.removeApplication(old2.id()); + assertEquals(1, oldDb.readApplication(application.id()).orElseThrow().instances().size()); + application = new LockedApplication(application, newDb.lock(application.id())).without(old2.name()).get(); + newDb.storeWithoutInstance(application, old2.id()); assertTrue(newDb.readApplication(application.id()).isEmpty()); - assertTrue(oldDb.readInstance(old1.id()).isEmpty()); - assertTrue(oldDb.readInstance(old2.id()).isEmpty()); + assertTrue(oldDb.readApplication(application.id()).isEmpty()); } private void runUpgrade(DeploymentTester tester, ApplicationId application, ApplicationVersion version) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 5441ed6aec0..545729b5474 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -177,7 +177,7 @@ public class DeploymentTriggerTest { tester.deployAndNotify(instance.id(), Optional.empty(), false, systemTest); tester.deployAndNotify(instance.id(), Optional.empty(), false, stagingTest); tester.deployAndNotify(instance.id(), Optional.empty(), false, productionUsCentral1); - assertEquals(Change.empty(), tester.instance(instance.id()).change()); + assertEquals(Change.empty(), tester.application(application.id()).change()); tester.assertNotRunning(systemTest, instance.id()); tester.assertNotRunning(stagingTest, instance.id()); tester.assertNotRunning(productionUsCentral1, instance.id()); @@ -203,7 +203,7 @@ public class DeploymentTriggerTest { tester.readyJobTrigger().maintain(); iTester.runJob(JobType.productionUsWest1); iTester.runJob(JobType.productionUsEast3); - assertEquals(Change.empty(), iTester.instance().change()); + assertEquals(Change.empty(), iTester.application().change()); tester.upgradeSystem(new Version("8.9")); iTester.runJob(JobType.systemTest); @@ -542,7 +542,7 @@ public class DeploymentTriggerTest { tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, productionUsWest1); assertEquals(BuildJob.defaultBuildNumber, tester.defaultInstance(application.id()).deploymentJobs().jobStatus() .get(productionUsWest1).lastSuccess().get().application().buildNumber().getAsLong()); - assertEquals((BuildJob.defaultBuildNumber + 1), tester.defaultInstance(application.id()).outstandingChange().application().get().buildNumber().getAsLong()); + assertEquals((BuildJob.defaultBuildNumber + 1), tester.application(application.id()).outstandingChange().application().get().buildNumber().getAsLong()); tester.readyJobTrigger().maintain(); // Platform upgrade keeps rolling, since it has already deployed in a production zone, and tests for the new revision have also started. @@ -551,23 +551,23 @@ public class DeploymentTriggerTest { assertEquals(2, tester.buildService().jobs().size()); // Upgrade is done, and oustanding change rolls out when block window ends. - assertEquals(Change.empty(), tester.defaultInstance(application.id()).change()); - assertFalse(tester.defaultInstance(application.id()).change().hasTargets()); - assertTrue(tester.defaultInstance(application.id()).outstandingChange().hasTargets()); + assertEquals(Change.empty(), tester.application(application.id()).change()); + assertFalse(tester.application(application.id()).change().hasTargets()); + assertTrue(tester.application(application.id()).outstandingChange().hasTargets()); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, stagingTest); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, systemTest); tester.clock().advance(Duration.ofHours(1)); tester.outstandingChangeDeployer().run(); - assertTrue(tester.defaultInstance(application.id()).change().hasTargets()); - assertFalse(tester.defaultInstance(application.id()).outstandingChange().hasTargets()); + assertTrue(tester.application(application.id()).change().hasTargets()); + assertFalse(tester.application(application.id()).outstandingChange().hasTargets()); tester.readyJobTrigger().run(); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, productionUsWest1); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, productionUsEast3); - assertFalse(tester.defaultInstance(application.id()).change().hasTargets()); - assertFalse(tester.defaultInstance(application.id()).outstandingChange().hasTargets()); + assertFalse(tester.application(application.id()).change().hasTargets()); + assertFalse(tester.application(application.id()).outstandingChange().hasTargets()); } @Test @@ -622,7 +622,8 @@ public class DeploymentTriggerTest { public void applicationVersionIsNotDowngraded() { Application application = tester.createApplication("app1", "tenant1", 1, 1L); Instance instance = tester.defaultInstance(application.id()); - Supplier<Instance> app = () -> tester.defaultInstance(application.id()); + Supplier<Application> apps = () -> tester.application(application.id()); + Supplier<Instance> instances = () -> tester.defaultInstance(application.id()); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .region("us-central-1") @@ -639,15 +640,15 @@ public class DeploymentTriggerTest { tester.deploy(productionUsCentral1, instance.id(), Optional.empty(), false); ApplicationVersion appVersion1 = ApplicationVersion.from(BuildJob.defaultSourceRevision, BuildJob.defaultBuildNumber + 1); - assertEquals(appVersion1, app.get().deployments().get(ZoneId.from("prod.us-central-1")).applicationVersion()); + assertEquals(appVersion1, instances.get().deployments().get(ZoneId.from("prod.us-central-1")).applicationVersion()); // Verify the application change is not removed when change is cancelled. tester.deploymentTrigger().cancelChange(application.id(), PLATFORM); - assertEquals(Change.of(appVersion1), app.get().change()); + assertEquals(Change.of(appVersion1), apps.get().change()); // Now cancel the change as is done through the web API. tester.deploymentTrigger().cancelChange(application.id(), ALL); - assertEquals(Change.empty(), app.get().change()); + assertEquals(Change.empty(), apps.get().change()); // A new version is released, which should now deploy the currently deployed application version to avoid downgrades. Version version1 = new Version("6.2"); @@ -665,7 +666,7 @@ public class DeploymentTriggerTest { // Finally, the two production jobs complete, in order. tester.deployAndNotify(instance.id(), Optional.empty(), true, productionUsCentral1); tester.deployAndNotify(instance.id(), Optional.empty(), true, productionEuWest1); - assertEquals(appVersion1, app.get().deployments().get(ZoneId.from("prod.us-central-1")).applicationVersion()); + assertEquals(appVersion1, instances.get().deployments().get(ZoneId.from("prod.us-central-1")).applicationVersion()); } @Test @@ -673,8 +674,8 @@ public class DeploymentTriggerTest { Application application1 = tester.createApplication("app1", "tenant1", 1, 1L); Application application2 = tester.createApplication("app2", "tenant2", 2, 2L); Instance instance1 = tester.defaultInstance(application1.id()); - Instance instance2 = tester.defaultInstance(application2.id()); - Supplier<Instance> app1 = () -> tester.defaultInstance(application1.id()); + Supplier<Application> app1s = () -> tester.application(application1.id()); + Supplier<Instance> instance1s = () -> tester.defaultInstance(application1.id()); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .region("us-central-1") @@ -701,21 +702,21 @@ public class DeploymentTriggerTest { tester.completeUpgradeWithError(application1, version2, applicationPackage, productionUsCentral1); tester.deploy(productionUsCentral1, instance1.id(), applicationPackage); tester.deployAndNotify(instance1.id(), Optional.of(applicationPackage), false, productionUsCentral1); - assertEquals(version2, app1.get().deployments().get(productionUsCentral1.zone(main)).version()); + assertEquals(version2, instance1s.get().deployments().get(productionUsCentral1.zone(main)).version()); // version2 becomes broken and upgrade targets latest non-broken tester.upgrader().overrideConfidence(version2, VespaVersion.Confidence.broken); tester.computeVersionStatus(); tester.upgrader().maintain(); // Cancel upgrades to broken version - assertEquals("Change becomes latest non-broken version", Change.of(version1), app1.get().change()); + assertEquals("Change becomes latest non-broken version", Change.of(version1), app1s.get().change()); tester.deployAndNotify(instance1.id(), Optional.of(applicationPackage), false, productionUsCentral1); - Instant triggered = app1.get().deploymentJobs().jobStatus().get(productionUsCentral1).lastTriggered().get().at(); + Instant triggered = instance1s.get().deploymentJobs().jobStatus().get(productionUsCentral1).lastTriggered().get().at(); tester.clock().advance(Duration.ofHours(1)); // version1 proceeds 'til the last job, where it fails; us-central-1 is skipped, as current change is strictly dominated by what's deployed there. tester.deployAndNotify(instance1.id(), Optional.of(applicationPackage), true, systemTest); tester.deployAndNotify(instance1.id(), Optional.of(applicationPackage), true, stagingTest); - assertEquals(triggered, app1.get().deploymentJobs().jobStatus().get(productionUsCentral1).lastTriggered().get().at()); + assertEquals(triggered, instance1s.get().deploymentJobs().jobStatus().get(productionUsCentral1).lastTriggered().get().at()); tester.deployAndNotify(instance1.id(), Optional.of(applicationPackage), false, productionEuWest1); //Eagerly triggered system and staging tests complete. @@ -729,15 +730,15 @@ public class DeploymentTriggerTest { tester.deployAndNotify(instance1.id(), Optional.of(applicationPackage), true, stagingTest); tester.assertRunning(productionUsCentral1, instance1.id()); - assertEquals(version2, app1.get().deployments().get(productionUsCentral1.zone(main)).version()); - assertEquals(42, app1.get().deployments().get(productionUsCentral1.zone(main)).applicationVersion().buildNumber().getAsLong()); - assertNotEquals(triggered, app1.get().deploymentJobs().jobStatus().get(productionUsCentral1).lastTriggered().get().at()); + assertEquals(version2, instance1s.get().deployments().get(productionUsCentral1.zone(main)).version()); + assertEquals(42, instance1s.get().deployments().get(productionUsCentral1.zone(main)).applicationVersion().buildNumber().getAsLong()); + assertNotEquals(triggered, instance1s.get().deploymentJobs().jobStatus().get(productionUsCentral1).lastTriggered().get().at()); // Change has a higher application version than what is deployed -- deployment should trigger. tester.deployAndNotify(instance1.id(), Optional.of(applicationPackage), false, productionUsCentral1); tester.deploy(productionUsCentral1, instance1.id(), applicationPackage); - assertEquals(version2, app1.get().deployments().get(productionUsCentral1.zone(main)).version()); - assertEquals(43, app1.get().deployments().get(productionUsCentral1.zone(main)).applicationVersion().buildNumber().getAsLong()); + assertEquals(version2, instance1s.get().deployments().get(productionUsCentral1.zone(main)).version()); + assertEquals(43, instance1s.get().deployments().get(productionUsCentral1.zone(main)).applicationVersion().buildNumber().getAsLong()); // Change is again strictly dominated, and us-central-1 is skipped, even though it is still failing. tester.clock().advance(Duration.ofHours(2).plus(Duration.ofSeconds(1))); // Enough time for retry @@ -749,8 +750,8 @@ public class DeploymentTriggerTest { tester.deployAndNotify(instance1.id(), Optional.empty(), true, systemTest); tester.deployAndNotify(instance1.id(), Optional.empty(), true, stagingTest); tester.deployAndNotify(instance1.id(), Optional.of(applicationPackage), true, productionEuWest1); - assertFalse(app1.get().change().hasTargets()); - assertFalse(app1.get().deploymentJobs().jobStatus().get(productionUsCentral1).isSuccess()); + assertFalse(app1s.get().change().hasTargets()); + assertFalse(instance1s.get().deploymentJobs().jobStatus().get(productionUsCentral1).isSuccess()); } @Test @@ -805,7 +806,7 @@ public class DeploymentTriggerTest { tester.deployAndNotify(instance.id(), Optional.empty(), false, productionUsEast3); tester.deployAndNotify(instance.id(), Optional.empty(), true, productionUsEast3); tester.deployAndNotify(instance.id(), Optional.empty(), true, productionEuWest1); - assertFalse(app.get().change().hasTargets()); + assertFalse(tester.application(application.id()).change().hasTargets()); assertEquals(43, app.get().deploymentJobs().jobStatus().get(productionEuWest1).lastSuccess().get().application().buildNumber().getAsLong()); assertEquals(43, app.get().deploymentJobs().jobStatus().get(productionUsEast3).lastSuccess().get().application().buildNumber().getAsLong()); } @@ -941,7 +942,7 @@ public class DeploymentTriggerTest { tester.clock().advance(Duration.ofSeconds(1)); // Advance time so that we can detect jobs in progress tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), false, JobType.productionUsEast3); assertEquals("Production job is retried", 1, tester.buildService().jobs().size()); - assertEquals("Application has pending upgrade to " + version, version, tester.defaultInstance(app.id()).change().platform().get()); + assertEquals("Application has pending upgrade to " + version, version, tester.application(app.id()).change().platform().get()); // Another version is released, which cancels any pending upgrades to lower versions version = Version.fromString("6.4"); @@ -949,7 +950,7 @@ public class DeploymentTriggerTest { tester.upgrader().maintain(); tester.jobCompletion(JobType.productionUsEast3).application(app).unsuccessful().submit(); assertEquals("Application starts upgrading to new version", 2, tester.buildService().jobs().size()); - assertEquals("Application has pending upgrade to " + version, version, tester.defaultInstance(app.id()).change().platform().get()); + assertEquals("Application has pending upgrade to " + version, version, tester.application(app.id()).change().platform().get()); // Failure re-deployer did not retry failing job for prod.us-east-3, since it no longer had an available change assertFalse("Job is not retried", tester.buildService().jobs().stream() @@ -992,7 +993,7 @@ public class DeploymentTriggerTest { assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); tester.upgrader().maintain(); tester.readyJobTrigger().maintain(); - assertEquals("Application has pending upgrade to " + version, version, tester.defaultInstance(app.id()).change().platform().get()); + assertEquals("Application has pending upgrade to " + version, version, tester.application(app.id()).change().platform().get()); // system-test fails and is left with a retry tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), false, JobType.systemTest); @@ -1005,7 +1006,7 @@ public class DeploymentTriggerTest { tester.buildService().remove(buildJob(instance.id(), systemTest)); tester.upgrader().maintain(); tester.readyJobTrigger().maintain(); - assertEquals("Application has pending upgrade to " + version, version, tester.defaultInstance(app.id()).change().platform().get()); + assertEquals("Application has pending upgrade to " + version, version, tester.application(app.id()).change().platform().get()); // Cancellation of outdated version and triggering on a new version is done by the upgrader. assertEquals(version, tester.defaultInstance(app.id()).deploymentJobs().jobStatus().get(systemTest).lastTriggered().get().platform()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java index bcc6fcfda44..3dc9de5619a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java @@ -152,8 +152,8 @@ public class InternalDeploymentTester { assertFalse(instance().deployments().values().stream() .anyMatch(deployment -> deployment.applicationVersion().equals(applicationVersion))); - assertEquals(applicationVersion, instance().change().application().get()); - assertFalse(instance().change().platform().isPresent()); + assertEquals(applicationVersion, application().change().application().get()); + assertFalse(application().change().platform().isPresent()); runJob(JobType.systemTest); runJob(JobType.stagingTest); @@ -171,8 +171,8 @@ public class InternalDeploymentTester { tester.upgradeSystem(version); assertFalse(instance().deployments().values().stream() .anyMatch(deployment -> deployment.version().equals(version))); - assertEquals(version, instance().change().platform().get()); - assertFalse(instance().change().application().isPresent()); + assertEquals(version, application().change().platform().get()); + assertFalse(application().change().application().isPresent()); runJob(JobType.systemTest); runJob(JobType.stagingTest); @@ -190,7 +190,7 @@ public class InternalDeploymentTester { assertTrue(tester.configServer().nodeRepository() .list(JobType.productionUsEast3.zone(tester.controller().system()), instanceId).stream() .allMatch(node -> node.currentVersion().equals(version))); - assertFalse(instance().change().hasTargets()); + assertFalse(application().change().hasTargets()); } /** @@ -282,7 +282,7 @@ public class InternalDeploymentTester { * Creates and submits a new application, and then starts the job of the given type. */ public RunId newRun(JobType type) { - assertFalse(instance().deploymentJobs().deployedInternally()); // Use this only once per test. + assertFalse(application().internal()); // Use this only once per test. newSubmission(); tester.readyJobTrigger().maintain(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java index ec98fbf69cb..5f210969b4d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java @@ -2,13 +2,16 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.organization.ApplicationSummary; import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.tenant.UserTenant; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; @@ -16,6 +19,9 @@ import org.junit.Before; import org.junit.Test; import java.time.Duration; +import java.util.Collection; +import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.function.Supplier; @@ -44,12 +50,12 @@ public class ApplicationOwnershipConfirmerTest { Optional<Contact> contact = Optional.of(tester.controllerTester().serviceRegistry().contactRetrieverMock().contact()); TenantName property = tester.controllerTester().createTenant("property", "domain", 1L, contact); tester.createAndDeploy(property, "application", 1, "default"); - Supplier<Instance> propertyApp = () -> tester.controller().applications().requireInstance(ApplicationId.from("property", "application", "default")); + Supplier<Application> propertyApp = () -> tester.controller().applications().requireApplication(TenantAndApplicationId.from("property", "application")); UserTenant user = UserTenant.create("by-user", contact); tester.controller().tenants().createUser(user); tester.createAndDeploy(user.name(), "application", 2, "default"); - Supplier<Instance> userApp = () -> tester.controller().applications().requireInstance(ApplicationId.from("by-user", "application", "default")); + Supplier<Application> userApp = () -> tester.controller().applications().requireApplication(TenantAndApplicationId.from("by-user", "application")); assertFalse("No issue is initially stored for a new application.", propertyApp.get().ownershipIssueId().isPresent()); assertFalse("No issue is initially stored for a new application.", userApp.get().ownershipIssueId().isPresent()); @@ -80,9 +86,15 @@ public class ApplicationOwnershipConfirmerTest { // The user deletes all production deployments — see that the issue is forgotten. assertEquals("Confirmation issue for user is sitll open.", issueId, userApp.get().ownershipIssueId()); - tester.controller().applications().deactivate(userApp.get().id(), userApp.get().productionDeployments().keySet().stream().findAny().get()); - tester.controller().applications().deactivate(userApp.get().id(), userApp.get().productionDeployments().keySet().stream().findAny().get()); - assertTrue("No production deployments are listed for user.", userApp.get().productionDeployments().isEmpty()); + tester.controller().applications().deactivate(userApp.get().id().defaultInstance(), + userApp.get().productionDeployments().values().stream() + .flatMap(List::stream) + .findAny().get().zone()); + tester.controller().applications().deactivate(userApp.get().id().defaultInstance(), + userApp.get().productionDeployments().values().stream() + .flatMap(List::stream) + .findAny().get().zone()); + assertTrue("No production deployments are listed for user.", userApp.get().require(InstanceName.defaultName()).productionDeployments().isEmpty()); confirmer.maintain(); // Time has passed, and a new confirmation issue is in order for the property which is still in production. diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java index cf33b354139..73ddeeaa0c6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; @@ -40,8 +41,9 @@ public class DeploymentMetricsMaintainerTest { deploy(application.id().defaultInstance(), Version.fromString("7.1")); DeploymentMetricsMaintainer maintainer = maintainer(tester.controller()); - Supplier<Instance> app = () -> tester.defaultInstance(application.id()); - Supplier<Deployment> deployment = () -> app.get().deployments().values().stream().findFirst().get(); + Supplier<Application> app = () -> tester.application(application.id()); + Supplier<Instance> instance = () -> tester.defaultInstance(application.id()); + Supplier<Deployment> deployment = () -> instance.get().deployments().values().stream().findFirst().get(); // No metrics gathered yet assertEquals(0, app.get().metrics().queryServiceQuality(), 0); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java index 02282894544..bec2c9c7bc4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java @@ -45,8 +45,8 @@ public class OutstandingChangeDeployerTest { tester.deploymentTrigger().triggerChange(app1.id(), Change.of(version)); tester.deploymentTrigger().triggerReadyJobs(); - assertEquals(Change.of(version), tester.defaultInstance("app1").change()); - assertFalse(tester.defaultInstance("app1").outstandingChange().hasTargets()); + assertEquals(Change.of(version), tester.application(app1.id()).change()); + assertFalse(tester.application(app1.id()).outstandingChange().hasTargets()); tester.jobCompletion(JobType.component) .application(app1) @@ -56,14 +56,14 @@ public class OutstandingChangeDeployerTest { .submit(); Instance instance = tester.defaultInstance("app1"); - assertTrue(instance.outstandingChange().hasTargets()); - assertEquals("1.0.43-cafed00d", instance.outstandingChange().application().get().id()); + assertTrue(tester.application(app1.id()).outstandingChange().hasTargets()); + assertEquals("1.0.43-cafed00d", tester.application(app1.id()).outstandingChange().application().get().id()); assertEquals(2, tester.buildService().jobs().size()); deployer.maintain(); tester.deploymentTrigger().triggerReadyJobs(); assertEquals("No effect as job is in progress", 2, tester.buildService().jobs().size()); - assertEquals("1.0.43-cafed00d", instance.outstandingChange().application().get().id()); + assertEquals("1.0.43-cafed00d", tester.application(app1.id()).outstandingChange().application().get().id()); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, JobType.systemTest); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, JobType.stagingTest); @@ -75,12 +75,12 @@ public class OutstandingChangeDeployerTest { deployer.maintain(); tester.deploymentTrigger().triggerReadyJobs(); instance = tester.defaultInstance("app1"); - assertEquals("1.0.43-cafed00d", instance.change().application().get().id()); + assertEquals("1.0.43-cafed00d", tester.application(app1.id()).change().application().get().id()); List<BuildService.BuildJob> jobs = tester.buildService().jobs(); assertEquals(1, jobs.size()); assertEquals(JobType.productionUsWest1.jobName(), jobs.get(0).jobName()); assertEquals(app1.id().defaultInstance(), jobs.get(0).applicationId()); - assertFalse(tester.defaultInstance("app1").outstandingChange().hasTargets()); + assertFalse(tester.application(app1.id()).outstandingChange().hasTargets()); } } 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 e247c2af3c9..d979f1753c4 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 @@ -204,11 +204,11 @@ public class UpgraderTest { tester.triggerUntilQuiescence(); assertEquals("Upgrade of defaults are scheduled", 10, tester.buildService().jobs().size()); - assertEquals(version4, tester.defaultInstance(default0.id()).change().platform().get()); - assertEquals(version4, tester.defaultInstance(default1.id()).change().platform().get()); - assertEquals(version4, tester.defaultInstance(default2.id()).change().platform().get()); - assertEquals(version4, tester.defaultInstance(default3.id()).change().platform().get()); - assertEquals(version4, tester.defaultInstance(default4.id()).change().platform().get()); + assertEquals(version4, tester.application(default0.id()).change().platform().get()); + assertEquals(version4, tester.application(default1.id()).change().platform().get()); + assertEquals(version4, tester.application(default2.id()).change().platform().get()); + assertEquals(version4, tester.application(default3.id()).change().platform().get()); + assertEquals(version4, tester.application(default4.id()).change().platform().get()); tester.completeUpgrade(default0, version4, "default"); // State: Default applications started upgrading to version4 (and one completed) @@ -224,11 +224,11 @@ public class UpgraderTest { tester.triggerUntilQuiescence(); assertEquals("Upgrade of defaults are scheduled", 10, tester.buildService().jobs().size()); - assertEquals(version5, tester.defaultInstance(default0.id()).change().platform().get()); - assertEquals(version4, tester.defaultInstance(default1.id()).change().platform().get()); - assertEquals(version4, tester.defaultInstance(default2.id()).change().platform().get()); - assertEquals(version4, tester.defaultInstance(default3.id()).change().platform().get()); - assertEquals(version4, tester.defaultInstance(default4.id()).change().platform().get()); + assertEquals(version5, tester.application(default0.id()).change().platform().get()); + assertEquals(version4, tester.application(default1.id()).change().platform().get()); + assertEquals(version4, tester.application(default2.id()).change().platform().get()); + assertEquals(version4, tester.application(default3.id()).change().platform().get()); + assertEquals(version4, tester.application(default4.id()).change().platform().get()); tester.completeUpgrade(default1, version4, "default"); tester.completeUpgrade(default2, version4, "default"); @@ -260,7 +260,7 @@ public class UpgraderTest { assertEquals("Upgrade of defaults are scheduled on " + version4 + " instead, since " + version5 + " is broken: " + "This is default3 since it failed upgrade on both " + version4 + " and " + version5, 2, tester.buildService().jobs().size()); - assertEquals(version4, tester.defaultInstance(default3.id()).change().platform().get()); + assertEquals(version4, tester.application(default3.id()).change().platform().get()); } @Test @@ -371,7 +371,7 @@ public class UpgraderTest { // staging-test fails and failure is recorded tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), false, stagingTest); assertTrue("Failure is recorded", tester.defaultInstance(app.id()).deploymentJobs().hasFailures()); - assertTrue("Application has pending change", tester.defaultInstance(app.id()).change().hasTargets()); + assertTrue("Application has pending change", tester.application(app.id()).change().hasTargets()); // New version is released version = Version.fromString("6.4"); @@ -435,7 +435,7 @@ public class UpgraderTest { tester.triggerUntilQuiescence(); // apps pass system-test, but do not trigger next jobs as upgrade is cancelled - assertFalse("No change present", tester.applications().requireInstance(default4.id().defaultInstance()).change().hasTargets()); + assertFalse("No change present", tester.application(default4.id()).change().hasTargets()); tester.jobCompletion(systemTest).application(default0).submit(); tester.jobCompletion(systemTest).application(default1).submit(); tester.jobCompletion(systemTest).application(default2).submit(); @@ -510,7 +510,7 @@ public class UpgraderTest { tester.upgrader().maintain(); tester.triggerUntilQuiescence(); assertEquals("Upgrade scheduled for remaining apps", 10, tester.buildService().jobs().size()); - assertEquals("default4 is still upgrading to 5.1", v1, tester.defaultInstance(default4.id()).change().platform().get()); + assertEquals("default4 is still upgrading to 5.1", v1, tester.application(default4.id()).change().platform().get()); // 4/5 applications fail (in the last prod zone) and lowers confidence tester.completeUpgradeWithError(default0, v2, "default", productionUsEast3); @@ -538,7 +538,7 @@ public class UpgraderTest { assertEquals(v2, tester.defaultInstance("default0").deployments().get(ZoneId.from("prod.us-west-1")).version()); assertEquals("Last zone is upgraded to v1", v1, tester.defaultInstance("default0").deployments().get(ZoneId.from("prod.us-east-3")).version()); - assertFalse(tester.defaultInstance("default0").change().hasTargets()); + assertFalse(tester.application(default0.id()).change().hasTargets()); } @Test @@ -822,9 +822,8 @@ public class UpgraderTest { tester.jobCompletion(stagingTest).application(default4).unsuccessful().submit(); // 5th app never reports back and has a dead job, but no ongoing change - Instance deadLocked = tester.applications().requireInstance(default4.id().defaultInstance()); - tester.assertRunning(systemTest, deadLocked.id()); - assertFalse("No change present", deadLocked.change().hasTargets()); + tester.assertRunning(systemTest, tester.defaultInstance(default4.id()).id()); + assertFalse("No change present", tester.application(default4.id()).change().hasTargets()); // 4 out of 5 applications are repaired and confidence is restored ApplicationPackage defaultApplicationPackageV2 = new ApplicationPackageBuilder() @@ -850,10 +849,10 @@ public class UpgraderTest { tester.completeUpgrade(default2, version, defaultApplicationPackageV2); tester.completeUpgrade(default3, version, defaultApplicationPackageV2); - assertEquals(version, tester.defaultInstance(default0.id()).oldestDeployedPlatform().get()); - assertEquals(version, tester.defaultInstance(default1.id()).oldestDeployedPlatform().get()); - assertEquals(version, tester.defaultInstance(default2.id()).oldestDeployedPlatform().get()); - assertEquals(version, tester.defaultInstance(default3.id()).oldestDeployedPlatform().get()); + assertEquals(version, tester.application(default0.id()).oldestDeployedPlatform().get()); + assertEquals(version, tester.application(default1.id()).oldestDeployedPlatform().get()); + assertEquals(version, tester.application(default2.id()).oldestDeployedPlatform().get()); + assertEquals(version, tester.application(default3.id()).oldestDeployedPlatform().get()); } @Test @@ -959,7 +958,7 @@ public class UpgraderTest { Application canary0 = tester.createAndDeploy("canary0", 1, "canary"); Application default0 = tester.createAndDeploy("default0", 2, default0ApplicationPackage); tester.applications().lockApplicationOrThrow(default0.id(), a -> tester.applications().store(a.withMajorVersion(6))); - assertEquals(OptionalInt.of(6), tester.applications().getInstance(default0.id().defaultInstance()).get().majorVersion()); + assertEquals(OptionalInt.of(6), tester.application(default0.id()).majorVersion()); // New major version is released version = Version.fromString("7.0"); @@ -1179,16 +1178,16 @@ public class UpgraderTest { assertEquals(Collections.emptyList(), tester.buildService().jobs()); // No jobs left. tester.outstandingChangeDeployer().run(); - assertFalse(tester.defaultInstance(app.id()).change().hasTargets()); + assertFalse(tester.application(app.id()).change().hasTargets()); tester.clock().advance(Duration.ofHours(2)); tester.outstandingChangeDeployer().run(); - assertTrue(tester.defaultInstance(app.id()).change().hasTargets()); + assertTrue(tester.application(app.id()).change().hasTargets()); tester.readyJobTrigger().run(); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, productionUsWest1); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, productionUsCentral1); tester.deployAndNotify(instance.id(), Optional.of(applicationPackage), true, productionUsEast3); - assertFalse(tester.defaultInstance(app.id()).change().hasTargets()); + assertFalse(tester.application(app.id()).change().hasTargets()); } @Test @@ -1207,33 +1206,33 @@ public class UpgraderTest { tester.deploymentTrigger().forceChange(application.id(), Change.empty().withPin()); tester.deployCompletely(application, applicationPackage); - assertFalse(tester.defaultInstance(application.id()).change().hasTargets()); - assertTrue(tester.defaultInstance(application.id()).change().isPinned()); + assertFalse(tester.application(application.id()).change().hasTargets()); + assertTrue(tester.application(application.id()).change().isPinned()); assertEquals(2, tester.defaultInstance(application.id()).deployments().size()); // Application does not upgrade. Version version1 = Version.fromString("6.3"); tester.upgradeSystem(version1); tester.upgrader().maintain(); - assertFalse(tester.defaultInstance(application.id()).change().hasTargets()); - assertTrue(tester.defaultInstance(application.id()).change().isPinned()); + assertFalse(tester.application(application.id()).change().hasTargets()); + assertTrue(tester.application(application.id()).change().isPinned()); // New application package is deployed. tester.deployCompletely(application, applicationPackage, BuildJob.defaultBuildNumber + 1); - assertFalse(tester.defaultInstance(application.id()).change().hasTargets()); - assertTrue(tester.defaultInstance(application.id()).change().isPinned()); + assertFalse(tester.application(application.id()).change().hasTargets()); + assertTrue(tester.application(application.id()).change().isPinned()); // Application upgrades to new version when pin is removed. tester.deploymentTrigger().cancelChange(application.id(), PIN); tester.upgrader().maintain(); - assertTrue(tester.defaultInstance(application.id()).change().hasTargets()); - assertFalse(tester.defaultInstance(application.id()).change().isPinned()); + assertTrue(tester.application(application.id()).change().hasTargets()); + assertFalse(tester.application(application.id()).change().isPinned()); // Application is pinned to new version, and upgrade is therefore not cancelled, even though confidence is broken. tester.deploymentTrigger().forceChange(application.id(), Change.empty().withPin()); tester.upgrader().maintain(); tester.readyJobTrigger().maintain(); - assertEquals(version1, tester.defaultInstance(application.id()).change().platform().get()); + assertEquals(version1, tester.application(application.id()).change().platform().get()); // Application fails upgrade after one zone is complete, and is pinned again to the old version. tester.deployAndNotify(instance.id(), Optional.empty(), true, systemTest); @@ -1244,16 +1243,16 @@ public class UpgraderTest { tester.deploymentTrigger().cancelChange(application.id(), ALL); tester.deploymentTrigger().forceChange(application.id(), Change.of(version0).withPin()); tester.buildService().clear(); - assertEquals(version0, tester.defaultInstance(application.id()).change().platform().get()); + assertEquals(version0, tester.application(application.id()).change().platform().get()); // Application downgrades to pinned version. tester.readyJobTrigger().maintain(); tester.deployAndNotify(instance.id(), Optional.empty(), true, systemTest); tester.deployAndNotify(instance.id(), Optional.empty(), true, stagingTest); tester.deployAndNotify(instance.id(), Optional.empty(), true, productionUsEast3); - assertTrue(tester.defaultInstance(application.id()).change().hasTargets()); + assertTrue(tester.application(application.id()).change().hasTargets()); tester.deployAndNotify(instance.id(), Optional.empty(), true, productionUsWest1); - assertFalse(tester.defaultInstance(application.id()).change().hasTargets()); + assertFalse(tester.application(application.id()).change().hasTargets()); } @Test @@ -1288,14 +1287,14 @@ public class UpgraderTest { // App 2 is allowed on new major and upgrades tester.controller().applications().lockApplicationIfPresent(app2.id(), app -> tester.applications().store(app.withMajorVersion(7))); tester.upgrader().maintain(); - assertEquals(version2, tester.controller().applications().requireInstance(app2.id().defaultInstance()).change().platform().get()); + assertEquals(version2, tester.application(app2.id()).change().platform().orElseThrow()); // App 1 is unpinned and upgrades to latest 6 tester.controller().applications().lockApplicationIfPresent(app1.id(), app -> tester.controller().applications().store(app.withChange(app.get().change().withoutPin()))); tester.upgrader().maintain(); assertEquals("Application upgrades to latest allowed major", version1, - tester.controller().applications().requireInstance(app1.id().defaultInstance()).change().platform().get()); + tester.application(app1.id()).change().platform().orElseThrow()); } @Test @@ -1303,7 +1302,8 @@ public class UpgraderTest { DeploymentTester tester = new DeploymentTester(); Application application = tester.createApplication("app1", "tenant1", 1, 1L); Instance instance = tester.defaultInstance(application.id()); - Supplier<Instance> app = () -> tester.defaultInstance(application.id()); + Supplier<Application> applications = () -> tester.application(application.id()); + Supplier<Instance> instances = () -> tester.defaultInstance(application.id()); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .region("us-central-1") @@ -1319,7 +1319,7 @@ public class UpgraderTest { Version v2 = Version.fromString("6.2"); tester.upgradeSystem(v2); tester.upgrader().maintain(); - assertEquals(Change.of(v2), app.get().change()); + assertEquals(Change.of(v2), applications.get().change()); tester.deployAndNotify(instance.id(), Optional.empty(), true, systemTest); tester.deployAndNotify(instance.id(), Optional.empty(), true, stagingTest); tester.deployAndNotify(instance.id(), Optional.empty(), true, productionUsCentral1); @@ -1329,13 +1329,13 @@ public class UpgraderTest { tester.computeVersionStatus(); tester.upgrader().maintain(); tester.deployAndNotify(instance.id(), Optional.empty(), true, productionUsWest1); - assertTrue(app.get().change().isEmpty()); + assertTrue(applications.get().change().isEmpty()); // Next version is released Version v3 = Version.fromString("6.3"); tester.upgradeSystem(v3); tester.upgrader().maintain(); - assertEquals(Change.of(v3), app.get().change()); + assertEquals(Change.of(v3), applications.get().change()); tester.deployAndNotify(instance.id(), Optional.empty(), true, systemTest); tester.deployAndNotify(instance.id(), Optional.empty(), true, stagingTest); @@ -1345,22 +1345,22 @@ public class UpgraderTest { // Before deployment completes, v1->v3 combination is tested as us-east-3 is still on v1 tester.readyJobTrigger().maintain(); tester.deployAndNotify(instance.id(), Optional.empty(), true, stagingTest); - assertEquals(v1, app.get().deploymentJobs().jobStatus().get(stagingTest).lastSuccess().get().sourcePlatform().get()); - assertEquals(v3, app.get().deploymentJobs().jobStatus().get(stagingTest).lastSuccess().get().platform()); + assertEquals(v1, instances.get().deploymentJobs().jobStatus().get(stagingTest).lastSuccess().get().sourcePlatform().get()); + assertEquals(v3, instances.get().deploymentJobs().jobStatus().get(stagingTest).lastSuccess().get().platform()); // First deployment fails and then successfully upgrades to v3 tester.jobCompletion(productionUsCentral1).application(application).unsuccessful().submit(); tester.jobCompletion(productionUsCentral1).application(application).submit(); // Deployments are now on 3 versions - assertEquals(v3, app.get().deployments().get(productionUsCentral1.zone(main)).version()); - assertEquals(v2, app.get().deployments().get(productionUsWest1.zone(main)).version()); - assertEquals(v1, app.get().deployments().get(productionUsEast3.zone(main)).version()); + assertEquals(v3, instances.get().deployments().get(productionUsCentral1.zone(main)).version()); + assertEquals(v2, instances.get().deployments().get(productionUsWest1.zone(main)).version()); + assertEquals(v1, instances.get().deployments().get(productionUsEast3.zone(main)).version()); // Need to test v2->v3 combination again before upgrading second deployment tester.readyJobTrigger().maintain(); - assertEquals(v2, app.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().sourcePlatform().get()); - assertEquals(v3, app.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().platform()); + assertEquals(v2, instances.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().sourcePlatform().get()); + assertEquals(v3, instances.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().platform()); tester.deployAndNotify(instance.id(), Optional.empty(), true, stagingTest); // Second deployment upgrades @@ -1368,13 +1368,13 @@ public class UpgraderTest { // ... now we have to test v1->v3 again :( tester.readyJobTrigger().maintain(); - assertEquals(v1, app.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().sourcePlatform().get()); - assertEquals(v3, app.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().platform()); + assertEquals(v1, instances.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().sourcePlatform().get()); + assertEquals(v3, instances.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().platform()); tester.deployAndNotify(instance.id(), Optional.empty(), true, stagingTest); // Upgrade completes tester.deployAndNotify(instance.id(), Optional.empty(), true, productionUsEast3); - assertTrue("Upgrade complete", app.get().change().isEmpty()); + assertTrue("Upgrade complete", applications.get().change().isEmpty()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OldCuratorDb.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OldCuratorDb.java index 8aea72a9ac4..f7fdd91d74e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OldCuratorDb.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OldCuratorDb.java @@ -6,6 +6,7 @@ import com.google.inject.Inject; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.path.Path; @@ -13,11 +14,13 @@ import com.yahoo.slime.Slime; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.auditlog.AuditLog; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.Step; @@ -63,7 +66,7 @@ import static java.util.stream.Collectors.collectingAndThen; */ public class OldCuratorDb { - private static final Logger log = Logger.getLogger(OldCuratorDb.class.getName()); + private static final Logger log = Logger.getLogger(CuratorDb.class.getName()); private static final Duration deployLockTimeout = Duration.ofMinutes(30); private static final Duration defaultLockTimeout = Duration.ofMinutes(5); private static final Duration defaultTryLockTimeout = Duration.ofSeconds(1); @@ -83,6 +86,7 @@ public class OldCuratorDb { private final ControllerVersionSerializer controllerVersionSerializer = new ControllerVersionSerializer(); private final ConfidenceOverrideSerializer confidenceOverrideSerializer = new ConfidenceOverrideSerializer(); private final TenantSerializer tenantSerializer = new TenantSerializer(); + private final ApplicationSerializer applicationSerializer = new ApplicationSerializer(); private final InstanceSerializer instanceSerializer = new InstanceSerializer(); private final RunSerializer runSerializer = new RunSerializer(); private final OsVersionSerializer osVersionSerializer = new OsVersionSerializer(); @@ -133,6 +137,10 @@ public class OldCuratorDb { return lock(lockPath(name), defaultLockTimeout.multipliedBy(2)); } + public Lock lock(TenantAndApplicationId id) { + return lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); + } + public Lock lock(ApplicationId id) { return lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); } @@ -329,45 +337,97 @@ public class OldCuratorDb { curator.delete(tenantPath(name)); } - // -------------- Application --------------------------------------------- + // -------------- Applications --------------------------------------------- - public void writeInstance(Instance instance) { - curator.set(applicationPath(instance.id()), asJson(instanceSerializer.toSlime(instance))); - curator.set(instancePath(instance.id()), asJson(instanceSerializer.toSlime(instance))); + public void writeApplication(Application application) { + curator.set(applicationPath(application.id()), asJson(applicationSerializer.toSlime(application))); + for (InstanceName name : application.instances().keySet()) { + curator.set(oldApplicationPath(application.id().instance(name)), + asJson(instanceSerializer.toSlime(application.legacy(name)))); + } } - public Optional<Instance> readInstance(ApplicationId application) { - return readSlime(applicationPath(application)).map(instanceSerializer::fromSlime); + public Optional<Application> readApplication(TenantAndApplicationId application) { + List<Instance> instances = readInstances(id -> TenantAndApplicationId.from(id).equals(application)); + return Application.aggregate(instances); } - public List<Instance> readInstances() { - return readInstances(ignored -> true); + public List<Application> readApplications() { + return readApplications(ignored -> true); } - public List<Instance> readInstances(TenantName name) { - return readInstances(application -> application.tenant().equals(name)); + public List<Application> readApplications(TenantName name) { + return readApplications(application -> application.tenant().equals(name)); + } + + private Stream<TenantAndApplicationId> readTenantAndApplicationIds() { + return readInstanceIds().map(TenantAndApplicationId::from).distinct(); + } + + private List<Application> readApplications(Predicate<TenantAndApplicationId> applicationFilter) { + return readTenantAndApplicationIds().filter(applicationFilter) + .sorted() + .map(this::readApplication) + .flatMap(Optional::stream) + .collect(Collectors.toUnmodifiableList()); + } + + private Optional<Instance> readInstance(ApplicationId application) { + return readSlime(oldApplicationPath(application)).map(instanceSerializer::fromSlime); } private Stream<ApplicationId> readInstanceIds() { return curator.getChildren(applicationRoot).stream() .filter(id -> id.split(":").length == 3) - .distinct() .map(ApplicationId::fromSerializedForm); } - private List<Instance> readInstances(Predicate<ApplicationId> instanceFilter) { - return readInstanceIds().filter(instanceFilter) + private List<Instance> readInstances(Predicate<ApplicationId> applicationFilter) { + return readInstanceIds().filter(applicationFilter) + .sorted() .map(this::readInstance) .flatMap(Optional::stream) .collect(Collectors.toUnmodifiableList()); } - public void removeInstance(ApplicationId id) { + public void removeApplication(ApplicationId id) { // WARNING: This is part of a multi-step data move operation, so don't touch!!! - curator.delete(applicationPath(id)); - curator.delete(instancePath(id)); + curator.delete(oldApplicationPath(id)); + if (readApplication(TenantAndApplicationId.from(id)).isEmpty()) + curator.delete(applicationPath(TenantAndApplicationId.from(id))); } + public void clearInstanceRoot() { + curator.delete(instanceRoot); + } + + /** + * Migration plan: + * + * Add filter for reading only Instance from old application path RELEASED + * Write Instance to instance and old application path RELEASED + * + * Lock on application level for instance mutations MERGED + * + * Write Instance to instance and application and old application paths DONE TO CHANGE DONE + * Read Instance from instance path DONE TO REMOVE DONE + * Duplicate Application from Instance, with helper classes DONE + * Write Application to instance and application and old application paths DONE TO CHANGE DONE + * Read Application from instance path DONE TO REMOVE DONE + * Use Application where applicable DONE !!! + * Lock instances and application on same level: tenant + application DONE TO CHANGE DONE + * When reading an application, read all instances, and aggregate them DONE + * Write application with instances to application path DONE + * Write all instances of an application to old application path DONE + * Remove everything under instance root DONE + * + * Read Application with instances from application path (with filter) + * Stop locking applications on instance level + * + * Stop writing Instance to old application path + * Remove unused parts of Instance (Used only for legacy serialization) + */ + // -------------- Job Runs ------------------------------------------------ public void writeLastRun(Run run) { @@ -503,10 +563,14 @@ public class OldCuratorDb { .append(tenant.value()); } - private Path lockPath(ApplicationId application) { + private Path lockPath(TenantAndApplicationId application) { return lockPath(application.tenant()) - .append(application.application().value()) - .append(application.instance().value()); + .append(application.application().value()); + } + + private Path lockPath(ApplicationId instance) { + return lockPath(TenantAndApplicationId.from(instance)) + .append(instance.instance().value()); } private Path lockPath(ApplicationId application, ZoneId zone) { @@ -583,7 +647,11 @@ public class OldCuratorDb { return tenantRoot.append(name.value()); } - private static Path applicationPath(ApplicationId application) { + private static Path applicationPath(TenantAndApplicationId id) { + return applicationRoot.append(id.serialized()); + } + + private static Path oldApplicationPath(ApplicationId application) { return applicationRoot.append(application.serializedForm()); } @@ -616,3 +684,4 @@ public class OldCuratorDb { } } + diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java index fb5f28e1963..6cb808133e3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java @@ -55,7 +55,7 @@ public class JobControllerApiHandlerHelperTest { // Revision 1 gets deployed everywhere. ApplicationVersion revision1 = tester.deployNewSubmission(); - assertEquals(2, tester.instance().deploymentJobs().projectId().getAsLong()); + assertEquals(2, tester.application().projectId().getAsLong()); tester.clock().advance(Duration.ofMillis(1000)); // Revision 2 gets deployed everywhere except in us-east-3. diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java index 4fb3c8f0913..6fd4beac056 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java @@ -1,11 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.docproc.jdisc; -import com.yahoo.docproc.jdisc.metric.NullMetric; import com.yahoo.document.DocumentUtil; -import com.yahoo.jdisc.Metric; import com.yahoo.log.LogLevel; -import com.yahoo.statistics.*; import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Logger; @@ -22,12 +19,6 @@ class DocprocThreadManager { private final AtomicLong bytesFinished = new AtomicLong(0); DocprocThreadManager(double maxConcurrentFactor, double documentExpansionFactor, int containerCoreMemoryMb) { - this(maxConcurrentFactor, documentExpansionFactor, containerCoreMemoryMb, Statistics.nullImplementation, - new NullMetric()); - } - - DocprocThreadManager(double maxConcurrentFactor, double documentExpansionFactor, int containerCoreMemoryMb, - Statistics statistics, Metric metric) { this((long) (((double) DocumentUtil.calculateMaxPendingSize(maxConcurrentFactor, documentExpansionFactor, containerCoreMemoryMb)) * maxConcurrentFactor)); } diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java index 23ab73c7f1d..e1a902c8d5c 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java @@ -20,7 +20,7 @@ public class DocprocThreadPoolExecutor extends ThreadPoolExecutor { public DocprocThreadPoolExecutor(int maxNumThreads, BlockingQueue<Runnable> queue, DocprocThreadManager threadMgr) { super((maxNumThreads > 0) ? maxNumThreads : Runtime.getRuntime().availableProcessors(), - (maxNumThreads > 0) ? maxNumThreads : 8192, + (maxNumThreads > 0) ? maxNumThreads : 2048, 1, TimeUnit.SECONDS, queue, new DaemonThreadFactory("docproc-")); diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java index ec74fb9246d..5b7b9d85a91 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java @@ -19,7 +19,6 @@ import com.yahoo.docproc.jdisc.messagebus.MbusRequestContext; import com.yahoo.docproc.proxy.SchemaMap; import com.yahoo.document.DocumentTypeManager; import com.yahoo.document.config.DocumentmanagerConfig; -import com.yahoo.documentapi.ThroughputLimitQueue; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.Request; import com.yahoo.jdisc.handler.AbstractRequestHandler; @@ -33,7 +32,6 @@ import com.yahoo.statistics.Statistics; import java.util.TimerTask; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.PriorityBlockingQueue; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.SynchronousQueue; @@ -96,29 +94,20 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { DocumentProcessingHandlerParameters params) { this(docprocServiceRegistry, documentProcessorComponentRegistry, docFactoryRegistry, new DocprocThreadPoolExecutor(params.getMaxNumThreads(), - chooseQueueType(params), + chooseQueueType(params.getMaxNumThreads()), new DocprocThreadManager(params.getMaxConcurrentFactor(), params.getDocumentExpansionFactor(), - params.getContainerCoreMemoryMb(), - params.getStatisticsManager(), - params.getMetric())), + params.getContainerCoreMemoryMb())), params.getDocumentTypeManager(), params.getChainsModel(), params.getSchemaMap(), params.getStatisticsManager(), params.getMetric(), params.getContainerDocConfig()); } - private static BlockingQueue<Runnable> chooseQueueType(DocumentProcessingHandlerParameters params) { - if (params.getMaxQueueTimeMs() > 0) { - return new ThroughputLimitQueue<>(params.getMaxQueueTimeMs()); - } - if (params.getMaxQueueTimeMs() == 0) { - return new PriorityBlockingQueue<>(); // Probably no need to bound this queue, see bug #4254537 - } - if (params.getMaxNumThreads() > 0) { - return new LinkedBlockingQueue<>(); - } - return new SynchronousQueue<>(); + private static BlockingQueue<Runnable> chooseQueueType(int maxNumThreads) { + return (maxNumThreads > 0) + ? new LinkedBlockingQueue<>() + : new SynchronousQueue<>(); } @Inject @@ -138,7 +127,6 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { .setMaxConcurrentFactor(containerMbusConfig.maxConcurrentFactor()) .setDocumentExpansionFactor(containerMbusConfig.documentExpansionFactor()) .setContainerCoreMemoryMb(containerMbusConfig.containerCoreMemory()) - .setMaxQueueTimeMs(docprocConfig.maxqueuetimems()) .setDocumentTypeManager(new DocumentTypeManager(docManConfig)) .setChainsModel(buildFromConfig(chainsConfig)).setSchemaMap(configureMapping(mappingConfig)) .setStatisticsManager(manager) diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java index cec943306c8..bf308e39218 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java @@ -21,7 +21,6 @@ public class DocumentProcessingHandlerParameters { private double maxConcurrentFactor = 0.2; private double documentExpansionFactor = 20.0; private int containerCoreMemoryMb = 50; - private long maxQueueTimeMs = 0; private DocumentTypeManager documentTypeManager = null; private ChainsModel chainsModel = null; private SchemaMap schemaMap = null; @@ -86,21 +85,6 @@ public class DocumentProcessingHandlerParameters { } /** - * Returns the maximum time (in milliseconds) that a document may stay in the input queue. The default value - * of 0 disables this functionality. - * - * @return the maximum time (in milliseconds) that a document may stay in the input queue. - */ - public long getMaxQueueTimeMs() { - return maxQueueTimeMs; - } - - public DocumentProcessingHandlerParameters setMaxQueueTimeMs(long maxQueueTimeMs) { - this.maxQueueTimeMs = maxQueueTimeMs; - return this; - } - - /** * Returns the maximum number of thread that the thread pool will ever attempt to run simultaneously. * * @return the maximum number of thread that the thread pool will ever attempt to run simultaneously. diff --git a/docproc/src/main/resources/configdefinitions/docproc.def b/docproc/src/main/resources/configdefinitions/docproc.def index 645fbcce1b7..53f5c2de21f 100644 --- a/docproc/src/main/resources/configdefinitions/docproc.def +++ b/docproc/src/main/resources/configdefinitions/docproc.def @@ -6,7 +6,9 @@ namespace=config.docproc # 0 Gives a PriorityQueue # Negative values gives a SynchronousQueue if numthreads <= 0 (which increases thread pool size as required), # otherwise a LinkedBlockingQueue +# Depreacated and a noop maxqueuetimems int default=-1 # The number of threads in the DocprocHandler worker thread pool +# Default is number of cpu's, but is dynamic up to 2048 numthreads int default=-1 diff --git a/documentapi/abi-spec.json b/documentapi/abi-spec.json index dc8daf737e4..80fccbd9161 100644 --- a/documentapi/abi-spec.json +++ b/documentapi/abi-spec.json @@ -524,28 +524,6 @@ ], "fields": [] }, - "com.yahoo.documentapi.ThroughputLimitQueue": { - "superClass": "java.util.concurrent.LinkedBlockingQueue", - "interfaces": [], - "attributes": [ - "public" - ], - "methods": [ - "public void <init>(long)", - "public void <init>(com.yahoo.concurrent.Timer, long)", - "public boolean add(java.lang.Object)", - "public boolean offer(java.lang.Object)", - "public java.lang.Object poll()", - "public void put(java.lang.Object)", - "public boolean offer(java.lang.Object, long, java.util.concurrent.TimeUnit)", - "public java.lang.Object take()", - "public java.lang.Object poll(long, java.util.concurrent.TimeUnit)", - "public int capacity()", - "public int remainingCapacity()", - "public boolean addAll(java.util.Collection)" - ], - "fields": [] - }, "com.yahoo.documentapi.UpdateResponse": { "superClass": "com.yahoo.documentapi.Response", "interfaces": [], diff --git a/documentapi/src/main/java/com/yahoo/documentapi/ThroughputLimitQueue.java b/documentapi/src/main/java/com/yahoo/documentapi/ThroughputLimitQueue.java deleted file mode 100644 index 579cdaced04..00000000000 --- a/documentapi/src/main/java/com/yahoo/documentapi/ThroughputLimitQueue.java +++ /dev/null @@ -1,164 +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.documentapi; - -import com.yahoo.concurrent.SystemTimer; -import com.yahoo.concurrent.Timer; - -import java.util.Collection; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.TimeUnit; -import java.util.logging.Logger; - -/** - * Queue that limits it's size based on the throughput. Allows the queue to keep a certain number of - * seconds of processing in its queue. - */ -public class ThroughputLimitQueue<M> extends LinkedBlockingQueue<M> { - private static Logger log = Logger.getLogger(ThroughputLimitQueue.class.getName()); - - double averageWaitTime = 0; - long maxWaitTime = 0; - long startTime; - int capacity = 2; - Timer timer; - - /** - * Creates a new queue. - * - * @param queueSizeInMs The maximum time we wish to have objects waiting in the queue. - */ - public ThroughputLimitQueue(long queueSizeInMs) { - this(SystemTimer.INSTANCE, queueSizeInMs); - } - - /** - * Creates a new queue. Used for unit testing. - * - * @param t Used to measure time spent in the queue. Subclass for unit testing, or use SystemTimer for regular use. - * @param queueSizeInMs The maximum time we wish to have objects waiting in the queue. - */ - public ThroughputLimitQueue(Timer t, long queueSizeInMs) { - maxWaitTime = queueSizeInMs; - timer = t; - } - - // Doc inherited from BlockingQueue - public boolean add(M m) { - if (!offer(m)) { - throw new IllegalStateException("Queue full"); - } - return true; - } - - // Doc inherited from BlockingQueue - public boolean offer(M m) { - return remainingCapacity() > 0 && super.offer(m); - } - - /** - * Calculates the average waiting time and readjusts the queue capacity. - * - * @param m The last message that was read from queue, if any. - * @return Returns m. - */ - private M calculateAverage(M m) { - if (m == null) { - startTime = 0; - return null; - } - - if (startTime != 0) { - long waitTime = timer.milliTime() - startTime; - - if (averageWaitTime == 0) { - averageWaitTime = waitTime; - } else { - averageWaitTime = 0.99 * averageWaitTime + 0.01 * waitTime; - } - - int newCapacity = Math.max(2, (int)Math.round(maxWaitTime / averageWaitTime)); - if (newCapacity != capacity) { - log.fine("Capacity of throughput queue changed from " + capacity + " to " + newCapacity); - capacity = newCapacity; - } - } - - if (!isEmpty()) { - startTime = timer.milliTime(); - } else { - startTime = 0; - } - - return m; - } - - // Doc inherited from BlockingQueue - public M poll() { - return calculateAverage(super.poll()); - } - - // Doc inherited from BlockingQueue - public void put(M m) throws InterruptedException { - offer(m, Long.MAX_VALUE, TimeUnit.SECONDS); - } - - // Doc inherited from BlockingQueue - public boolean offer(M m, long l, TimeUnit timeUnit) throws InterruptedException { - long timeWaited = 0; - while (timeWaited < timeUnit.toMillis(l)) { - if (offer(m)) { - return true; - } - - Thread.sleep(10); - timeWaited += 10; - } - - return false; - } - - // Doc inherited from BlockingQueue - public M take() throws InterruptedException { - return poll(Long.MAX_VALUE, TimeUnit.SECONDS); - } - - // Doc inherited from BlockingQueue - public M poll(long l, TimeUnit timeUnit) throws InterruptedException { - long timeWaited = 0; - while (timeWaited < timeUnit.toMillis(l)) { - M elem = poll(); - if (elem != null) { - return elem; - } - - Thread.sleep(10); - timeWaited += 10; - } - - return null; - } - - /** - * @return Returns the maximum capacity of the queue - */ - public int capacity() { - return capacity; - } - - // Doc inherited from BlockingQueue - public int remainingCapacity() { - int sz = capacity - size(); - return (sz > 0) ? sz : 0; - } - - // Doc inherited from BlockingQueue - public boolean addAll(Collection<? extends M> ms) { - for (M m : ms) { - if (!offer(m)) { - return false; - } - } - - return true; - } -} diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/test/ThroughputLimitQueueTestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/test/ThroughputLimitQueueTestCase.java deleted file mode 100644 index 75e98bc895a..00000000000 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/test/ThroughputLimitQueueTestCase.java +++ /dev/null @@ -1,49 +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.documentapi.messagebus.test; - -import com.yahoo.documentapi.ThroughputLimitQueue; -import com.yahoo.concurrent.Timer; -import org.junit.Test; - -import static org.junit.Assert.assertEquals; - -/** - * @author thomasg - */ -public class ThroughputLimitQueueTestCase { - - class TestTimer implements Timer { - public long milliTime = 0; - - public long milliTime() { - return milliTime; - } - } - - @Test - public void testCapacity() { - TestTimer t = new TestTimer(); - t.milliTime = 10; - ThroughputLimitQueue<Object> q = new ThroughputLimitQueue<Object>(t, 2000); - - q.add(new Object()); - q.add(new Object()); - q.remove(); - t.milliTime += 10; - q.remove(); - - assertEquals(200, q.capacity()); - - for (int i = 0; i < 1000; i++) { - q.add(new Object()); - q.add(new Object()); - t.milliTime += 100; - q.remove(); - t.milliTime += 100; - q.remove(); - } - - assertEquals(20, q.capacity()); - } - -} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/StandaloneMain.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/StandaloneMain.java index 770c59dec35..c24e583ea46 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/StandaloneMain.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/StandaloneMain.java @@ -39,9 +39,9 @@ public class StandaloneMain { // We're not logging at this point since the application is responsible // for setting up logging. System.out.println("debug\tInitializing application without privileges."); + setupSignalHandlers(); loader.init(bundleLocation, false); loader.start(); - setupSignalHandlers(); waitForShutdown(); System.out.println("debug\tTrying to shutdown in a controlled manner."); log.log(Level.INFO, "JDisc shutting down"); diff --git a/parent/pom.xml b/parent/pom.xml index 504ae2ade4d..36443db2c28 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -651,12 +651,13 @@ <version>1.7</version> </dependency> <dependency> - <!-- Explicitly included to get Zookeeper version 3.4.14, + <!-- Explicitly force Zookeeper version, can be excluded if you want the Zookeeper version - used by curator by default --> + used by curator by default + --> <groupId>org.apache.zookeeper</groupId> <artifactId>zookeeper</artifactId> - <version>3.4.14</version> + <version>${zookeeper.version}</version> </dependency> <dependency> <groupId>org.assertj</groupId> @@ -787,6 +788,7 @@ <protobuf.version>3.7.0</protobuf.version> <surefire.version>2.22.0</surefire.version> <tensorflow.version>1.12.0</tensorflow.version> + <zookeeper.version>3.4.14</zookeeper.version> <doclint>all</doclint> <test.hide>true</test.hide> diff --git a/searchcore/CMakeLists.txt b/searchcore/CMakeLists.txt index ee262a5fa5c..53435780d9a 100644 --- a/searchcore/CMakeLists.txt +++ b/searchcore/CMakeLists.txt @@ -40,7 +40,6 @@ vespa_define_module( src/vespa/searchcore/proton/server src/vespa/searchcore/proton/summaryengine src/vespa/searchcore/proton/test - src/vespa/searchcore/util APPS src/apps/proton diff --git a/searchcore/src/apps/proton/CMakeLists.txt b/searchcore/src/apps/proton/CMakeLists.txt index a8bfa9a39a8..c42ed048ce1 100644 --- a/searchcore/src/apps/proton/CMakeLists.txt +++ b/searchcore/src/apps/proton/CMakeLists.txt @@ -24,7 +24,6 @@ vespa_add_executable(searchcore_proton_app searchcore_grouping searchcore_proton_metrics searchcore_fconfig - searchcore_util storageserver_storageapp searchlib_searchlib_uca ) diff --git a/searchcore/src/apps/tests/CMakeLists.txt b/searchcore/src/apps/tests/CMakeLists.txt index 55c371791a3..42be4091f66 100644 --- a/searchcore/src/apps/tests/CMakeLists.txt +++ b/searchcore/src/apps/tests/CMakeLists.txt @@ -19,7 +19,6 @@ vespa_add_executable(searchcore_persistenceconformance_test_app TEST searchcore_grouping searchcore_proton_metrics searchcore_fconfig - searchcore_util vdstestlib persistence_persistence_conformancetest searchlib_searchlib_uca diff --git a/searchcore/src/tests/proton/common/attribute_updater/CMakeLists.txt b/searchcore/src/tests/proton/common/attribute_updater/CMakeLists.txt index d25a88c1a71..8b552cb2b47 100644 --- a/searchcore/src/tests/proton/common/attribute_updater/CMakeLists.txt +++ b/searchcore/src/tests/proton/common/attribute_updater/CMakeLists.txt @@ -4,6 +4,5 @@ vespa_add_executable(searchcore_attribute_updater_test_app TEST attribute_updater_test.cpp DEPENDS searchcore_pcommon - searchcore_util ) vespa_add_test(NAME searchcore_attribute_updater_test_app COMMAND searchcore_attribute_updater_test_app) diff --git a/searchcore/src/tests/proton/docsummary/CMakeLists.txt b/searchcore/src/tests/proton/docsummary/CMakeLists.txt index 906a1e642f5..ce1a0b3d68c 100644 --- a/searchcore/src/tests/proton/docsummary/CMakeLists.txt +++ b/searchcore/src/tests/proton/docsummary/CMakeLists.txt @@ -22,7 +22,6 @@ vespa_add_executable(searchcore_docsummary_test_app TEST searchcore_grouping searchcore_proton_metrics searchcore_fconfig - searchcore_util searchlib_searchlib_uca ) vespa_add_executable(searchcore_summaryfieldconverter_test_app diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp index 0a9f3127844..0e521e473ae 100644 --- a/searchcore/src/tests/proton/docsummary/docsummary.cpp +++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp @@ -23,7 +23,6 @@ #include <vespa/searchcore/proton/server/searchview.h> #include <vespa/searchcore/proton/server/summaryadapter.h> #include <vespa/searchlib/common/gatecallback.h> -#include <vespa/searchlib/common/transport.h> #include <vespa/searchlib/engine/docsumapi.h> #include <vespa/searchlib/index/docbuilder.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> @@ -359,7 +358,7 @@ Test::assertTensor(const Tensor::UP & exp, const std::string & fieldName, uint32_t classId; ASSERT_LESS_EQUAL(sizeof(classId), docsum.data.size()); memcpy(&classId, docsum.data.c_str(), sizeof(classId)); - ASSERT_EQUAL(::search::fs4transport::SLIME_MAGIC_ID, classId); + ASSERT_EQUAL(::search::docsummary::SLIME_MAGIC_ID, classId); vespalib::Slime slime; vespalib::Memory serialized(docsum.data.c_str() + sizeof(classId), docsum.data.size() - sizeof(classId)); @@ -384,7 +383,7 @@ getSlime(const DocsumReply &reply, uint32_t id, bool relaxed) uint32_t classId; ASSERT_LESS_EQUAL(sizeof(classId), docsum.data.size()); memcpy(&classId, docsum.data.c_str(), sizeof(classId)); - ASSERT_EQUAL(::search::fs4transport::SLIME_MAGIC_ID, classId); + ASSERT_EQUAL(search::docsummary::SLIME_MAGIC_ID, classId); vespalib::Slime slime; vespalib::Memory serialized(docsum.data.c_str() + sizeof(classId), docsum.data.size() - sizeof(classId)); diff --git a/searchcore/src/tests/proton/documentdb/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/CMakeLists.txt index cd67bd82a06..d4a08e9de5c 100644 --- a/searchcore/src/tests/proton/documentdb/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/CMakeLists.txt @@ -19,7 +19,6 @@ vespa_add_executable(searchcore_documentdb_test_app TEST searchcore_grouping searchcore_proton_metrics searchcore_fconfig - searchcore_util searchlib_searchlib_uca ) vespa_add_test(NAME searchcore_documentdb_test_app COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/documentdb_test.sh diff --git a/searchcore/src/tests/proton/documentdb/buckethandler/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/buckethandler/CMakeLists.txt index d48778ffd1b..e955aa76d37 100644 --- a/searchcore/src/tests/proton/documentdb/buckethandler/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/buckethandler/CMakeLists.txt @@ -12,7 +12,6 @@ vespa_add_executable(searchcore_buckethandler_test_app TEST searchcore_bucketdb searchcore_pcommon searchcore_grouping - searchcore_util searchcore_fconfig ) vespa_add_test(NAME searchcore_buckethandler_test_app COMMAND searchcore_buckethandler_test_app) diff --git a/searchcore/src/tests/proton/documentdb/clusterstatehandler/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/clusterstatehandler/CMakeLists.txt index 28b79100eda..77bdae70ebd 100644 --- a/searchcore/src/tests/proton/documentdb/clusterstatehandler/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/clusterstatehandler/CMakeLists.txt @@ -10,7 +10,6 @@ vespa_add_executable(searchcore_clusterstatehandler_test_app TEST searchcore_attribute searchcore_pcommon searchcore_grouping - searchcore_util searchcore_fconfig ) vespa_add_test(NAME searchcore_clusterstatehandler_test_app COMMAND searchcore_clusterstatehandler_test_app) diff --git a/searchcore/src/tests/proton/documentdb/combiningfeedview/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/combiningfeedview/CMakeLists.txt index e046b81b8bd..e810b2191bb 100644 --- a/searchcore/src/tests/proton/documentdb/combiningfeedview/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/combiningfeedview/CMakeLists.txt @@ -13,7 +13,6 @@ vespa_add_executable(searchcore_combiningfeedview_test_app TEST searchcore_pcommon searchcore_grouping searchcore_proton_metrics - searchcore_util searchcore_fconfig ) vespa_add_test(NAME searchcore_combiningfeedview_test_app COMMAND searchcore_combiningfeedview_test_app) diff --git a/searchcore/src/tests/proton/documentdb/configurer/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/configurer/CMakeLists.txt index fe09e017ba5..a7cdfff8085 100644 --- a/searchcore/src/tests/proton/documentdb/configurer/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/configurer/CMakeLists.txt @@ -18,7 +18,6 @@ vespa_add_executable(searchcore_configurer_test_app TEST searchcore_grouping searchcore_proton_metrics searchcore_fconfig - searchcore_util searchlib_searchlib_uca ) vespa_add_test(NAME searchcore_configurer_test_app COMMAND searchcore_configurer_test_app) diff --git a/searchcore/src/tests/proton/documentdb/document_subdbs/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/document_subdbs/CMakeLists.txt index f4a9d90add9..697666f8e02 100644 --- a/searchcore/src/tests/proton/documentdb/document_subdbs/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/document_subdbs/CMakeLists.txt @@ -18,7 +18,6 @@ vespa_add_executable(searchcore_document_subdbs_test_app TEST searchcore_pcommon searchcore_grouping searchcore_proton_metrics - searchcore_util searchcore_fconfig searchlib_searchlib_uca ) diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt index b23ab73cd6e..1c11ed745a6 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt @@ -13,7 +13,6 @@ vespa_add_executable(searchcore_documentbucketmover_test_app TEST searchcore_bucketdb searchcore_pcommon searchcore_grouping - searchcore_util searchcore_fconfig ) vespa_add_test(NAME searchcore_documentbucketmover_test_app COMMAND searchcore_documentbucketmover_test_app) diff --git a/searchcore/src/tests/proton/documentdb/feedhandler/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/feedhandler/CMakeLists.txt index 8f93629b6a6..84027130cdb 100644 --- a/searchcore/src/tests/proton/documentdb/feedhandler/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/feedhandler/CMakeLists.txt @@ -13,7 +13,6 @@ vespa_add_executable(searchcore_feedhandler_test_app TEST searchcore_pcommon searchcore_grouping searchcore_proton_metrics - searchcore_util searchcore_fconfig ) vespa_add_test(NAME searchcore_feedhandler_test_app COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/feedhandler_test.sh diff --git a/searchcore/src/tests/proton/documentdb/feedview/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/feedview/CMakeLists.txt index ebf28f3bf16..a46ca246d9a 100644 --- a/searchcore/src/tests/proton/documentdb/feedview/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/feedview/CMakeLists.txt @@ -14,7 +14,6 @@ vespa_add_executable(searchcore_feedview_test_app TEST searchcore_pcommon searchcore_grouping searchcore_proton_metrics - searchcore_util searchcore_fconfig ) vespa_add_test(NAME searchcore_feedview_test_app COMMAND searchcore_feedview_test_app) diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/maintenancecontroller/CMakeLists.txt index 6b25971df83..0864f444ecf 100644 --- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/CMakeLists.txt @@ -14,7 +14,6 @@ vespa_add_executable(searchcore_maintenancecontroller_test_app TEST searchcore_persistenceengine searchcore_grouping searchcore_proton_metrics - searchcore_util searchcore_fconfig searchlib_test ) @@ -33,7 +32,6 @@ vespa_add_executable(searchcore_frozenbucketsmap_test_app TEST searchcore_persistenceengine searchcore_grouping searchcore_proton_metrics - searchcore_util searchcore_fconfig ) vespa_add_test(NAME searchcore_frozenbucketsmap_test_app COMMAND searchcore_frozenbucketsmap_test_app) diff --git a/searchcore/src/tests/proton/feed_and_search/CMakeLists.txt b/searchcore/src/tests/proton/feed_and_search/CMakeLists.txt index fdc2a925ae1..2c37e4f4a71 100644 --- a/searchcore/src/tests/proton/feed_and_search/CMakeLists.txt +++ b/searchcore/src/tests/proton/feed_and_search/CMakeLists.txt @@ -3,6 +3,5 @@ vespa_add_executable(searchcore_feed_and_search_test_app TEST SOURCES feed_and_search.cpp DEPENDS - searchcore_util ) vespa_add_test(NAME searchcore_feed_and_search_test_app COMMAND searchcore_feed_and_search_test_app) diff --git a/searchcore/src/tests/proton/index/CMakeLists.txt b/searchcore/src/tests/proton/index/CMakeLists.txt index ef40e291e18..1ffad6cdbf8 100644 --- a/searchcore/src/tests/proton/index/CMakeLists.txt +++ b/searchcore/src/tests/proton/index/CMakeLists.txt @@ -7,7 +7,6 @@ vespa_add_executable(searchcore_indexmanager_test_app TEST searchcore_index searchcore_flushengine searchcore_pcommon - searchcore_util gtest ) vespa_add_executable(searchcore_fusionrunner_test_app TEST @@ -17,7 +16,6 @@ vespa_add_executable(searchcore_fusionrunner_test_app TEST searchcore_server searchcore_index searchcore_pcommon - searchcore_util ) vespa_add_executable(searchcore_diskindexcleaner_test_app TEST SOURCES diff --git a/searchcore/src/tests/proton/matching/CMakeLists.txt b/searchcore/src/tests/proton/matching/CMakeLists.txt index 0fce2f6aca2..e3c5fa2bd97 100644 --- a/searchcore/src/tests/proton/matching/CMakeLists.txt +++ b/searchcore/src/tests/proton/matching/CMakeLists.txt @@ -12,7 +12,6 @@ vespa_add_executable(searchcore_matching_test_app TEST searchcore_bucketdb searchcore_pcommon searchcore_grouping - searchcore_util searchlib_searchlib_uca searchlib_test ) diff --git a/searchcore/src/tests/proton/matching/matching_test.cpp b/searchcore/src/tests/proton/matching/matching_test.cpp index 3f68b54aca2..950321533b0 100644 --- a/searchcore/src/tests/proton/matching/matching_test.cpp +++ b/searchcore/src/tests/proton/matching/matching_test.cpp @@ -18,7 +18,6 @@ #include <vespa/searchlib/aggregation/perdocexpression.h> #include <vespa/searchlib/attribute/extendableattributes.h> #include <vespa/searchlib/common/featureset.h> -#include <vespa/searchlib/common/transport.h> #include <vespa/searchlib/engine/docsumrequest.h> #include <vespa/searchlib/engine/searchrequest.h> #include <vespa/searchlib/engine/docsumreply.h> @@ -573,27 +572,22 @@ TEST("require that re-ranking is diverse with diversity = 1/10") { } TEST("require that sortspec can be used (multi-threaded)") { - for (bool drop_sort_data: {false, true}) { - for (size_t threads = 1; threads <= 16; ++threads) { - MyWorld world; - world.basicSetup(); - world.basicResults(); - SearchRequest::SP request = world.createSimpleRequest("f1", "spread"); - request->sortSpec = "+a1"; - if (drop_sort_data) { - request->queryFlags |= fs4transport::QFLAG_DROP_SORTDATA; - } - SearchReply::UP reply = world.performSearch(request, threads); - ASSERT_EQUAL(9u, reply->hits.size()); - EXPECT_EQUAL(document::DocumentId("id:ns:searchdocument::100").getGlobalId(), reply->hits[0].gid); - EXPECT_EQUAL(zero_rank_value, reply->hits[0].metric); - EXPECT_EQUAL(document::DocumentId("id:ns:searchdocument::200").getGlobalId(), reply->hits[1].gid); - EXPECT_EQUAL(zero_rank_value, reply->hits[1].metric); - EXPECT_EQUAL(document::DocumentId("id:ns:searchdocument::300").getGlobalId(), reply->hits[2].gid); - EXPECT_EQUAL(zero_rank_value, reply->hits[2].metric); - EXPECT_EQUAL(drop_sort_data, reply->sortIndex.empty()); - EXPECT_EQUAL(drop_sort_data, reply->sortData.empty()); - } + for (size_t threads = 1; threads <= 16; ++threads) { + MyWorld world; + world.basicSetup(); + world.basicResults(); + SearchRequest::SP request = world.createSimpleRequest("f1", "spread"); + request->sortSpec = "+a1"; + SearchReply::UP reply = world.performSearch(request, threads); + ASSERT_EQUAL(9u, reply->hits.size()); + EXPECT_EQUAL(document::DocumentId("id:ns:searchdocument::100").getGlobalId(), reply->hits[0].gid); + EXPECT_EQUAL(zero_rank_value, reply->hits[0].metric); + EXPECT_EQUAL(document::DocumentId("id:ns:searchdocument::200").getGlobalId(), reply->hits[1].gid); + EXPECT_EQUAL(zero_rank_value, reply->hits[1].metric); + EXPECT_EQUAL(document::DocumentId("id:ns:searchdocument::300").getGlobalId(), reply->hits[2].gid); + EXPECT_EQUAL(zero_rank_value, reply->hits[2].metric); + EXPECT_FALSE(reply->sortIndex.empty()); + EXPECT_FALSE(reply->sortData.empty()); } } diff --git a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp index 23ef86a46b7..a9cae7d8ab7 100644 --- a/searchcore/src/tests/proton/summaryengine/summaryengine.cpp +++ b/searchcore/src/tests/proton/summaryengine/summaryengine.cpp @@ -7,7 +7,7 @@ #include <vespa/searchlib/util/slime_output_raw_buf_adapter.h> #include <vespa/vespalib/data/databuffer.h> #include <vespa/vespalib/util/compressor.h> -#include <vespa/searchlib/common/transport.h> +#include <vespa/searchsummary/docsummary/docsumwriter.h> #include <vespa/metrics/metricset.h> #include <vespa/fnet/frt/rpcrequest.h> @@ -336,7 +336,7 @@ void createSummary(search::RawBuf &buf) { vespalib::Slime summary; summary.setObject().setLong("long", 982); - uint32_t magic = search::fs4transport::SLIME_MAGIC_ID; + uint32_t magic = search::docsummary::SLIME_MAGIC_ID; buf.append(&magic, sizeof(magic)); search::SlimeOutputRawBufAdapter adapter(buf); BinaryFormat::encode(summary, adapter); diff --git a/searchcore/src/vespa/searchcore/common/.gitignore b/searchcore/src/vespa/searchcore/common/.gitignore deleted file mode 100644 index e69de29bb2d..00000000000 --- a/searchcore/src/vespa/searchcore/common/.gitignore +++ /dev/null diff --git a/searchcore/src/vespa/searchcore/common/OWNERS b/searchcore/src/vespa/searchcore/common/OWNERS deleted file mode 100644 index 9dc0c2d970d..00000000000 --- a/searchcore/src/vespa/searchcore/common/OWNERS +++ /dev/null @@ -1 +0,0 @@ -baldersheim diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp index c65257e7f6a..5951d07a2cb 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp +++ b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp @@ -5,7 +5,6 @@ #include <vespa/searchlib/attribute/iattributemanager.h> #include <vespa/searchlib/common/location.h> #include <vespa/searchlib/common/matching_elements.h> -#include <vespa/searchlib/common/transport.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/util/stringfmt.h> @@ -50,7 +49,6 @@ DocsumContext::initState() { const DocsumRequest & req = _request; _docsumState._args.initFromDocsumRequest(req); - _docsumState._args.SetQueryFlags(req.queryFlags & ~search::fs4transport::QFLAG_DROP_SORTDATA); _docsumState._docsumcnt = req.hits.size(); _docsumState._docsumbuf = (_docsumState._docsumcnt > 0) @@ -176,7 +174,7 @@ DocsumContext::FillRankFeatures(search::docsummary::GetDocsumsState * state, sea { assert(&_docsumState == state); // check if we are allowed to run - if ((state->_args.GetQueryFlags() & search::fs4transport::QFLAG_DUMP_FEATURES) == 0) { + if ( ! state->_args.dumpFeatures()) { return; } state->_rankFeatures = _matcher->getRankFeatures(_request, _searchCtx, _attrCtx, _sessionMgr); diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index 596635a416a..87c0283d228 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -224,7 +224,7 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl !_rankSetup->getSecondPhaseRank().empty(), !willNotNeedRanking(request, groupingContext)); ResultProcessor rp(attrContext, metaStore, sessionMgr, groupingContext, sessionId, - request.sortSpec, params.offset, params.hits, request.should_drop_sort_data()); + request.sortSpec, params.offset, params.hits); size_t numThreadsPerSearch = computeNumThreadsPerSearch(mtf->estimate(), rankProperties); LimitedThreadBundleWrapper limitedThreadBundle(threadBundle, numThreadsPerSearch); diff --git a/searchcore/src/vespa/searchcore/proton/matching/result_processor.cpp b/searchcore/src/vespa/searchcore/proton/matching/result_processor.cpp index dedda1504a5..445aab310d9 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/result_processor.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/result_processor.cpp @@ -61,8 +61,7 @@ ResultProcessor::ResultProcessor(IAttributeContext &attrContext, GroupingContext &groupingContext, const vespalib::string &sessionId, const vespalib::string &sortSpec, - size_t offset, size_t hits, - bool drop_sort_data) + size_t offset, size_t hits) : _attrContext(attrContext), _metaStore(metaStore), _sessionMgr(sessionMgr), @@ -71,7 +70,6 @@ ResultProcessor::ResultProcessor(IAttributeContext &attrContext, _sortSpec(sortSpec), _offset(offset), _hits(hits), - _drop_sort_data(drop_sort_data), _wasMerged(false) { if (!_groupingContext.empty()) { @@ -140,7 +138,7 @@ ResultProcessor::makeReply(PartialResultUP full_result) dst.metric = src._rankValue; LOG(debug, "convertLidToGid: hit[%zu]: lid(%u) -> gid(%s)", i, docId, dst.gid.toString().c_str()); } - if (result.hasSortData() && (hitcnt > 0) && !_drop_sort_data) { + if (result.hasSortData() && (hitcnt > 0)) { size_t sortDataSize = result.sortDataSize(); for (size_t i = 0; i < hitOffset; ++i) { sortDataSize -= result.sortData(i).second; diff --git a/searchcore/src/vespa/searchcore/proton/matching/result_processor.h b/searchcore/src/vespa/searchcore/proton/matching/result_processor.h index 41c764d247b..48a8aecebe1 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/result_processor.h +++ b/searchcore/src/vespa/searchcore/proton/matching/result_processor.h @@ -88,7 +88,6 @@ private: const vespalib::string &_sortSpec; size_t _offset; size_t _hits; - bool _drop_sort_data; bool _wasMerged; public: @@ -98,8 +97,7 @@ public: GroupingContext & groupingContext, const vespalib::string & sessionId, const vespalib::string & sortSpec, - size_t offset, size_t hits, - bool drop_sort_data); + size_t offset, size_t hits); ~ResultProcessor(); size_t countFS4Hits(); diff --git a/searchcore/src/vespa/searchcore/util/.gitignore b/searchcore/src/vespa/searchcore/util/.gitignore deleted file mode 100644 index 51f1b16bde5..00000000000 --- a/searchcore/src/vespa/searchcore/util/.gitignore +++ /dev/null @@ -1,14 +0,0 @@ -*.exp -*.ilk -*.lib -*.pdb -.depend -ID -Makefile -extcase -extcase.exe -extprop -extprop.exe -mklicense -result -util.lib diff --git a/searchcore/src/vespa/searchcore/util/CMakeLists.txt b/searchcore/src/vespa/searchcore/util/CMakeLists.txt deleted file mode 100644 index 0d02385be94..00000000000 --- a/searchcore/src/vespa/searchcore/util/CMakeLists.txt +++ /dev/null @@ -1,7 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_library(searchcore_util STATIC - SOURCES - eventloop.cpp - log.cpp - DEPENDS -) diff --git a/searchcore/src/vespa/searchcore/util/OWNERS b/searchcore/src/vespa/searchcore/util/OWNERS deleted file mode 100644 index 9dc0c2d970d..00000000000 --- a/searchcore/src/vespa/searchcore/util/OWNERS +++ /dev/null @@ -1 +0,0 @@ -baldersheim diff --git a/searchcore/src/vespa/searchcore/util/autoptr.h b/searchcore/src/vespa/searchcore/util/autoptr.h deleted file mode 100644 index ba294e097e4..00000000000 --- a/searchcore/src/vespa/searchcore/util/autoptr.h +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -template <class T> -class FastS_AutoPtr -{ -private: - FastS_AutoPtr(const FastS_AutoPtr &); - FastS_AutoPtr& operator=(const FastS_AutoPtr &); - - T *_val; - void Clean() { - if (_val != NULL) { - delete _val; - _val = NULL; - } - } -public: - FastS_AutoPtr() : _val(NULL) { } - explicit FastS_AutoPtr(T *val) - : _val(val) - { - } - ~FastS_AutoPtr() { Clean(); } - void Set(T *val) { Clean(); _val = val; } - T *Get() const { return _val; } - T *HandOver() { T *ret = _val; _val = NULL; return ret; } - void Drop() { - if (_val != NULL) { - delete _val; - _val = NULL; - } - } -}; - - -template <class T> -class FastS_AutoRefCntPtr -{ -private: - FastS_AutoRefCntPtr(const FastS_AutoRefCntPtr &); - FastS_AutoRefCntPtr& operator=(const FastS_AutoRefCntPtr &); - - T *_val; - void Clean() { - if (_val != NULL) - _val->subRef(); - } -public: - FastS_AutoRefCntPtr() : _val(NULL) { } - explicit FastS_AutoRefCntPtr(T *val) {_val = val; } - ~FastS_AutoRefCntPtr() { Clean(); } - void Set(T *val) { Clean(); _val = val; } - void SetDup(T *val) { - Clean(); - if (val != NULL) - val->addRef(); - _val = val; - } - T *Get() const { return _val; } - T *GetDup() { - if (_val != NULL) - _val->addRef(); - return _val; - } - T *HandOver() { T *ret = _val; _val = NULL; return ret; } - void Drop() { - if (_val != NULL) { - _val->subRef(); - _val = NULL; - } - } -}; - - -class FastS_AutoCharPtr -{ -private: - FastS_AutoCharPtr(const FastS_AutoCharPtr &); - FastS_AutoCharPtr& operator=(const FastS_AutoCharPtr &); - - char *_val; - void Clean() { - if (_val != NULL) - free(_val); - } -public: - FastS_AutoCharPtr() - : _val(NULL) - { - } - explicit FastS_AutoCharPtr(char *val) - : _val(val) - { - } - ~FastS_AutoCharPtr() { Clean(); } - void Set(char *val) { Clean(); _val = val; } - char *Get() const { return _val; } - char *HandOver() { char *ret = _val; _val = NULL; return ret; } - void Drop() { - if (_val != NULL) { - free(_val); - _val = NULL; - } - } -}; - diff --git a/searchcore/src/vespa/searchcore/util/description.html b/searchcore/src/vespa/searchcore/util/description.html deleted file mode 100644 index 025ae212095..00000000000 --- a/searchcore/src/vespa/searchcore/util/description.html +++ /dev/null @@ -1 +0,0 @@ -<!-- Short description for make kdoc. --> diff --git a/searchcore/src/vespa/searchcore/util/eventloop.cpp b/searchcore/src/vespa/searchcore/util/eventloop.cpp deleted file mode 100644 index 3cc9bf3bfc6..00000000000 --- a/searchcore/src/vespa/searchcore/util/eventloop.cpp +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "eventloop.h" -#include <cstdio> - -double FastS_TimeOut::_val[FastS_TimeOut::valCnt]; - -void -FastS_TimeOut::WriteTime(char* buffer, size_t bufsize, double xtime) -{ - snprintf(buffer, bufsize, "%.3fs ", xtime); -} diff --git a/searchcore/src/vespa/searchcore/util/eventloop.h b/searchcore/src/vespa/searchcore/util/eventloop.h deleted file mode 100644 index 0a17ec1287a..00000000000 --- a/searchcore/src/vespa/searchcore/util/eventloop.h +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <cstddef> - -class FastS_TimeOut -{ -public: - enum ValName { - maxSockSilent, // 0 - valCnt // 1 - Must be last, used as array size: - }; - static double _val[valCnt]; - - static void WriteTime(char* buffer, size_t bufsize, double xtime); -}; - - diff --git a/searchcore/src/vespa/searchcore/util/log.cpp b/searchcore/src/vespa/searchcore/util/log.cpp deleted file mode 100644 index fc25fa1f346..00000000000 --- a/searchcore/src/vespa/searchcore/util/log.cpp +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include <vespa/searchcore/util/log.h> - -#include <vespa/log/log.h> -LOG_SETUP(".searchcore.util.log"); - -/** - * assert and abort functions. - */ -void __FastS_assert_fail(const char *assertion, - const char *file, - unsigned int line, - const char * function) -{ - const char *vtag = V_TAG; - if (function != NULL) { - LOG(error, "FATAL: %s:%d (%s) %s: Failed assertion: '%s'", - file, line, vtag, function, assertion); - fprintf(stderr, "%s:%d (%s) %s: Failed assertion: '%s'\n", - file, line, vtag, function, assertion); - } else { - LOG(error, "FATAL: %s:%d (%s): Failed assertion: '%s'", - file, line, vtag, assertion); - fprintf(stderr, "%s:%d (%s): Failed assertion: '%s'\n", - file, line, vtag, assertion); - } - EV_STOPPING("", "assert failed"); - abort(); -} - -void __FastS_abort(const char *message, - const char *file, - unsigned int line, - const char * function) -{ - const char *vtag = V_TAG; - if (function != NULL) { - LOG(error, "FATAL: %s:%d (%s) %s: Abort called. Reason: %s", - file, line, vtag, function, message); - fprintf(stderr, "%s:%d (%s) %s: Abort called. Reason: %s\n", - file, line, vtag, function, message); - } else { - LOG(error, "FATAL: %s:%d (%s): Abort called. Reason: %s", - file, line, vtag, message); - fprintf(stderr, "%s:%d (%s): Abort called. Reason: %s\n", - file, line, vtag, message); - } - EV_STOPPING("", "aborted"); - abort(); -} diff --git a/searchcore/src/vespa/searchcore/util/log.h b/searchcore/src/vespa/searchcore/util/log.h deleted file mode 100644 index 72aa525a421..00000000000 --- a/searchcore/src/vespa/searchcore/util/log.h +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - - -/* - * Define FastS_abort and FastS_assert macro's. - */ - -/** - * This logs an "assertion failed" message and aborts. - */ -extern void __FastS_assert_fail(const char *assertion, - const char *file, - unsigned int line, - const char * function); - -/** - * This logs an "abort" message and aborts. - */ -extern void __FastS_abort(const char *message, - const char *file, - unsigned int line, - const char * function); - -# ifndef __STRING -# define __STRING(x) #x -# endif - -# ifndef V_TAG -# define V_TAG "NOTAG" -# endif - -# ifndef __ASSERT_FUNCTION -# define __ASSERT_FUNCTION NULL -# endif - - -# define FastS_abort(msg) \ - (__FastS_abort(msg, __FILE__, __LINE__, __ASSERT_FUNCTION), abort()) - -# ifndef NDEBUG -# define FastS_assert(expr) \ - ((void) ((expr) ? 0 : \ - (__FastS_assert_fail (__STRING(expr), \ - __FILE__, __LINE__, \ - __ASSERT_FUNCTION), 0))) -# else -# define FastS_assert(expr) -# endif // #ifndef NDEBUG - diff --git a/searchcore/src/vespa/searchcore/util/stlishheap.h b/searchcore/src/vespa/searchcore/util/stlishheap.h deleted file mode 100644 index 3a04f8f2d39..00000000000 --- a/searchcore/src/vespa/searchcore/util/stlishheap.h +++ /dev/null @@ -1,396 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -/* These algorithms only work for standard C-type arrays, not generic - iterators the way STL works. To make them work for stl-type - iterators, we need to create a set of traits classes, but that is - kind of overkill until we need that functionality. */ - -/* You can use these functions to customize the heap */ - -template<typename T> -inline bool -FastS_min(T a, T b) -{ - return a < b; -} - -template<typename T> -inline bool -FastS_max(T a, T b) -{ - return b < a; -} - -/* Push obj onto the heap first of length len. len must be large - * enough to include the new object. For example if you have a heap - * with 3 elements and want to push a new element onto the heap, len - * should be 4. - */ - -template <typename T, typename Comp> -inline void -FastS_push_heap(T *first, int len, T obj, Comp comp) -{ - int x = len - 1; - int parent = (x - 1)/2; - while (x > 0 && comp(*(first + parent), obj)) { - *(first + x) = *(first + parent); - x = parent; - parent = (x - 1)/2; - } - *(first + x) = obj; -} - -/* Pop the largest element off the heap, reducing the size of the heap - * by 1. (Note: it is the responsibility of the caller to keep track - * of the size of the heap.) - */ - -template<typename T, typename Comp> -inline T -FastS_pop_heap(T *first, int len, Comp comp) -{ - /* The algorithm we use is a variation of the textbook algorithm. - We first remove the first element, then instead of putting the - last element at the top of the heap and heapify(), we propagate - the "hole" left by the removed first element to the bottom. Then - we copy the last element into the hole and push this element - upwards. Since the last element has a high probability of being - pushed down to the bottom anyways, this reduces the number of - comparisons we need to do. */ - T ret = *first; - /* right child */ - int topidx = 0; - int childidx = 2; - /* while both right and left child exist.. */ - while (childidx < len) { - /* compare right to left child */ - if (comp(*(first + childidx), *(first + childidx - 1))) - childidx--; - *(first + topidx) = *(first + childidx); - topidx = childidx; - childidx = 2 * (topidx + 1); - } - /* if only left child exists.. */ - if (childidx == len) { - *(first + topidx) = *(first + childidx - 1); - topidx = childidx - 1; - } - /* now topidx is the hole.. */ - FastS_push_heap(first, topidx + 1, *(first + len - 1), comp); - return ret; -} - -/* Pop the largest element off the heap, and push a new element on the - * heap in the same operation. - */ - -template<typename T, typename Comp> -inline T -FastS_pop_push_heap(T *first, int len, T obj, Comp comp) -{ - T ret = *first; - /* right child */ - int topidx = 0; - int childidx = 2; - /* while both right and left child exist.. */ - while (childidx < len) { - /* compare right to left child */ - if (comp(*(first + childidx), *(first + childidx - 1))) - childidx--; - *(first + topidx) = *(first + childidx); - topidx = childidx; - childidx = 2 * (topidx + 1); - } - /* if only left child exists.. */ - if (childidx == len) { - *(first + topidx) = *(first + childidx - 1); - topidx = childidx - 1; - } - /* now topidx is the hole.. */ - FastS_push_heap(first, topidx + 1, obj, comp); - return ret; -} - -/* Similar to FastS_pop_heap, this function, given a "hole" in the - * heap, heapify()es the heap downwards. It then inserts obj and - * adjusts the heap upwards. - */ - -template<typename T, typename Comp> -inline void -FastS__adjust_heap(T *first, int len, int hole, T obj, Comp comp) -{ - /* right child */ - int topidx = hole; - int childidx = 2 * (hole + 1); - /* while both right and left child exist.. */ - while (childidx < len) { - /* compare right to left child */ - if (comp(*(first + childidx), *(first + childidx - 1))) - childidx--; - *(first + topidx) = *(first + childidx); - topidx = childidx; - childidx = 2 * (topidx + 1); - } - /* if only left child exists.. */ - if (childidx == len) { - *(first + topidx) = *(first + childidx - 1); - topidx = childidx - 1; - } - /* now first[topidx] is the hole.. */ - FastS_push_heap(first, topidx + 1, obj, comp); -} - -template <typename T, typename Comp> -inline void -FastS_make_heap(T *first, int len, Comp comp) -{ - if (len < 2) - return; - int parent = (len - 2)/2; - for (/**/; parent >= 0; parent--) { - int holeidx = parent; - T obj = *(first + parent); - int childidx = 2 * (parent + 1); - while (childidx < len) { - if (comp(*(first + childidx), *(first + childidx - 1))) - childidx--; - if (comp(*(first + childidx), obj)) { - *(first + holeidx) = obj; - goto nextparent; - } else { - *(first + holeidx) = *(first + childidx); - holeidx = childidx; - childidx = 2* (holeidx + 1); - } - } - if (childidx == len) { - if (comp(*(first + childidx - 1), obj)) { - *(first + holeidx) = obj; - } else { - *(first + holeidx) = *(first + childidx - 1); - *(first + childidx - 1) = obj; - } - } else /* childidx > len */ - *(first + holeidx) = obj; - nextparent: - ; - } -} - -template <typename T, typename Comp> -inline void -FastS_sort_heap(T *first, int len, Comp comp) -{ - while (len > 0) { - *(first + len - 1) = FastS_pop_heap(first, len, comp); - len--; - } -} - -template <typename T, typename Comp> -inline bool -FastS_is_heap(T *first, int len, Comp comp) -{ - for (int i = 0; i < len; i++) { - int left = 2 * i + 1; - int right = 2 * i + 2; - if (left < len && comp(*(first + i), *(first + left))) - return false; - if (right < len && comp(*(first + i), *(first + right))) - return false; - } - return true; -} - -//////////////////////////////////////////////////////// -// Similar to the above, but without comparator support -//////////////////////////////////////////////////////// - -template <typename T> -inline void -FastS_push_heap(T *first, int len, T obj) -{ - int x = len - 1; - int parent = (x - 1)/2; - while (x > 0 && *(first + parent) < obj) { - *(first + x) = *(first + parent); - x = parent; - parent = (x - 1)/2; - } - *(first + x) = obj; -} - -/* Pop the largest element off the heap, reducing the size of the heap - * by 1. (Note: it is the responsibility of the caller to keep track - * of the size of the heap.) - */ - -template<typename T> -inline T -FastS_pop_heap(T *first, int len) -{ - /* The algorithm we use is a variation of the textbook algorithm. - We first remove the first element, then instead of putting the - last element at the top of the heap and heapify(), we propagate - the "hole" left by the removed first element to the bottom. Then - we copy the last element into the hole and push this element - upwards. Since the last element has a high probability of being - pushed down to the bottom anyways, this reduces the number of - comparisons we need to do. */ - T ret = *first; - /* right child */ - int topidx = 0; - int childidx = 2; - /* while both right and left child exist.. */ - while (childidx < len) { - /* compare right to left child */ - if (*(first + childidx) < *(first + childidx - 1)) - childidx--; - *(first + topidx) = *(first + childidx); - topidx = childidx; - childidx = 2 * (topidx + 1); - } - /* if only left child exists.. */ - if (childidx == len) { - *(first + topidx) = *(first + childidx - 1); - topidx = childidx - 1; - } - /* now topidx is the hole.. */ - FastS_push_heap(first, topidx + 1, *(first + len - 1)); - return ret; -} - - -/* Pop the largest element off the heap, and push a new element on the - * heap in the same operation. - */ - -template<typename T> -inline T -FastS_pop_push_heap(T *first, int len, T obj) -{ - T ret = *first; - /* right child */ - int topidx = 0; - int childidx = 2; - /* while both right and left child exist.. */ - while (childidx < len) { - /* compare right to left child */ - if (*(first + childidx) < *(first + childidx - 1)) - childidx--; - *(first + topidx) = *(first + childidx); - topidx = childidx; - childidx = 2 * (topidx + 1); - } - /* if only left child exists.. */ - if (childidx == len) { - *(first + topidx) = *(first + childidx - 1); - topidx = childidx - 1; - } - /* now topidx is the hole.. */ - FastS_push_heap(first, topidx + 1, obj); - return ret; -} - - -/* Similar to FastS_pop_heap, this function, given a "hole" in the - * heap, heapify()es the heap downwards. It then inserts obj and - * adjusts the heap upwards. - */ - -template<typename T> -inline void -FastS__adjust_heap(T *first, int len, int hole, T obj) -{ - /* right child */ - int topidx = hole; - int childidx = 2 * (hole + 1); - /* while both right and left child exist.. */ - while (childidx < len) { - /* compare right to left child */ - if (*(first + childidx) < *(first + childidx - 1)) - childidx--; - *(first + topidx) = *(first + childidx); - topidx = childidx; - childidx = 2 * (topidx + 1); - } - /* if only left child exists.. */ - if (childidx == len) { - *(first + topidx) = *(first + childidx - 1); - topidx = childidx - 1; - } - /* now first[topidx] is the hole.. */ - FastS_push_heap(first, topidx + 1, obj); -} - -template <typename T> -inline void -FastS_make_heap(T *first, int len) -{ - if (len < 2) - return; - int parent = (len - 2)/2; - for (/**/; parent >= 0; parent--) { - int holeidx = parent; - T obj = *(first + parent); - int childidx = 2 * (parent + 1); - while (childidx < len) { - // Find largest of left, right child of holeidx, and object. - if (*(first + childidx) < *(first + childidx - 1)) - childidx--; - if (*(first + childidx) < obj) { - *(first + holeidx) = obj; - goto nextparent; - } else { - // If child is largest, put it at holeidx, and - // look further down. - *(first + holeidx) = *(first + childidx); - holeidx = childidx; - childidx = 2* (holeidx + 1); - } - } - if (childidx == len) { - // Only left child exists - if (*(first + childidx - 1) < obj) { - *(first + holeidx) = obj; - } else { - *(first + holeidx) = *(first + childidx - 1); - *(first + childidx - 1) = obj; - } - } else /* childidx > len */ - *(first + holeidx) = obj; - nextparent: - ; - } -} - -template <typename T> -inline void -FastS_sort_heap(T *first, int len) -{ - while (len > 0) { - *(first + len - 1) = FastS_pop_heap(first, len); - len--; - } -} - -template <typename T> -inline bool -FastS_is_heap(T *first, int len) -{ - for (int i = 0; i < len; i++) { - int left = 2 * i + 1; - int right = 2 * i + 2; - if (left < len && *(first + i) < *(first + left)) - return false; - if (right < len && *(first + i) < *(first + right)) - return false; - } - return true; -} - - diff --git a/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp b/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp index e38820b6e8b..7526326b6ca 100644 --- a/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp +++ b/searchlib/src/tests/engine/proto_converter/proto_converter_test.cpp @@ -2,7 +2,6 @@ #include <vespa/vespalib/gtest/gtest.h> #include <vespa/searchlib/engine/proto_converter.h> -#include <vespa/searchlib/common/transport.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/data/slime/binary_format.h> @@ -343,14 +342,14 @@ TEST_F(DocsumRequestTest, require_that_cache_query_is_converted) { proto.set_cache_query(true); convert(); EXPECT_TRUE(request.propertiesMap.cacheProperties().lookup("query").found()); - EXPECT_FALSE((request.queryFlags & search::fs4transport::QFLAG_DUMP_FEATURES) != 0); + EXPECT_FALSE(request.dumpFeatures); } TEST_F(DocsumRequestTest, require_that_dump_features_is_converted) { proto.set_dump_features(true); convert(); EXPECT_FALSE(request.propertiesMap.cacheProperties().lookup("query").found()); - EXPECT_TRUE((request.queryFlags & search::fs4transport::QFLAG_DUMP_FEATURES) != 0); + EXPECT_TRUE(request.dumpFeatures); } TEST_F(DocsumRequestTest, require_that_rank_profile_is_converted) { diff --git a/searchlib/src/vespa/searchlib/common/transport.h b/searchlib/src/vespa/searchlib/common/transport.h deleted file mode 100644 index 301acde7bd6..00000000000 --- a/searchlib/src/vespa/searchlib/common/transport.h +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <cstdint> - -namespace search::fs4transport { - -/** - * Instead of using a 32-bit number to send the 'usehardware' flag, we - * now use this 32-bit number to send 32 flags. The currently defined flags - * are as follows: - * <ul> - * <li><b>QFLAG_EXTENDED_COVERAGE</b>: Indicates that the it is able to receive extended coverage information.</li> - * <li><b>QFLAG_COVERAGE_NODES</b>: Indicate that it is able to handle nodes information.</li> - * <li><b>QFLAG_ESTIMATE</b>: Indicates that the query is performed to get - * an estimate of the total number of hits</li> - * <li><b>QFLAG_DUMP_FEATURES</b>: Dump detailed ranking information. Note that - * this flag will only be considered when sent in a - * GETDOCSUMSX packet. Is is put here to avoid having - * 2 separate query related flag spaces</li> - * <li><b>QFLAG_DROP_SORTDATA</b>: Don't return any sort data even if sortspec - * is used.</li> - * <li><b>QFLAG_NO_RESULTCACHE</b>: Do not use any result cache. Perform query no matter what.</li> - * </ul> - **/ -enum queryflags { - QFLAG_DROP_SORTDATA = 0x00004000, - QFLAG_DUMP_FEATURES = 0x00040000 -}; - -// docsum class for slime tunneling -const uint32_t SLIME_MAGIC_ID = 0x55555555; - -} diff --git a/searchlib/src/vespa/searchlib/engine/proto_converter.cpp b/searchlib/src/vespa/searchlib/engine/proto_converter.cpp index 1736cf1f72a..2495a6e12bd 100644 --- a/searchlib/src/vespa/searchlib/engine/proto_converter.cpp +++ b/searchlib/src/vespa/searchlib/engine/proto_converter.cpp @@ -5,7 +5,6 @@ #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/data/slime/binary_format.h> #include <vespa/vespalib/data/smart_buffer.h> -#include <vespa/searchlib/common/transport.h> namespace search::engine { @@ -119,9 +118,7 @@ ProtoConverter::docsum_request_from_proto(const ProtoDocsumRequest &proto, Docsu if (proto.cache_query()) { request.propertiesMap.lookupCreate(MapNames::CACHES).add("query", "true"); } - if (proto.dump_features()) { - request.queryFlags |= fs4transport::QFLAG_DUMP_FEATURES; - } + request.dumpFeatures = proto.dump_features(); request.ranking = proto.rank_profile(); if ((proto.feature_overrides_size() + proto.tensor_feature_overrides_size()) > 0) { auto &feature_overrides = request.propertiesMap.lookupCreate(MapNames::FEATURE); diff --git a/searchlib/src/vespa/searchlib/engine/request.cpp b/searchlib/src/vespa/searchlib/engine/request.cpp index 956653d5269..84615105579 100644 --- a/searchlib/src/vespa/searchlib/engine/request.cpp +++ b/searchlib/src/vespa/searchlib/engine/request.cpp @@ -1,14 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "request.h" -#include <vespa/searchlib/common/transport.h> namespace search::engine { Request::Request(RelativeTime relativeTime) : _relativeTime(std::move(relativeTime)), _timeOfDoom(fastos::TimeStamp(fastos::TimeStamp::FUTURE)), - queryFlags(0), + dumpFeatures(false), ranking(), location(), propertiesMap(), @@ -35,10 +34,4 @@ fastos::TimeStamp Request::getTimeLeft() const return _timeOfDoom - _relativeTime.now(); } -bool -Request::should_drop_sort_data() const -{ - return ((queryFlags & fs4transport::QFLAG_DROP_SORTDATA) != 0); -} - } diff --git a/searchlib/src/vespa/searchlib/engine/request.h b/searchlib/src/vespa/searchlib/engine/request.h index f5f24b6743f..a021ec6bfaa 100644 --- a/searchlib/src/vespa/searchlib/engine/request.h +++ b/searchlib/src/vespa/searchlib/engine/request.h @@ -28,8 +28,6 @@ public: return vespalib::stringref(&stackDump[0], stackDump.size()); } - bool should_drop_sort_data() const; - void setTraceLevel(uint32_t level, uint32_t minLevel) const { _trace.setLevel(level); _trace.start(minLevel); @@ -43,7 +41,7 @@ private: fastos::TimeStamp _timeOfDoom; public: /// Everything here should move up to private section and have accessors - uint32_t queryFlags; + bool dumpFeatures; vespalib::string ranking; vespalib::string location; PropertiesMap propertiesMap; diff --git a/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp b/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp index efeb066135f..c0d9ec2de3a 100644 --- a/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp +++ b/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp @@ -4,7 +4,6 @@ #include <vespa/searchsummary/docsummary/docsumwriter.h> #include <vespa/searchsummary/docsummary/resultpacker.h> #include <vespa/searchsummary/docsummary/docsumstate.h> -#include <vespa/searchlib/common/transport.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/searchlib/util/slime_output_raw_buf_adapter.h> @@ -43,7 +42,7 @@ struct DocsumFixture : IDocsumStore, GetDocsumsStateCallback { ASSERT_GREATER(buf.GetUsedLen(), sizeof(classId)); memcpy(&classId, buf.GetDrainPos(), sizeof(classId)); buf.Drain(sizeof(classId)); - EXPECT_EQUAL(classId, ::search::fs4transport::SLIME_MAGIC_ID); + EXPECT_EQUAL(classId, SLIME_MAGIC_ID); EXPECT_GREATER(vespalib::slime::BinaryFormat ::decode(Memory(buf.GetDrainPos(), buf.GetUsedLen()), slime), 0u); } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp index 4e3540fb573..0e647b43e78 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp @@ -4,7 +4,6 @@ #include "docsumstate.h" #include "docsum_field_writer_state.h" #include <vespa/searchcommon/common/undefinedvalues.h> -#include <vespa/searchlib/common/transport.h> #include <vespa/searchlib/util/slime_output_raw_buf_adapter.h> #include <vespa/searchlib/attribute/iattributemanager.h> #include <vespa/vespalib/data/slime/slime.h> @@ -20,7 +19,7 @@ uint32_t IDocsumWriter::slime2RawBuf(const Slime & slime, RawBuf & buf) { const uint32_t preUsed = buf.GetUsedLen(); - const uint32_t magic = ::search::fs4transport::SLIME_MAGIC_ID; + const uint32_t magic = SLIME_MAGIC_ID; buf.append(&magic, sizeof(magic)); SlimeOutputRawBufAdapter adapter(buf); vespalib::slime::BinaryFormat::encode(slime, adapter); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.h index 92b26d5cf14..e5dd2793089 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.h @@ -16,6 +16,8 @@ using search::IAttributeManager; namespace search::docsummary { +static constexpr uint32_t SLIME_MAGIC_ID = 0x55555555; + class IDocsumWriter { public: diff --git a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.cpp b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.cpp index 0029a57581c..8f8166a2806 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.cpp @@ -1,14 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "getdocsumargs.h" -#include "resultconfig.h" namespace search::docsummary { GetDocsumArgs::GetDocsumArgs() : _ranking(), - _qflags(0), _resultClassName(), + _dumpFeatures(false), _stackItems(0), _stackDump(), _location(), @@ -35,7 +34,7 @@ void GetDocsumArgs::initFromDocsumRequest(const search::engine::DocsumRequest &req) { _ranking = req.ranking; - _qflags = req.queryFlags; + _dumpFeatures = req.dumpFeatures; _resultClassName = req.resultClassName; _stackItems = req.stackItems; _stackDump = req.stackDump; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h index f0574e0d21c..ce5dc695f08 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h @@ -15,8 +15,8 @@ public: private: vespalib::string _ranking; - uint32_t _qflags; vespalib::string _resultClassName; + bool _dumpFeatures; uint32_t _stackItems; std::vector<char> _stackDump; vespalib::string _location; @@ -29,7 +29,6 @@ public: void initFromDocsumRequest(const search::engine::DocsumRequest &req); void SetRankProfile(const vespalib::string &ranking) { _ranking = ranking; } - void SetQueryFlags(uint32_t qflags) { _qflags = qflags; } void setResultClassName(vespalib::stringref name) { _resultClassName = name; } void SetStackDump(uint32_t stackItems, uint32_t stackDumpLen, const char *stackDump); void setLocation(vespalib::stringref location) { @@ -45,7 +44,8 @@ public: return vespalib::stringref(&_stackDump[0], _stackDump.size()); } - uint32_t GetQueryFlags() const { return _qflags; } + void dumpFeatures(bool v) { _dumpFeatures = v; } + bool dumpFeatures() const { return _dumpFeatures; } const PropsMap &propertiesMap() const { return _propertiesMap; } diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp index a1f1d41f87a..63972ca139d 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp @@ -9,7 +9,6 @@ #include <vespa/document/datatype/mapdatatype.h> #include <vespa/searchlib/aggregation/modifiers.h> #include <vespa/searchlib/common/packets.h> -#include <vespa/searchlib/common/transport.h> #include <vespa/searchlib/uca/ucaconverter.h> #include <vespa/searchlib/features/setup.h> #include <vespa/vespalib/geo/zcurve.h> @@ -51,6 +50,11 @@ ForceWordfolderInit::ForceWordfolderInit() static ForceWordfolderInit _G_forceNormWordFolderInit; +// Leftovers from FS4 protocol with limited use here. +enum queryflags { + QFLAG_DUMP_FEATURES = 0x00040000 +}; + AttributeVector::SP createMultiValueAttribute(const vespalib::string & name, const document::FieldValue & fv, bool arrayType) @@ -223,11 +227,11 @@ void SearchVisitor::init(const Parameters & params) LOG(debug, "Received rank profile: %s", _rankController.getRankProfile().c_str()); } - if (params.lookup("queryflags", valueRef) ) { - vespalib::string tmp(valueRef.data(), valueRef.size()); - LOG(debug, "Received query flags: 0x%lx", strtoul(tmp.c_str(), nullptr, 0)); - uint32_t queryFlags = strtoul(tmp.c_str(), nullptr, 0); - _rankController.setDumpFeatures((queryFlags & search::fs4transport::QFLAG_DUMP_FEATURES) != 0); + int queryFlags = 0; + if (params.get("queryflags", queryFlags)) { + bool dumpFeatures = (queryFlags & QFLAG_DUMP_FEATURES) != 0; + _summaryGenerator.getDocsumState()._args.dumpFeatures(dumpFeatures); + _rankController.setDumpFeatures(dumpFeatures); LOG(debug, "QFLAG_DUMP_FEATURES: %s", _rankController.getDumpFeatures() ? "true" : "false"); } @@ -263,11 +267,6 @@ void SearchVisitor::init(const Parameters & params) _summaryGenerator.getDocsumState()._args.SetRankProfile(tmp); } - int queryFlags = 0; - if (params.get("queryflags", queryFlags)) { - _summaryGenerator.getDocsumState()._args.SetQueryFlags(queryFlags); - } - vespalib::string location; if (params.lookup("location", valueRef)) { location = vespalib::string(valueRef.data(), valueRef.size()); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java index d3be7829927..bea9af458b4 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java @@ -1,5 +1,4 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.athenz.identityprovider.client; import com.google.common.cache.CacheBuilder; @@ -67,7 +66,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen private final static Duration ROLE_TOKEN_EXPIRY = Duration.ofMinutes(30); // TODO Make path to trust store config - private static final Path DEFAULT_TRUST_STORE = Paths.get(Defaults.getDefaults().underVespaHome("share/ssl/certs/yahoo_certificate_bundle.jks")); + private static final Path DEFAULT_TRUST_STORE = Paths.get("/opt/yahoo/share/ssl/certs/yahoo_certificate_bundle.jks"); public static final String CERTIFICATE_EXPIRY_METRIC_NAME = "athenz-tenant-cert.expiry.seconds"; @@ -78,7 +77,6 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen private final ScheduledExecutorService scheduler; private final Clock clock; private final AthenzService identity; - private final String dnsSuffix; private final URI ztsEndpoint; private final MutableX509KeyManager identityKeyManager = new MutableX509KeyManager(); @@ -102,7 +100,6 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen } // Test only - AthenzIdentityProviderImpl(IdentityConfig config, Metric metric, Path trustStore, @@ -115,7 +112,6 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen this.scheduler = scheduler; this.clock = clock; this.identity = new AthenzService(config.domain(), config.service()); - this.dnsSuffix = config.athenzDnsSuffix(); this.ztsEndpoint = URI.create(config.ztsUrl()); roleSslContextCache = createCache(ROLE_SSL_CONTEXT_EXPIRY, this::createRoleSslContext); roleSpecificRoleTokenCache = createCache(ROLE_TOKEN_EXPIRY, this::createRoleToken); diff --git a/vespaclient-java/src/main/java/com/yahoo/dummyreceiver/DummyReceiver.java b/vespaclient-java/src/main/java/com/yahoo/dummyreceiver/DummyReceiver.java index b1fd8e8cffa..a32aa75d994 100755 --- a/vespaclient-java/src/main/java/com/yahoo/dummyreceiver/DummyReceiver.java +++ b/vespaclient-java/src/main/java/com/yahoo/dummyreceiver/DummyReceiver.java @@ -2,7 +2,6 @@ package com.yahoo.dummyreceiver; import com.yahoo.concurrent.DaemonThreadFactory; -import com.yahoo.documentapi.ThroughputLimitQueue; import com.yahoo.documentapi.messagebus.MessageBusDocumentAccess; import com.yahoo.documentapi.messagebus.MessageBusParams; import com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet; @@ -42,7 +41,6 @@ public class DummyReceiver implements MessageHandler { private boolean instant = false; private ThreadPoolExecutor executor = null; private int threads = 10; - private long maxQueueTime = -1; private BlockingQueue<Runnable> queue; private boolean verbose = false; private boolean helpOption = false; @@ -76,7 +74,7 @@ public class DummyReceiver implements MessageHandler { params.getMessageBusParams().setMaxPendingCount(0); params.getMessageBusParams().setMaxPendingSize(0); da = new MessageBusDocumentAccess(params); - queue = (maxQueueTime < 0) ? new LinkedBlockingDeque<>() : new ThroughputLimitQueue<>(maxQueueTime); + queue = new LinkedBlockingDeque<>(); session = da.getMessageBus().createDestinationSession("default", true, this); executor = new ThreadPoolExecutor(threads, threads, 5, TimeUnit.SECONDS, queue, new DaemonThreadFactory()); System.out.println("Registered listener at " + name + "/default with 0 max pending and sleep time of " + sleepTime); @@ -156,8 +154,6 @@ public class DummyReceiver implements MessageHandler { instant = true; } else if ("--silent".equals(arg)) { silentNum = Long.parseLong(getParam(args, arg)); - } else if ("--maxqueuetime".equals(arg)) { - maxQueueTime = Long.parseLong(getParam(args, arg)); } else if ("--threads".equals(arg)) { threads = Integer.parseInt(getParam(args, arg)); } else if ("--verbose".equals(arg)) { diff --git a/vespalib/src/tests/net/socket/socket_test.cpp b/vespalib/src/tests/net/socket/socket_test.cpp index c7cb2a0b6d9..88146cd4fb2 100644 --- a/vespalib/src/tests/net/socket/socket_test.cpp +++ b/vespalib/src/tests/net/socket/socket_test.cpp @@ -324,6 +324,15 @@ TEST("require that sockets can be set blocking and non-blocking") { TEST_DO(verifier.verify_blocking(false)); } +TEST("require that server sockets use non-blocking underlying socket") { + ServerSocket tcp_server("tcp/0"); + ServerSocket ipc_server("ipc/file:my_socket"); + test::SocketOptionsVerifier tcp_verifier(tcp_server.get_fd()); + test::SocketOptionsVerifier ipc_verifier(ipc_server.get_fd()); + TEST_DO(tcp_verifier.verify_blocking(false)); + TEST_DO(ipc_verifier.verify_blocking(false)); +} + TEST("require that tcp nodelay can be enabled and disabled") { SocketHandle handle(socket(my_inet(), SOCK_STREAM, 0)); test::SocketOptionsVerifier verifier(handle.get()); diff --git a/vespalib/src/vespa/vespalib/net/server_socket.cpp b/vespalib/src/vespa/vespalib/net/server_socket.cpp index cc363f49763..2928a9a72b0 100644 --- a/vespalib/src/vespa/vespalib/net/server_socket.cpp +++ b/vespalib/src/vespa/vespalib/net/server_socket.cpp @@ -4,12 +4,27 @@ #include "socket_spec.h" #include <sys/stat.h> #include <dirent.h> +#include <errno.h> +#include <chrono> +#include <thread> #include <vespa/log/log.h> LOG_SETUP(".vespalib.net.server_socket"); namespace vespalib { +namespace { + +SocketHandle adjust_blocking(SocketHandle handle, bool value) { + if (handle.valid() && handle.set_blocking(value)) { + return handle; + } else { + return SocketHandle(); + } +} + +bool is_blocked(int err) { return ((err == EWOULDBLOCK) || (err == EAGAIN)); } + bool is_socket(const vespalib::string &path) { struct stat info; if (path.empty() || (lstat(path.c_str(), &info) != 0)) { @@ -18,6 +33,8 @@ bool is_socket(const vespalib::string &path) { return S_ISSOCK(info.st_mode); } +} + void ServerSocket::cleanup() { @@ -27,8 +44,10 @@ ServerSocket::cleanup() } ServerSocket::ServerSocket(const SocketSpec &spec) - : _handle(spec.server_address().listen()), - _path(spec.path()) + : _handle(adjust_blocking(spec.server_address().listen(), false)), + _path(spec.path()), + _blocking(true), + _shutdown(false) { if (!_handle.valid() && is_socket(_path)) { if (!spec.client_address().connect_async().valid()) { @@ -54,7 +73,9 @@ ServerSocket::ServerSocket(int port) ServerSocket::ServerSocket(ServerSocket &&rhs) : _handle(std::move(rhs._handle)), - _path(std::move(rhs._path)) + _path(std::move(rhs._path)), + _blocking(rhs._blocking), + _shutdown(rhs._shutdown.load(std::memory_order_acquire)) { rhs._path.clear(); } @@ -65,6 +86,8 @@ ServerSocket::operator=(ServerSocket &&rhs) cleanup(); _handle = std::move(rhs._handle); _path = std::move(rhs._path); + _blocking = rhs._blocking; + _shutdown.store(rhs._shutdown.load(std::memory_order_acquire), std::memory_order_release); rhs._path.clear(); return *this; } @@ -78,13 +101,28 @@ ServerSocket::address() const void ServerSocket::shutdown() { + _shutdown.store(true, std::memory_order_release); _handle.shutdown(); } SocketHandle ServerSocket::accept() { - return _handle.accept(); + if (!_blocking) { + return adjust_blocking(_handle.accept(), true); + } else { + for (;;) { + if (_shutdown.load(std::memory_order_acquire)) { + errno = EIO; + return SocketHandle(); + } + SocketHandle res = _handle.accept(); + if (res.valid() || !is_blocked(errno)) { + return adjust_blocking(std::move(res), true); + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + } } } // namespace vespalib diff --git a/vespalib/src/vespa/vespalib/net/server_socket.h b/vespalib/src/vespa/vespalib/net/server_socket.h index 1ad0a738117..af6f0a31175 100644 --- a/vespalib/src/vespa/vespalib/net/server_socket.h +++ b/vespalib/src/vespa/vespalib/net/server_socket.h @@ -4,6 +4,7 @@ #include "socket_handle.h" #include "socket_address.h" +#include <atomic> namespace vespalib { @@ -14,9 +15,9 @@ class ServerSocket private: SocketHandle _handle; vespalib::string _path; + bool _blocking; + std::atomic<bool> _shutdown; - explicit ServerSocket(SocketHandle handle); - static ServerSocket listen(const SocketSpec &spec); void cleanup(); public: ServerSocket() : _handle(), _path() {} @@ -30,7 +31,10 @@ public: int get_fd() const { return _handle.get(); } SocketAddress address() const; void shutdown(); - bool set_blocking(bool value) { return _handle.set_blocking(value); } + bool set_blocking(bool value) { + _blocking = value; + return true; + } SocketHandle accept(); }; diff --git a/vespalib/src/vespa/vespalib/websocket/acceptor.cpp b/vespalib/src/vespa/vespalib/websocket/acceptor.cpp index d99035e0be7..4ba1bf3b908 100644 --- a/vespalib/src/vespa/vespalib/websocket/acceptor.cpp +++ b/vespalib/src/vespa/vespalib/websocket/acceptor.cpp @@ -3,34 +3,15 @@ #include "acceptor.h" #include <vespa/vespalib/net/socket_spec.h> #include <functional> -#ifdef __APPLE__ -#include <poll.h> -#endif namespace vespalib::ws { void Acceptor::accept_main(Handler<Socket> &socket_handler) { -#ifdef __APPLE__ - _server_socket.set_blocking(false); -#endif while (!_is_closed) { -#ifdef __APPLE__ - pollfd fds; - fds.fd = _server_socket.get_fd(); - fds.events = POLLIN; - fds.revents = 0; - int res = poll(&fds, 1, 10); - if (res < 1 || fds.revents == 0 || _is_closed) { - continue; - } -#endif SocketHandle handle = _server_socket.accept(); if (handle.valid()) { -#ifdef __APPLE__ - handle.set_blocking(true); -#endif socket_handler.handle(std::make_unique<SimpleSocket>(std::move(handle))); } } diff --git a/vespalog/src/main/java/com/yahoo/log/LevelControllerRepo.java b/vespalog/src/main/java/com/yahoo/log/LevelControllerRepo.java index 1a0dbac7de9..364420796d3 100644 --- a/vespalog/src/main/java/com/yahoo/log/LevelControllerRepo.java +++ b/vespalog/src/main/java/com/yahoo/log/LevelControllerRepo.java @@ -14,10 +14,10 @@ public interface LevelControllerRepo { * @param component The component name string. * @return The LevelController corresponding to that component. Return null if not found. */ - public LevelController getLevelController(String component); + LevelController getLevelController(String component); /** * Close down the level controller repository. Cleanup should be done here. */ - public void close(); + void close(); } diff --git a/vespalog/src/main/java/com/yahoo/log/VespaFormatter.java b/vespalog/src/main/java/com/yahoo/log/VespaFormatter.java index 72a80227138..482d7049079 100644 --- a/vespalog/src/main/java/com/yahoo/log/VespaFormatter.java +++ b/vespalog/src/main/java/com/yahoo/log/VespaFormatter.java @@ -39,13 +39,14 @@ public class VespaFormatter extends SimpleFormatter { } private String serviceName; - private String componentPrefix; + private final String componentPrefix; /** * Default constructor */ public VespaFormatter() { this.serviceName = serviceNameUnsetValue; + this.componentPrefix = null; } /** @@ -193,9 +194,6 @@ public class VespaFormatter extends SimpleFormatter { String message = t.getMessage(); if (t.getCause() == null) { if (message == null) return t.getClass().getSimpleName(); - } else { - if (message == null) return null; - //if (message.equals(t.getCause().getClass().getName() + ": " + t.getCause().getMessage())) return null; } return message; } diff --git a/vespalog/src/main/java/com/yahoo/log/VespaLevelControllerRepo.java b/vespalog/src/main/java/com/yahoo/log/VespaLevelControllerRepo.java index d47f78521a2..85d92075827 100644 --- a/vespalog/src/main/java/com/yahoo/log/VespaLevelControllerRepo.java +++ b/vespalog/src/main/java/com/yahoo/log/VespaLevelControllerRepo.java @@ -49,13 +49,11 @@ public class VespaLevelControllerRepo implements LevelControllerRepo { * or logging without a logcontrol file) **/ private LevelController defaultLevelCtrl; - private String defaultLogLevel; public VespaLevelControllerRepo(String logCtlFn, String logLevel, String applicationPrefix) { this.logControlFilename = logCtlFn; - this.defaultLogLevel = logLevel; this.appPrefix = applicationPrefix; - defaultLevelCtrl = new DefaultLevelController(defaultLogLevel); + defaultLevelCtrl = new DefaultLevelController(logLevel); openCtlFile(); } @@ -158,7 +156,7 @@ public class VespaLevelControllerRepo implements LevelControllerRepo { if (ctlFile == null) { return defaultLevelCtrl; } - LevelController inherit = null; + LevelController inherit; int lastdot = suffix.lastIndexOf('.'); if (lastdot != -1) { @@ -173,18 +171,18 @@ public class VespaLevelControllerRepo implements LevelControllerRepo { } try { long len = ctlFile.length(); - String append; + StringBuilder sb = new StringBuilder(); if (suffix.equals("")) { - append = "default" + ": "; + sb.append("default: "); } else { - append = "." + suffix + ": "; + sb.append(".").append(suffix).append(": "); } - while ((len + append.length()) % 4 != 0) { - append = append + " "; + while ((len + sb.length()) % 4 != 0) { + sb.append(" "); } - append = append + inherit.getOnOffString() + "\n"; + sb.append(inherit.getOnOffString()).append("\n"); ctlFile.seek(ctlFile.length()); - ctlFile.writeBytes(append); + ctlFile.writeBytes(sb.toString()); extendMapping(); ctrl = levelControllerRepo.getLevelController(suffix); } catch(java.nio.channels.ClosedByInterruptException e) { diff --git a/vespalog/src/main/java/com/yahoo/log/VespaLogHandler.java b/vespalog/src/main/java/com/yahoo/log/VespaLogHandler.java index 336e9e06cb3..7eede7e1e35 100644 --- a/vespalog/src/main/java/com/yahoo/log/VespaLogHandler.java +++ b/vespalog/src/main/java/com/yahoo/log/VespaLogHandler.java @@ -31,7 +31,7 @@ class VespaLogHandler extends StreamHandler { * <DD> Log to specified file in append mode * </DL> */ - public VespaLogHandler(LogTarget logTarget, + VespaLogHandler(LogTarget logTarget, LevelControllerRepo levelControllerRepo, String serviceName, String applicationPrefix) { this.logTarget = logTarget; @@ -62,8 +62,7 @@ class VespaLogHandler extends StreamHandler { // provokes rotation of target setOutputStream(logTarget.open()); } catch (RuntimeException e) { - LogRecord r = new LogRecord(Level.SEVERE, - "Unable to open file target"); + LogRecord r = new LogRecord(Level.SEVERE, "Unable to open file target"); r.setThrown(e); emergencyLog(r); setOutputStream(System.err); @@ -73,7 +72,7 @@ class VespaLogHandler extends StreamHandler { closeFileTarget(); } - public LevelController getLevelControl(String component) { + LevelController getLevelControl(String component) { return repo.getLevelController(component); } @@ -96,8 +95,7 @@ class VespaLogHandler extends StreamHandler { emergencyLog(r); } catch (RuntimeException e) { - LogRecord r = new LogRecord(Level.SEVERE, - "Unable to open file target"); + LogRecord r = new LogRecord(Level.SEVERE, "Unable to open file target"); r.setThrown(e); emergencyLog(r); setOutputStream(System.err); @@ -106,7 +104,7 @@ class VespaLogHandler extends StreamHandler { /** Closes the target log file, if there is one */ - public void closeFileTarget() { + synchronized void closeFileTarget() { try { logTarget.close(); } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java index 5b52f757dad..5615e1133b8 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java @@ -30,7 +30,8 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { ZooKeeperServer(ZookeeperServerConfig zookeeperServerConfig, boolean startServer) { this.zookeeperServerConfig = zookeeperServerConfig; System.setProperty("zookeeper.jmx.log4j.disable", "true"); - System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, "" + zookeeperServerConfig.juteMaxBuffer()); + System.setProperty("zookeeper.snapshot.trust.empty", Boolean.valueOf(zookeeperServerConfig.trustEmptySnapshot()).toString()); + System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, Integer.valueOf(zookeeperServerConfig.juteMaxBuffer()).toString()); writeConfigToDisk(zookeeperServerConfig); zkServerThread = new Thread(this, "zookeeper server"); @@ -113,7 +114,7 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { public void run() { System.setProperty(ZOOKEEPER_JMX_LOG4J_DISABLE, "true"); String[] args = new String[]{getDefaults().underVespaHome(zookeeperServerConfig.zooKeeperConfigFile())}; - log.log(LogLevel.DEBUG, "Starting ZooKeeper server with config: " + args[0]); + log.log(LogLevel.DEBUG, "Starting ZooKeeper server with config file " + args[0]); log.log(LogLevel.INFO, "Trying to establish ZooKeeper quorum (from " + zookeeperServerHostnames(zookeeperServerConfig) + ")"); org.apache.zookeeper.server.quorum.QuorumPeerMain.main(args); } |