diff options
69 files changed, 615 insertions, 206 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricSet.java index cd6035204e2..41204ce6e9a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricSet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricSet.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.admin.monitoring; -import javax.annotation.concurrent.Immutable; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; @@ -14,10 +13,10 @@ import static java.util.Collections.unmodifiableMap; /** * Models a metric set containing a set of metrics and child metric sets. + * This should be immutable. * * @author gjoranv */ -@Immutable public class MetricSet { private final String id; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java index 05f2ebee446..6aec922af85 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java @@ -627,8 +627,7 @@ public class VespaMetricSet { // TODO: For the purpose of this file and likely elsewhere, all but the last aggregate specifier, // TODO: such as 'average' and 'sum' in the metric names below are just confusing and can be mentally - // TODO: disregarded when considering metric names. Consider cleaning up for Vespa 8. - // TODO Vespa 8 all metrics with .sum in the name should have that removed. + // TODO: disregarded when considering metric names. Consider cleaning up for Vespa 9. metrics.add(new Metric("vds.datastored.alldisks.docs.average")); metrics.add(new Metric("vds.datastored.alldisks.bytes.average")); metrics.add(new Metric("vds.visitor.allthreads.averagevisitorlifetime.sum.max")); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ResourceLimits.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ResourceLimits.java index 26bf8a33a73..df269c7a9a7 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/ResourceLimits.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ResourceLimits.java @@ -31,13 +31,12 @@ public class ResourceLimits implements FleetcontrollerConfig.Producer, ProtonCon @Override public void getConfig(FleetcontrollerConfig.Builder builder) { - // TODO: Choose other defaults when this is default enabled. // Note: The resource categories must match the ones used in host info reporting // between content nodes and cluster controller: // storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp builder.cluster_feed_block_limit.put("memory", memoryLimit.orElse(0.8)); - builder.cluster_feed_block_limit.put("disk", diskLimit.orElse(0.8)); - builder.cluster_feed_block_limit.put("attribute-address-space", 0.89); + builder.cluster_feed_block_limit.put("disk", diskLimit.orElse(0.75)); + builder.cluster_feed_block_limit.put("attribute-address-space", 0.9); } @Override diff --git a/config-model/src/test/derived/slice/query-profiles/default.xml b/config-model/src/test/derived/slice/query-profiles/default.xml new file mode 100644 index 00000000000..2535ca895ed --- /dev/null +++ b/config-model/src/test/derived/slice/query-profiles/default.xml @@ -0,0 +1,3 @@ +<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> +<query-profile id="default" type="DefaultQueryProfileType"> +</query-profile> diff --git a/config-model/src/test/derived/slice/query-profiles/types/DefaultQueryProfileType.xml b/config-model/src/test/derived/slice/query-profiles/types/DefaultQueryProfileType.xml new file mode 100644 index 00000000000..50970d8743f --- /dev/null +++ b/config-model/src/test/derived/slice/query-profiles/types/DefaultQueryProfileType.xml @@ -0,0 +1,4 @@ +<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> +<query-profile-type id="DefaultQueryProfileType" inherits="native"> + <field name="ranking.features.query(myTensor)" type="tensor<float>(key{})" /> +</query-profile-type> diff --git a/config-model/src/test/derived/slice/test.sd b/config-model/src/test/derived/slice/test.sd new file mode 100644 index 00000000000..fbb581d1b1d --- /dev/null +++ b/config-model/src/test/derived/slice/test.sd @@ -0,0 +1,21 @@ +search test { + + document test { + } + + rank-profile parent { + + function inline cpmScore() { + expression: myValue * mySlice(query(myTensor)) + } + + function inline myValue() { + expression: 4 + } + + function inline mySlice(myTensor) { + expression: myTensor{"NULL"} + } + } + +}
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/SliceTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/SliceTestCase.java new file mode 100644 index 00000000000..e6c7efd7052 --- /dev/null +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/SliceTestCase.java @@ -0,0 +1,27 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.searchdefinition.derived; + +import com.yahoo.component.ComponentId; +import com.yahoo.search.Query; +import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry; +import com.yahoo.search.query.profile.config.QueryProfileConfigurer; +import com.yahoo.searchdefinition.parser.ParseException; +import com.yahoo.vespa.model.container.search.QueryProfiles; +import org.junit.Test; + +import java.io.IOException; + +import static org.junit.Assert.assertEquals; + +/** + * @author bratseth + */ +public class SliceTestCase extends AbstractExportingTestCase { + + @Test + public void testSlice() throws IOException, ParseException { + ComponentId.resetGlobalCountersForTests(); + DerivedConfiguration c = assertCorrectDeriving("slice"); + } + +} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java index 2e735e9c1b0..f9fcb777408 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java @@ -121,7 +121,7 @@ public class FleetControllerClusterTest { assertEquals(3, limits.size()); assertEquals(expDisk, limits.get("disk"), DELTA); assertEquals(expMemory, limits.get("memory"), DELTA); - assertEquals(0.89, limits.get("attribute-address-space"), DELTA); + assertEquals(0.9, limits.get("attribute-address-space"), DELTA); } private FleetcontrollerConfig getConfigForResourceLimitsTuning(Double diskLimit, Double memoryLimit) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java index 85762cf530a..cc444d50c3e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java @@ -84,7 +84,7 @@ public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceL /** Returns the subset of instances which currently have failing jobs on the given version */ public InstanceList failingOn(Version version) { return matching(id -> ! instances.get(id).instanceJobs().get(id).failing() - .not().withStatus(outOfCapacity) + .not().outOfTestCapacity() .lastCompleted().on(version).isEmpty()); } @@ -95,7 +95,7 @@ public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceL /** Returns the subset of instances which are not currently failing any jobs. */ public InstanceList failing() { - return matching(id -> ! instances.get(id).instanceJobs().get(id).failing().not().withStatus(outOfCapacity).isEmpty()); + return matching(id -> ! instances.get(id).instanceJobs().get(id).failing().not().outOfTestCapacity().isEmpty()); } /** Returns the subset of instances which are currently failing an upgrade. */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index d3271a4abb1..84162729b43 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -392,10 +392,12 @@ public class DeploymentStatus { Optional<Instant> revisionReadyAt = step.dependenciesCompletedAt(change.withoutPlatform(), Optional.of(job)); // If neither change is ready, we guess based on the specified rollout. - if (platformReadyAt.isEmpty() && revisionReadyAt.isEmpty()) switch (rollout) { - case separate: return List.of(change.withoutApplication(), change); // Platform should stay ahead. - case leading: return List.of(change); // They should eventually join. - case simultaneous: return List.of(change.withoutPlatform(), change); // Revision should get ahead. + if (platformReadyAt.isEmpty() && revisionReadyAt.isEmpty()) { + switch (rollout) { + case separate: return List.of(change.withoutApplication(), change); // Platform should stay ahead. + case leading: return List.of(change); // They should eventually join. + case simultaneous: return List.of(change.withoutPlatform(), change); // Revision should get ahead. + } } // If only the revision is ready, we run that first. diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index 9af838b1f55..960205526b7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -15,18 +15,19 @@ import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import java.time.Duration; -import java.util.Collection; +import java.util.ArrayList; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Random; -import java.util.function.BinaryOperator; -import java.util.function.Function; +import java.util.function.UnaryOperator; +import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.PLATFORM; -import static java.util.Comparator.naturalOrder; +import static java.util.Comparator.reverseOrder; /** * Maintenance job which schedules applications for Vespa version upgrade @@ -54,91 +55,89 @@ public class Upgrader extends ControllerMaintainer { public double maintain() { // Determine target versions for each upgrade policy VersionStatus versionStatus = controller().readVersionStatus(); - Version canaryTarget = controller().systemVersion(versionStatus); - Collection<Version> defaultTargets = targetVersions(Confidence.normal, versionStatus); - Collection<Version> conservativeTargets = targetVersions(Confidence.high, versionStatus); + cancelBrokenUpgrades(versionStatus); - cancelUpgrades(versionStatus, canaryTarget, defaultTargets, conservativeTargets); - upgrade(versionStatus, canaryTarget, defaultTargets, conservativeTargets); - return 1.0; - } + Optional<Integer> targetMajorVersion = targetMajorVersion(); + InstanceList instances = instances(controller().systemVersion(versionStatus)); + for (UpgradePolicy policy : UpgradePolicy.values()) + updateTargets(versionStatus, instances, policy, targetMajorVersion); - /** Returns the target versions for given confidence, one per major version in the system */ - private Collection<Version> targetVersions(Confidence confidence, VersionStatus versionStatus) { - return versionStatus.versions().stream() - // Ensure we never pick a version newer than the system - .filter(v -> !v.versionNumber().isAfter(controller().systemVersion(versionStatus))) - .filter(v -> v.confidence().equalOrHigherThan(confidence)) - .map(VespaVersion::versionNumber) - .collect(Collectors.toMap(Version::getMajor, // Key on major version - Function.identity(), // Use version as value - BinaryOperator.<Version>maxBy(naturalOrder()))) // Pick highest version when merging versions within this major - .values(); + return 1.0; } /** Returns a list of all production application instances, except those which are pinned, which we should not manipulate here. */ private InstanceList instances(Version systemVersion) { return InstanceList.from(controller().jobController().deploymentStatuses(ApplicationList.from(controller().applications().readable()), systemVersion)) .withDeclaredJobs() + .shuffle(random) + .byIncreasingDeployedVersion() .unpinned(); } - private void cancelUpgrades(VersionStatus versionStatus, Version canaryTarget, Collection<Version> defaultTargets, Collection<Version> conservativeTargets) { - InstanceList instances = instances(controller().systemVersion(versionStatus)); + private void cancelBrokenUpgrades(VersionStatus versionStatus) { // Cancel upgrades to broken targets (let other ongoing upgrades complete to avoid starvation) + InstanceList instances = instances(controller().systemVersion(versionStatus)); for (VespaVersion version : versionStatus.versions()) { if (version.confidence() == Confidence.broken) - cancelUpgradesOf(instances.upgradingTo(version.versionNumber()) - .not().with(UpgradePolicy.canary), + cancelUpgradesOf(instances.upgradingTo(version.versionNumber()).not().with(UpgradePolicy.canary), version.versionNumber() + " is broken"); } + } - // Canaries should always try the canary target - cancelUpgradesOf(instances.upgrading() - .not().upgradingTo(canaryTarget) - .with(UpgradePolicy.canary), - "Outdated target version for Canaries"); - - // Cancel *failed* upgrades to earlier versions, as the new version may fix it - String reason = "Failing on outdated version"; - cancelUpgradesOf(instances.upgrading() - .failing() - .not().upgradingTo(defaultTargets) - .with(UpgradePolicy.defaultPolicy), - reason); - cancelUpgradesOf(instances.upgrading() - .failing() - .not().upgradingTo(conservativeTargets) - .with(UpgradePolicy.conservative), - reason); - } - - private void upgrade(VersionStatus versionStatus, Version canaryTarget, Collection<Version> defaultTargets, Collection<Version> conservativeTargets) { - InstanceList instances = instances(controller().systemVersion(versionStatus)); - Optional<Integer> targetMajorVersion = targetMajorVersion(); - upgrade(instances.with(UpgradePolicy.canary), canaryTarget, targetMajorVersion, instances.size()); - defaultTargets.forEach(target -> upgrade(instances.with(UpgradePolicy.defaultPolicy), target, targetMajorVersion, numberOfApplicationsToUpgrade())); - conservativeTargets.forEach(target -> upgrade(instances.with(UpgradePolicy.conservative), target, targetMajorVersion, numberOfApplicationsToUpgrade())); + private void updateTargets(VersionStatus versionStatus, InstanceList instances, UpgradePolicy policy, Optional<Integer> targetMajorVersion) { + InstanceList remaining = instances.with(policy); + List<Version> targetAndNewer = new ArrayList<>(); + UnaryOperator<InstanceList> cancellationCriterion = policy == UpgradePolicy.canary ? i -> i.not().upgradingTo(targetAndNewer) + : i -> i.failing() + .not().upgradingTo(targetAndNewer); + + Map<ApplicationId, Version> targets = new LinkedHashMap<>(); + for (Version version : targetsForConfidence(versionStatus, policy)) { + targetAndNewer.add(version); + InstanceList eligible = eligibleForVersion(remaining, version, cancellationCriterion, targetMajorVersion); + InstanceList outdated = cancellationCriterion.apply(eligible); + cancelUpgradesOf(outdated.upgrading(), "Upgrading to outdated versions"); + + remaining = remaining.not().matching(eligible.asList()::contains); // Prefer the newest target for each instance. + for (ApplicationId id : outdated.and(eligible.not().deploying())) + targets.put(id, version); + } + + int numberToUpgrade = policy == UpgradePolicy.canary ? instances.size() : numberOfApplicationsToUpgrade(); + for (ApplicationId id : instances.matching(targets.keySet()::contains).first(numberToUpgrade)) { + log.log(Level.INFO, "Triggering upgrade to " + targets.get(id) + " for " + id); + controller().applications().deploymentTrigger().triggerChange(id, Change.of(targets.get(id))); + } + } + + /** Returns target versions for given confidence, by descending version number. */ + private List<Version> targetsForConfidence(VersionStatus versions, UpgradePolicy policy) { + Version systemVersion = controller().systemVersion(versions); + if (policy == UpgradePolicy.canary) + return List.of(systemVersion); + + Confidence target = policy == UpgradePolicy.defaultPolicy ? Confidence.normal : Confidence.high; + return versions.versions().stream() + .filter(version -> ! version.versionNumber().isAfter(systemVersion) + && version.confidence().equalOrHigherThan(target)) + .map(VespaVersion::versionNumber) + .sorted(reverseOrder()) + .collect(Collectors.toList()); } - private void upgrade(InstanceList instances, Version version, Optional<Integer> targetMajorVersion, int numberToUpgrade) { + private InstanceList eligibleForVersion(InstanceList instances, Version version, UnaryOperator<InstanceList> cancellationCriterion, Optional<Integer> targetMajorVersion) { Change change = Change.of(version); - instances.not().failingOn(version) - .allowMajorVersion(version.getMajor(), targetMajorVersion.orElse(version.getMajor())) - .not().deploying() - .not().hasCompleted(change) // Avoid rescheduling change for instances without production steps - .onLowerVersionThan(version) - .canUpgradeAt(version, controller().clock().instant()) - .shuffle(random) // Shuffle so we do not always upgrade instances in the same order - .byIncreasingDeployedVersion() - .first(numberToUpgrade) - .forEach(instance -> controller().applications().deploymentTrigger().triggerChange(instance, change)); + return instances.not().failingOn(version) + .allowMajorVersion(version.getMajor(), targetMajorVersion.orElse(version.getMajor())) + .not().hasCompleted(change) // Avoid rescheduling change for instances without production steps. + .onLowerVersionThan(version) + .canUpgradeAt(version, controller().clock().instant()); } private void cancelUpgradesOf(InstanceList instances, String reason) { instances = instances.unpinned(); if (instances.isEmpty()) return; - log.info("Cancelling upgrading of " + instances.asList().size() + " instances: " + reason); + log.info("Cancelling upgrading of " + instances.asList() + " instances: " + reason); for (ApplicationId instance : instances.asList()) controller().applications().deploymentTrigger().cancelChange(instance, PLATFORM); } 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 fb34d57ba0d..9f108be9650 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 @@ -1263,9 +1263,6 @@ public class DeploymentTriggerTest { assertEquals(Change.empty(), app3.instance().change()); } - // TODO test multi-tier pipeline with various policies - // TODO test multi-tier pipeline with upgrade after new version is the candidate - @Test public void testRevisionJoinsUpgradeWithSeparateRollout() { var appPackage = new ApplicationPackageBuilder().region("us-central-1") @@ -1422,6 +1419,63 @@ public class DeploymentTriggerTest { } @Test + public void testsVeryLengthyPipeline() { + String lengthyDeploymentSpec = + "<deployment version='1.0'>\n" + + " <instance id='alpha'>\n" + + " <test />\n" + + " <staging />\n" + + " <upgrade rollout='simultaneous' />\n" + + " <prod>\n" + + " <region>us-east-3</region>\n" + + " <test>us-east-3</test>\n" + + " </prod>\n" + + " </instance>\n" + + " <instance id='beta'>\n" + + " <upgrade rollout='simultaneous' />\n" + + " <prod>\n" + + " <region>us-east-3</region>\n" + + " <test>us-east-3</test>\n" + + " </prod>\n" + + " </instance>\n" + + " <instance id='gamma'>\n" + + " <upgrade rollout='separate' />\n" + + " <prod>\n" + + " <region>us-east-3</region>\n" + + " <test>us-east-3</test>\n" + + " </prod>\n" + + " </instance>\n" + + "</deployment>\n"; + var appPackage = ApplicationPackageBuilder.fromDeploymentXml(lengthyDeploymentSpec); + var alpha = tester.newDeploymentContext("t", "a", "alpha"); + var beta = tester.newDeploymentContext("t", "a", "beta"); + var gamma = tester.newDeploymentContext("t", "a", "gamma"); + alpha.submit(appPackage).deploy(); + + // A version releases, but when the first upgrade has gotten through alpha, beta, and gamma, a newer version has high confidence. + var version0 = tester.controller().readSystemVersion(); + var version1 = new Version("7.1"); + var version2 = new Version("7.2"); + tester.controllerTester().upgradeSystem(version1); + + tester.upgrader().maintain(); + alpha.runJob(systemTest).runJob(stagingTest) + .runJob(productionUsEast3).runJob(testUsEast3); + assertEquals(Change.empty(), alpha.instance().change()); + + tester.upgrader().maintain(); + beta.runJob(productionUsEast3); + tester.controllerTester().upgradeSystem(version2); + beta.runJob(testUsEast3); + assertEquals(Change.empty(), beta.instance().change()); + + tester.upgrader().maintain(); + assertEquals(Change.of(version2), alpha.instance().change()); + assertEquals(Change.empty(), beta.instance().change()); + assertEquals(Change.of(version1), gamma.instance().change()); + } + + @Test public void testRevisionJoinsUpgradeWithLeadingRollout() { var appPackage = new ApplicationPackageBuilder().region("us-central-1") .region("us-east-3") 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 0e61ffd3ea6..265dedec8d0 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 @@ -35,6 +35,7 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobTy import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.ALL; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.PIN; +import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.PLATFORM; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -533,7 +534,7 @@ public class UpgraderTest { } @Test - public void testBlockVersionChangeHalfwayThoughThenNewRevision() { + public void testBlockVersionChangeHalfwayThroughThenNewRevision() { // Friday, 16:00 tester.at(Instant.parse("2017-09-29T16:00:00.00Z")); @@ -541,7 +542,7 @@ public class UpgraderTest { tester.controllerTester().upgradeSystem(version); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - // Block upgrades on weekends and ouside working hours + // Block upgrades on weekends and outside working hours .blockChange(false, true, "mon-fri", "00-09,17-23", "UTC") .blockChange(false, true, "sat-sun", "00-23", "UTC") .region("us-west-1") @@ -577,11 +578,12 @@ public class UpgraderTest { app.runJob(stagingTest).triggerJobs().jobAborted(productionUsCentral1); // Retry will include revision. tester.triggerJobs(); // Triggers us-central-1 before platform upgrade is cancelled. - // A new version is also released, cancelling the upgrade, since it is failing on a now outdated version. + // A new version is also released, and someone cancels the upgrade, suspecting it is faulty. tester.clock().advance(Duration.ofHours(17)); version = Version.fromString("6.4"); tester.controllerTester().upgradeSystem(version); tester.upgrader().maintain(); + tester.deploymentTrigger().cancelChange(app.instanceId(), PLATFORM); // us-central-1 succeeds upgrade to 6.3, with the revision, but us-east-3 wants to proceed with only the revision change. app.runJob(productionUsCentral1); diff --git a/document/abi-spec.json b/document/abi-spec.json index d5ad686cd1f..83a56d4fb5e 100644 --- a/document/abi-spec.json +++ b/document/abi-spec.json @@ -2576,6 +2576,24 @@ "public static final java.lang.String NAME" ] }, + "com.yahoo.document.fieldset.DocumentOnly": { + "superClass": "java.lang.Object", + "interfaces": [ + "com.yahoo.document.fieldset.FieldSet" + ], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>()", + "public boolean contains(com.yahoo.document.fieldset.FieldSet)", + "public com.yahoo.document.fieldset.FieldSet clone()", + "public bridge synthetic java.lang.Object clone()" + ], + "fields": [ + "public static final java.lang.String NAME" + ] + }, "com.yahoo.document.fieldset.FieldCollection": { "superClass": "java.util.ArrayList", "interfaces": [ diff --git a/document/src/main/java/com/yahoo/document/fieldset/DocumentOnly.java b/document/src/main/java/com/yahoo/document/fieldset/DocumentOnly.java new file mode 100644 index 00000000000..7ddfad13dfc --- /dev/null +++ b/document/src/main/java/com/yahoo/document/fieldset/DocumentOnly.java @@ -0,0 +1,18 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.document.fieldset; + +/** + * @author arnej27959 + */ +public class DocumentOnly implements FieldSet { + public static final String NAME = "[document]"; + @Override + public boolean contains(FieldSet o) { + return (o instanceof DocumentOnly || o instanceof DocIdOnly || o instanceof NoFields); + } + + @Override + public FieldSet clone() { + return new DocumentOnly(); + } +} diff --git a/document/src/main/java/com/yahoo/document/fieldset/FieldSetRepo.java b/document/src/main/java/com/yahoo/document/fieldset/FieldSetRepo.java index 41a1b898867..9b6507d171a 100644 --- a/document/src/main/java/com/yahoo/document/fieldset/FieldSetRepo.java +++ b/document/src/main/java/com/yahoo/document/fieldset/FieldSetRepo.java @@ -21,13 +21,14 @@ public class FieldSetRepo { FieldSet parseSpecialValues(String name) { if (name.equals(DocIdOnly.NAME)) { return new DocIdOnly(); } + else if (name.equals(DocumentOnly.NAME)) { return (new DocumentOnly()); } else if (name.equals(AllFields.NAME)) { return (new AllFields()); } else if (name.equals(NoFields.NAME)) { return (new NoFields()); } else if (name.equals("[docid]")) { return (new DocIdOnly()); } else { throw new IllegalArgumentException( "The only special names (enclosed in '[]') allowed are " + - "id, all, none"); + "id, all, document, none"); } } @@ -101,6 +102,8 @@ public class FieldSetRepo { return NoFields.NAME; } else if (fieldSet instanceof DocIdOnly) { return DocIdOnly.NAME; + } else if (fieldSet instanceof DocumentOnly) { + return DocumentOnly.NAME; } else { throw new IllegalArgumentException("Unknown field set type " + fieldSet); } @@ -112,6 +115,18 @@ public class FieldSetRepo { * fieldset. */ public void copyFields(Document source, Document target, FieldSet fieldSet) { + if (fieldSet instanceof DocumentOnly) { + var actual = source.getDataType().fieldSet(DocumentOnly.NAME); + if (actual != null) { + for (Iterator<Map.Entry<Field, FieldValue>> i = source.iterator(); i.hasNext();) { + Map.Entry<Field, FieldValue> v = i.next(); + if (actual.contains(v.getKey())) { + target.setFieldValue(v.getKey(), v.getValue()); + } + } + return; + } + } for (Iterator<Map.Entry<Field, FieldValue>> i = source.iterator(); i.hasNext();) { Map.Entry<Field, FieldValue> v = i.next(); @@ -126,6 +141,21 @@ public class FieldSetRepo { */ public void stripFields(Document target, FieldSet fieldSet) { List<Field> toStrip = new ArrayList<>(); + if (fieldSet instanceof DocumentOnly) { + var actual = target.getDataType().fieldSet(DocumentOnly.NAME); + if (actual != null) { + for (Iterator<Map.Entry<Field, FieldValue>> i = target.iterator(); i.hasNext();) { + Map.Entry<Field, FieldValue> v = i.next(); + if (! actual.contains(v.getKey())) { + toStrip.add(v.getKey()); + } + } + for (Field f : toStrip) { + target.removeFieldValue(f); + } + return; + } + } for (Iterator<Map.Entry<Field, FieldValue>> i = target.iterator(); i.hasNext();) { Map.Entry<Field, FieldValue> v = i.next(); diff --git a/document/src/test/java/com/yahoo/document/DocumentTestCaseBase.java b/document/src/test/java/com/yahoo/document/DocumentTestCaseBase.java index bf9960c3ca3..871e54ca46c 100644 --- a/document/src/test/java/com/yahoo/document/DocumentTestCaseBase.java +++ b/document/src/test/java/com/yahoo/document/DocumentTestCaseBase.java @@ -5,6 +5,8 @@ import com.yahoo.document.datatypes.FloatFieldValue; import com.yahoo.document.datatypes.Raw; import java.nio.ByteBuffer; +import java.util.List; +import java.util.Map; import static org.junit.Assert.assertNotNull; @@ -61,6 +63,8 @@ public class DocumentTestCaseBase { stringField = testDocType.getField("stringattr"); minField = testDocType.getField("Minattr"); + testDocType.addFieldSets(Map.of("[document]", List.of("stringattr", "intattr"))); + docMan.registerDocumentType(testDocType); } diff --git a/document/src/test/java/com/yahoo/document/fieldset/FieldSetTestCase.java b/document/src/test/java/com/yahoo/document/fieldset/FieldSetTestCase.java index e1e93adfb6d..b6dff63ac7b 100644 --- a/document/src/test/java/com/yahoo/document/fieldset/FieldSetTestCase.java +++ b/document/src/test/java/com/yahoo/document/fieldset/FieldSetTestCase.java @@ -23,6 +23,7 @@ public class FieldSetTestCase extends DocumentTestCaseBase { @Test public void testClone() throws Exception { assertTrue(new AllFields().clone() instanceof AllFields); + assertTrue(new DocumentOnly().clone() instanceof DocumentOnly); assertTrue(new NoFields().clone() instanceof NoFields); assertTrue(new DocIdOnly().clone() instanceof DocIdOnly); } @@ -32,6 +33,7 @@ public class FieldSetTestCase extends DocumentTestCaseBase { FieldSetRepo repo = new FieldSetRepo(); assertTrue(repo.parse(docMan, AllFields.NAME) instanceof AllFields); + assertTrue(repo.parse(docMan, DocumentOnly.NAME) instanceof DocumentOnly); assertTrue(repo.parse(docMan, NoFields.NAME) instanceof NoFields); assertTrue(repo.parse(docMan, DocIdOnly.NAME) instanceof DocIdOnly); @@ -72,21 +74,30 @@ public class FieldSetTestCase extends DocumentTestCaseBase { assertTrue(intAttr.contains(new DocIdOnly())); assertTrue(intAttr.contains(new NoFields())); assertFalse(intAttr.contains(new AllFields())); + assertFalse(intAttr.contains(new DocumentOnly())); assertFalse(new NoFields().contains(intAttr)); assertFalse(new NoFields().contains(new AllFields())); assertFalse(new NoFields().contains(new DocIdOnly())); + assertFalse(new NoFields().contains(new DocumentOnly())); assertTrue(new AllFields().contains(intAttr)); assertTrue(new AllFields().contains(rawAttr)); assertTrue(new AllFields().contains(new DocIdOnly())); + assertTrue(new AllFields().contains(new DocumentOnly())); assertTrue(new AllFields().contains(new NoFields())); assertTrue(new AllFields().contains(new AllFields())); assertTrue(new DocIdOnly().contains(new NoFields())); assertTrue(new DocIdOnly().contains(new DocIdOnly())); + assertFalse(new DocIdOnly().contains(new DocumentOnly())); assertFalse(new DocIdOnly().contains(intAttr)); + assertTrue(new DocumentOnly().contains(new NoFields())); + assertTrue(new DocumentOnly().contains(new DocIdOnly())); + assertTrue(new DocumentOnly().contains(new DocumentOnly())); + assertFalse(new DocumentOnly().contains(intAttr)); + assertContains("testdoc:rawattr,intattr", "testdoc:intattr"); assertNotContains("testdoc:intattr", "testdoc:rawattr,intattr"); assertContains("testdoc:intattr,rawattr", "testdoc:rawattr,intattr"); @@ -124,6 +135,7 @@ public class FieldSetTestCase extends DocumentTestCaseBase { doc.removeFieldValue("rawattr"); assertEquals("floatattr:3.56,stringattr:tjohei,intattr:50,byteattr:30", doCopyFields(doc, AllFields.NAME)); + assertEquals("stringattr:tjohei,intattr:50", doCopyFields(doc, DocumentOnly.NAME)); assertEquals("floatattr:3.56,byteattr:30", doCopyFields(doc, "testdoc:floatattr,byteattr")); } @@ -140,6 +152,7 @@ public class FieldSetTestCase extends DocumentTestCaseBase { doc.removeFieldValue("rawattr"); assertEquals("floatattr:3.56,stringattr:tjohei,intattr:50,byteattr:30", doStripFields(doc, AllFields.NAME)); + assertEquals("stringattr:tjohei,intattr:50", doStripFields(doc, DocumentOnly.NAME)); assertEquals("floatattr:3.56,byteattr:30", doStripFields(doc, "testdoc:floatattr,byteattr")); } @@ -150,6 +163,7 @@ public class FieldSetTestCase extends DocumentTestCaseBase { AllFields.NAME, NoFields.NAME, DocIdOnly.NAME, + DocumentOnly.NAME, "testdoc:rawattr", "testdoc:rawattr,intattr" }; diff --git a/document/src/tests/fieldsettest.cpp b/document/src/tests/fieldsettest.cpp index b4ee7713613..9f00fdd8c0d 100644 --- a/document/src/tests/fieldsettest.cpp +++ b/document/src/tests/fieldsettest.cpp @@ -28,6 +28,7 @@ TEST_F(FieldSetTest, testParsing) const DocumentTypeRepo& docRepo = testDocMan.getTypeRepo(); (void) dynamic_cast<AllFields&>(*FieldSetRepo::parse(docRepo, AllFields::NAME)); + (void) dynamic_cast<DocumentOnly&>(*FieldSetRepo::parse(docRepo, DocumentOnly::NAME)); (void) dynamic_cast<NoFields&>(*FieldSetRepo::parse(docRepo, NoFields::NAME)); (void) dynamic_cast<DocIdOnly&>(*FieldSetRepo::parse(docRepo, DocIdOnly::NAME)); @@ -73,6 +74,7 @@ TEST_F(FieldSetTest, testContains) NoFields none; AllFields all; + DocumentOnly doconly; DocIdOnly id; EXPECT_EQ(false, headerField.contains(type.getField("headerlongval"))); @@ -87,6 +89,8 @@ TEST_F(FieldSetTest, testContains) EXPECT_EQ(true, all.contains(id)); EXPECT_EQ(false, none.contains(id)); EXPECT_EQ(true, id.contains(none)); + EXPECT_EQ(true, doconly.contains(none)); + EXPECT_EQ(true, doconly.contains(id)); EXPECT_EQ(true, checkContains(repo, "testdoctype1:content,headerval", @@ -181,6 +185,9 @@ TEST_F(FieldSetTest, testCopyDocumentFields) "headerval: 5678\n" "hstringval: hello fantastic world\n"), doCopyFields(*src, repo, AllFields::NAME)); + EXPECT_EQ(std::string("headerval: 5678\n" + "hstringval: hello fantastic world\n"), + doCopyFields(*src, repo, DocumentOnly::NAME)); EXPECT_EQ(std::string("content: megafoo megabar\n" "hstringval: hello fantastic world\n"), doCopyFields(*src, repo, "testdoctype1:hstringval,content")); @@ -219,9 +226,19 @@ TEST_F(FieldSetTest, testDocumentSubsetCopy) EXPECT_EQ(doCopyFields(*src, repo, AllFields::NAME), stringifyFields(*doc)); } + { + Document::UP doc(FieldSet::createDocumentSubsetCopy(*src, DocumentOnly())); + // Test that document id and type are copied correctly. + EXPECT_TRUE(doc.get()); + EXPECT_EQ(src->getId(), doc->getId()); + EXPECT_EQ(src->getType(), doc->getType()); + EXPECT_EQ(doCopyFields(*src, repo, DocumentOnly::NAME), + stringifyFields(*doc)); + } const char* fieldSets[] = { AllFields::NAME, + DocumentOnly::NAME, NoFields::NAME, "testdoctype1:hstringval,content" }; @@ -239,6 +256,7 @@ TEST_F(FieldSetTest, testSerialize) const char* fieldSets[] = { AllFields::NAME, NoFields::NAME, + DocumentOnly::NAME, DocIdOnly::NAME, "testdoctype1:content", "testdoctype1:content,hstringval" @@ -264,6 +282,9 @@ TEST_F(FieldSetTest, testStripFields) "headerval: 5678\n" "hstringval: hello fantastic world\n"), doStripFields(*src, repo, AllFields::NAME)); + EXPECT_EQ(std::string("headerval: 5678\n" + "hstringval: hello fantastic world\n"), + doStripFields(*src, repo, DocumentOnly::NAME)); EXPECT_EQ(std::string("content: megafoo megabar\n" "hstringval: hello fantastic world\n"), doStripFields(*src, repo, "testdoctype1:hstringval,content")); diff --git a/document/src/vespa/document/base/testdocrepo.cpp b/document/src/vespa/document/base/testdocrepo.cpp index fed14eef792..c1930de8551 100644 --- a/document/src/vespa/document/base/testdocrepo.cpp +++ b/document/src/vespa/document/base/testdocrepo.cpp @@ -27,6 +27,7 @@ DocumenttypesConfig TestDocRepo::getDefaultConfig() { const int mystruct_id = -2092985851; const int structarray_id = 759956026; config_builder::DocumenttypesConfigBuilderHelper builder; + ::config::StringVector documentfields = { "headerval", "hstringval", "title" }; builder.document(type1_id, "testdoctype1", Struct("testdoctype1.header") .addField("headerval", DataType::T_INT) @@ -55,7 +56,9 @@ DocumenttypesConfig TestDocRepo::getDefaultConfig() { .addTensorField("sparse_tensor", "tensor(x{})") .addTensorField("sparse_xy_tensor", "tensor(x{},y{})") .addTensorField("sparse_float_tensor", "tensor<float>(x{})") - .addTensorField("dense_tensor", "tensor(x[2])")); + .addTensorField("dense_tensor", "tensor(x[2])")) + .doc_type.fieldsets["[document]"].fields.swap(documentfields); + builder.document(type2_id, "testdoctype2", Struct("testdoctype2.header") .addField("onlyinchild", DataType::T_INT), diff --git a/documentapi/src/main/java/com/yahoo/documentapi/VisitorParameters.java b/documentapi/src/main/java/com/yahoo/documentapi/VisitorParameters.java index 605bb95ba24..a9dfe0128e0 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/VisitorParameters.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/VisitorParameters.java @@ -4,6 +4,7 @@ package com.yahoo.documentapi; import com.yahoo.document.BucketId; import com.yahoo.document.FixedBucketSpaces; import com.yahoo.document.fieldset.AllFields; +import com.yahoo.document.fieldset.DocumentOnly; import com.yahoo.documentapi.messagebus.loadtypes.LoadType; import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol; import com.yahoo.messagebus.ThrottlePolicy; @@ -30,6 +31,7 @@ public class VisitorParameters extends Parameters { private long fromTimestamp = 0; private long toTimestamp = 0; boolean visitRemoves = false; + // TODO Vespa 8: change to DocumentOnly.NAME; private String fieldSet = AllFields.NAME; boolean visitInconsistentBuckets = false; private ProgressToken resumeToken = null; diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java index 259b598dbf5..a39f8e81757 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java @@ -7,6 +7,7 @@ import com.yahoo.document.DocumentPut; import com.yahoo.document.DocumentRemove; import com.yahoo.document.DocumentUpdate; import com.yahoo.document.fieldset.AllFields; +import com.yahoo.document.fieldset.DocumentOnly; import com.yahoo.documentapi.AsyncParameters; import com.yahoo.documentapi.AsyncSession; import com.yahoo.documentapi.DocumentIdResponse; @@ -127,6 +128,7 @@ public class MessageBusAsyncSession implements MessageBusSession, AsyncSession { @Override public Result get(DocumentId id, DocumentOperationParameters parameters) { + // TODO Vespa 8: change to DocumentOnly.NAME GetDocumentMessage msg = new GetDocumentMessage(id, parameters.fieldSet().orElse(AllFields.NAME)); msg.setPriority(parameters.priority().orElse(DocumentProtocol.Priority.NORMAL_1)); return send(msg, parameters); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusSyncSession.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusSyncSession.java index 4bc96ed1d22..5537d122e16 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusSyncSession.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusSyncSession.java @@ -7,6 +7,7 @@ import com.yahoo.document.DocumentPut; import com.yahoo.document.DocumentRemove; import com.yahoo.document.DocumentUpdate; import com.yahoo.document.fieldset.AllFields; +import com.yahoo.document.fieldset.DocumentOnly; import com.yahoo.documentapi.AsyncParameters; import com.yahoo.documentapi.DocumentAccessException; import com.yahoo.documentapi.DocumentOperationParameters; @@ -152,6 +153,7 @@ public class MessageBusSyncSession implements MessageBusSession, SyncSession, Re @Override public Document get(DocumentId id, DocumentOperationParameters parameters, Duration timeout) { + // TODO Vespa 8: change to DocumentOnly.NAME GetDocumentMessage msg = new GetDocumentMessage(id, parameters.fieldSet().orElse(AllFields.NAME)); msg.setPriority(parameters.priority().orElse(DocumentProtocol.Priority.NORMAL_1)); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/CreateVisitorMessage.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/CreateVisitorMessage.java index cae0587ae39..097632f609f 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/CreateVisitorMessage.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/CreateVisitorMessage.java @@ -4,6 +4,7 @@ package com.yahoo.documentapi.messagebus.protocol; import com.yahoo.document.BucketId; import com.yahoo.document.FixedBucketSpaces; import com.yahoo.document.fieldset.AllFields; +import com.yahoo.document.fieldset.DocumentOnly; import java.util.ArrayList; import java.util.Iterator; @@ -24,6 +25,7 @@ public class CreateVisitorMessage extends DocumentMessage { private long fromTime = 0; private long toTime = 0; private boolean visitRemoves = false; + // TODO Vespa 8: change to DocumentOnly.NAME private String fieldSet = AllFields.NAME; private boolean visitInconsistentBuckets = false; private Map<String, byte[]> params = new TreeMap<>(); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/GetDocumentMessage.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/GetDocumentMessage.java index 669c355b2d8..0f6e738cb86 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/GetDocumentMessage.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/GetDocumentMessage.java @@ -3,6 +3,7 @@ package com.yahoo.documentapi.messagebus.protocol; import com.yahoo.document.DocumentId; import com.yahoo.document.fieldset.AllFields; +import com.yahoo.document.fieldset.DocumentOnly; import java.util.Arrays; @@ -11,6 +12,7 @@ import java.util.Arrays; */ public class GetDocumentMessage extends DocumentMessage { + // TODO Vespa 8: change to DocumentOnly.NAME final static String DEFAULT_FIELD_SET = AllFields.NAME; private DocumentId documentId = null; private String fieldSet = DEFAULT_FIELD_SET; diff --git a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp index 512e12bec71..d1a9ffc5b49 100644 --- a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp +++ b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp @@ -21,6 +21,8 @@ #include <vespa/eval/eval/check_type.h> #include <vespa/vespalib/stllike/hash_set.h> #include <vespa/vespalib/util/approx.h> +#include <vespa/vespalib/util/size_literals.h> +#include <vespa/vespalib/util/malloc_mmap_guard.h> #include <limits> double vespalib_eval_ldexp(double a, double b) { return std::ldexp(a, b); } @@ -728,6 +730,8 @@ LLVMWrapper::compile(llvm::raw_ostream * dumpStream) // Set relocation model to silence valgrind on CentOS 8 / aarch64 _engine.reset(llvm::EngineBuilder(std::move(_module)).setOptLevel(llvm::CodeGenOpt::Aggressive).setRelocationModel(llvm::Reloc::Static).create()); assert(_engine && "llvm jit not available for your platform"); + + MallocMmapGuard largeAllocsAsMMap(1_Mi); _engine->finalizeObject(); } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/UriPattern.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/UriPattern.java index 4d9a843b8fb..1ddf91aee2b 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/UriPattern.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/UriPattern.java @@ -29,7 +29,6 @@ import java.util.regex.Pattern; */ public class UriPattern implements Comparable<UriPattern> { - public static final int DEFAULT_PRIORITY = 0; private static final Pattern PATTERN = Pattern.compile("([^:]+)://([^:/]+)(:((\\*)|([0-9]+)))?/(.*)", Pattern.UNICODE_CASE | Pattern.CANON_EQ); private final String pattern; @@ -38,9 +37,6 @@ public class UriPattern implements Comparable<UriPattern> { private final int port; private final GlobPattern path; - // TODO Vespa 8 jonmv remove - private final int priority; - /** * <p>Creates a new instance of this class that represents the given pattern string, with a priority of <code>0</code>. * The input string must be on the form <code><scheme>://<host>[:<port>]<path></code>, where @@ -59,31 +55,6 @@ public class UriPattern implements Comparable<UriPattern> { port = parseOrZero(matcher.group(4)); path = GlobPattern.compile(nonNullOrWildcard(matcher.group(7))); pattern = scheme + "://" + host + ":" + (port > 0 ? port : "*") + "/" + path; - this.priority = DEFAULT_PRIORITY; - } - - /** - * <p>Creates a new instance of this class that represents the given pattern string, with the given priority. The - * input string must be on the form <code><scheme>://<host>[:<port>]<path></code>, where - * '*' can be used as a wildcard character at any position.</p> - * - * @deprecated Use {@link #UriPattern(String)} and let's avoid another complication here. - * @param uri The pattern to parse. - * @param priority The priority of this pattern. - * @throws IllegalArgumentException If the pattern could not be parsed. - */ - @Deprecated(forRemoval = true, since = "7") - public UriPattern(String uri, int priority) { - Matcher matcher = PATTERN.matcher(uri); - if (!matcher.find()) { - throw new IllegalArgumentException(uri); - } - scheme = GlobPattern.compile(normalizeScheme(nonNullOrWildcard(matcher.group(1)))); - host = GlobPattern.compile(nonNullOrWildcard(matcher.group(2))); - port = parseOrZero(matcher.group(4)); - path = GlobPattern.compile(nonNullOrWildcard(matcher.group(7))); - pattern = scheme + "://" + host + ":" + (port > 0 ? port : "*") + "/" + path; - this.priority = priority; } /** @@ -135,10 +106,6 @@ public class UriPattern implements Comparable<UriPattern> { @Override public int compareTo(UriPattern rhs) { int cmp; - cmp = rhs.priority - priority; - if (cmp != 0) { - return cmp; - } cmp = scheme.compareTo(rhs.scheme); if (cmp != 0) { return cmp; diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/application/UriPatternTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/application/UriPatternTestCase.java index f7488aa392a..32327ace4f4 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/application/UriPatternTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/application/UriPatternTestCase.java @@ -147,20 +147,6 @@ public class UriPatternTestCase { } @Test - @SuppressWarnings("removal") - public void requireThatPrioritiesAreOrderedDescending() { - assertCompareLt(new UriPattern("scheme://host:69/path", 1), - new UriPattern("scheme://host:69/path", 0)); - } - - @Test - @SuppressWarnings("removal") - public void requireThatPriorityOrdersBeforeScheme() { - assertCompareLt(new UriPattern("*://host:69/path", 1), - new UriPattern("scheme://host:69/path", 0)); - } - - @Test public void requireThatSchemesAreOrdered() { assertCompareLt("b://host:69/path", "a://host:69/path"); diff --git a/screwdriver.yaml b/screwdriver.yaml index 14d9902d335..57b7a3c05ae 100644 --- a/screwdriver.yaml +++ b/screwdriver.yaml @@ -202,6 +202,7 @@ jobs: - SAMPLE_APPS_DEPLOY_KEY - VESPA_DEPLOY_KEY - DOCKER_HUB_DEPLOY_KEY + - GHCR_DEPLOY_KEY environment: GIT_SSH_COMMAND: "ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no" diff --git a/searchcore/src/vespa/searchcore/config/proton.def b/searchcore/src/vespa/searchcore/config/proton.def index 81f8b49eb49..808535924f1 100644 --- a/searchcore/src/vespa/searchcore/config/proton.def +++ b/searchcore/src/vespa/searchcore/config/proton.def @@ -446,7 +446,7 @@ initialize.threads int default = 0 ## Portion of max address space used in components in attribute vectors ## before put and update operations in feed is blocked. -writefilter.attribute.address_space_limit double default = 0.9 +writefilter.attribute.address_space_limit double default = 0.92 ## Portion of physical memory that can be resident memory in anonymous mapping ## by the proton process before put and update portion of feed is blocked. diff --git a/searchlib/src/main/java/com/yahoo/searchlib/expression/ConstantNode.java b/searchlib/src/main/java/com/yahoo/searchlib/expression/ConstantNode.java index 2bab10d1830..e26900ac77e 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/expression/ConstantNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/expression/ConstantNode.java @@ -16,9 +16,7 @@ public class ConstantNode extends ExpressionNode { public static final int classId = registerClass(0x4000 + 49, ConstantNode.class); private ResultNode value = null; - public ConstantNode() { - - } + public ConstantNode() {} public ConstantNode(ResultNode value) { this.value = value; diff --git a/searchlib/src/main/javacc/RankingExpressionParser.jj b/searchlib/src/main/javacc/RankingExpressionParser.jj index 0d46ab4ddb6..865820320d8 100755 --- a/searchlib/src/main/javacc/RankingExpressionParser.jj +++ b/searchlib/src/main/javacc/RankingExpressionParser.jj @@ -1032,11 +1032,13 @@ Slice.DimensionValue dimensionValue(Optional dimensionName) : { value = expression() { - if (value instanceof ReferenceNode && ((ReferenceNode)value).reference().isIdentifier()) - return new Slice.DimensionValue(dimensionName, ((ReferenceNode)value).reference().name()); - else - return new Slice.DimensionValue(dimensionName, TensorFunctionNode.wrapScalar(value)); -} + if (value instanceof ReferenceNode && ((ReferenceNode)value).reference().isIdentifier()) // A label + return new Slice.DimensionValue(dimensionName, ((ReferenceNode)value).reference().name()); + else if (value instanceof ConstantNode && ((ConstantNode)value).getValue() instanceof StringValue) // A quoted label + return new Slice.DimensionValue(dimensionName, ((StringValue)((ConstantNode)value).getValue()).asString()); + else + return new Slice.DimensionValue(dimensionName, TensorFunctionNode.wrapScalar(value)); + } } String label() : diff --git a/searchlib/src/tests/aggregator/perdocexpr.cpp b/searchlib/src/tests/aggregator/perdocexpr.cpp index e0071e28428..291114ffa90 100644 --- a/searchlib/src/tests/aggregator/perdocexpr.cpp +++ b/searchlib/src/tests/aggregator/perdocexpr.cpp @@ -1605,7 +1605,7 @@ AttributeGuard createInt8Attribute() { } AttributeGuard createBoolAttribute() { - SingleBoolAttribute *selectAttr1(new SingleBoolAttribute("selectAttr1", search::GrowStrategy())); + SingleBoolAttribute *selectAttr1(new SingleBoolAttribute("selectAttr1", search::GrowStrategy(), false)); DocId docId(0); selectAttr1->addDoc(docId); selectAttr1->setBit(docId, true); diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index d7d0bfd4012..73dd7d2f776 100644 --- a/searchlib/src/tests/attribute/attribute_test.cpp +++ b/searchlib/src/tests/attribute/attribute_test.cpp @@ -2292,10 +2292,13 @@ AttributeTest::test_paged_attribute(const vespalib::string& name, const vespalib lid_mapping_size = 17000; sv_maxlid = 1500; } + if (cfg.basicType() == search::attribute::BasicType::Type::BOOL) { + lid_mapping_size = rounded_size * 8 + 100; + } LOG(info, "test_paged_attribute '%s'", name.c_str()); auto av = createAttribute(name, cfg); auto v = std::dynamic_pointer_cast<IntegerAttribute>(av); - ASSERT_TRUE(v); + ASSERT_TRUE(v || (!cfg.collectionType().isMultiValue() && !cfg.fastSearch())); auto size1 = stat_size(swapfile); // Grow mapping from lid to value or multivalue index addClearedDocs(av, lid_mapping_size); @@ -2355,6 +2358,9 @@ AttributeTest::test_paged_attributes() cfg4.setPaged(true); cfg4.setFastSearch(true); EXPECT_EQUAL(7, test_paged_attribute("fs-int-mv-paged", basedir + "/3.fs-int-mv-paged/swapfile", cfg4)); + search::attribute::Config cfg5(BasicType::BOOL, CollectionType::SINGLE); + cfg5.setPaged(true); + EXPECT_EQUAL(1, test_paged_attribute("std-bool-sv-paged", basedir + "/4.std-bool-sv-paged/swapfile", cfg5)); vespalib::alloc::MmapFileAllocatorFactory::instance().setup(""); vespalib::rmdir(basedir, true); } diff --git a/searchlib/src/tests/attribute/searchcontext/searchcontext_test.cpp b/searchlib/src/tests/attribute/searchcontext/searchcontext_test.cpp index de54386e4af..29f7b5c0276 100644 --- a/searchlib/src/tests/attribute/searchcontext/searchcontext_test.cpp +++ b/searchlib/src/tests/attribute/searchcontext/searchcontext_test.cpp @@ -1830,7 +1830,7 @@ private: public: BoolAttributeFixture(const SimpleResult& true_docs, uint32_t num_docs) - : _attr("bool_attr", search::GrowStrategy()) + : _attr("bool_attr", search::GrowStrategy(), false) { _attr.addDocs(num_docs); for (uint32_t i = 0; i < true_docs.getHitCount(); ++i) { diff --git a/searchlib/src/tests/common/bitvector/bitvector_test.cpp b/searchlib/src/tests/common/bitvector/bitvector_test.cpp index faeebf9984a..a9850756ac6 100644 --- a/searchlib/src/tests/common/bitvector/bitvector_test.cpp +++ b/searchlib/src/tests/common/bitvector/bitvector_test.cpp @@ -8,11 +8,16 @@ #include <vespa/searchlib/common/bitvectoriterator.h> #include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vespa/searchlib/fef/termfieldmatchdataarray.h> +#include <vespa/vespalib/test/memory_allocator_observer.h> #include <vespa/vespalib/util/rand48.h> #include <algorithm> using namespace search; +using vespalib::alloc::Alloc; +using vespalib::alloc::test::MemoryAllocatorObserver; +using AllocStats = MemoryAllocatorObserver::Stats; + namespace { std::string @@ -653,5 +658,26 @@ TEST("requireThatGrowWorks") g.trimHoldLists(2); } +TEST("require that growable bit vectors keeps memory allocator") +{ + AllocStats stats; + auto memory_allocator = std::make_unique<MemoryAllocatorObserver>(stats); + Alloc init_alloc = Alloc::alloc_with_allocator(memory_allocator.get()); + vespalib::GenerationHolder g; + GrowableBitVector v(200, 200, g, &init_alloc); + EXPECT_EQUAL(AllocStats(1, 0), stats); + v.resize(1); + EXPECT_EQUAL(AllocStats(2, 1), stats); + v.reserve(2000); + EXPECT_EQUAL(AllocStats(3, 1), stats); + v.extend(4000); + EXPECT_EQUAL(AllocStats(4, 1), stats); + v.shrink(200); + EXPECT_EQUAL(AllocStats(4, 1), stats); + v.resize(1); + EXPECT_EQUAL(AllocStats(5, 2), stats); + g.transferHoldLists(1); + g.trimHoldLists(2); +} TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/queryeval/queryeval.cpp b/searchlib/src/tests/queryeval/queryeval.cpp index cdfe1cfca88..cc9e90bb761 100644 --- a/searchlib/src/tests/queryeval/queryeval.cpp +++ b/searchlib/src/tests/queryeval/queryeval.cpp @@ -339,7 +339,7 @@ class DummySingleValueBitNumericAttributeBlueprint : public SimpleLeafBlueprint public: DummySingleValueBitNumericAttributeBlueprint(const SimpleResult & result) : SimpleLeafBlueprint(FieldSpecBaseList()), - _a("a", search::GrowStrategy()), + _a("a", search::GrowStrategy(), false), _sc(), _tfmd() { diff --git a/searchlib/src/vespa/searchlib/attribute/createsinglefastsearch.cpp b/searchlib/src/vespa/searchlib/attribute/createsinglefastsearch.cpp index 71eb88d4e52..66e6e7ccd4d 100644 --- a/searchlib/src/vespa/searchlib/attribute/createsinglefastsearch.cpp +++ b/searchlib/src/vespa/searchlib/attribute/createsinglefastsearch.cpp @@ -24,7 +24,7 @@ AttributeFactory::createSingleFastSearch(stringref name, const Config & info) assert(info.fastSearch()); switch(info.basicType().type()) { case BasicType::BOOL: - return std::make_shared<SingleBoolAttribute>(name, info.getGrowStrategy()); + return std::make_shared<SingleBoolAttribute>(name, info.getGrowStrategy(), info.paged()); case BasicType::UINT2: case BasicType::UINT4: break; diff --git a/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp b/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp index 3294ecd8dd2..fe2b0c9f989 100644 --- a/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp +++ b/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp @@ -21,7 +21,7 @@ AttributeFactory::createSingleStd(stringref name, const Config & info) assert(info.collectionType().type() == attribute::CollectionType::SINGLE); switch(info.basicType().type()) { case BasicType::BOOL: - return std::make_shared<SingleBoolAttribute>(name, info.getGrowStrategy()); + return std::make_shared<SingleBoolAttribute>(name, info.getGrowStrategy(), info.paged()); case BasicType::UINT2: return std::make_shared<SingleValueSemiNibbleNumericAttribute>(name, info.getGrowStrategy()); case BasicType::UINT4: diff --git a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp index 3fe7d147c1d..474265c914b 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp @@ -17,9 +17,10 @@ namespace search { using attribute::Config; SingleBoolAttribute:: -SingleBoolAttribute(const vespalib::string &baseFileName, const GrowStrategy & grow) - : IntegerAttributeTemplate<int8_t>(baseFileName, Config(BasicType::BOOL, CollectionType::SINGLE).setGrowStrategy(grow), BasicType::BOOL), - _bv(0, 0, getGenerationHolder()) +SingleBoolAttribute(const vespalib::string &baseFileName, const GrowStrategy & grow, bool paged) + : IntegerAttributeTemplate<int8_t>(baseFileName, Config(BasicType::BOOL, CollectionType::SINGLE).setGrowStrategy(grow).setPaged(paged), BasicType::BOOL), + _init_alloc(get_initial_alloc()), + _bv(0, 0, getGenerationHolder(), get_memory_allocator() ? &_init_alloc : nullptr) { } diff --git a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.h b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.h index 78c07719271..469bd54fa55 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.h @@ -15,7 +15,7 @@ namespace search { class SingleBoolAttribute final : public IntegerAttributeTemplate<int8_t> { public: - SingleBoolAttribute(const vespalib::string & baseFileName, const search::GrowStrategy & grow); + SingleBoolAttribute(const vespalib::string & baseFileName, const search::GrowStrategy & grow, bool paged); ~SingleBoolAttribute() override; void onCommit() override; @@ -108,6 +108,7 @@ private: int8_t getFast(DocId doc) const { return _bv.testBit(doc) ? 1 : 0; } + vespalib::alloc::Alloc _init_alloc; GrowableBitVector _bv; }; diff --git a/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp b/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp index d428ce52953..ab56317db6f 100644 --- a/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp @@ -59,10 +59,10 @@ AllocatedBitVector::AllocatedBitVector(Index numberOfElements, Alloc buffer, siz { } -AllocatedBitVector::AllocatedBitVector(Index numberOfElements, Index capacityBits, const void * rhsBuf, size_t rhsSize) : +AllocatedBitVector::AllocatedBitVector(Index numberOfElements, Index capacityBits, const void * rhsBuf, size_t rhsSize, const Alloc* init_alloc) : BitVector(), _capacityBits(capacityBits), - _alloc(allocatePaddedAndAligned(0, numberOfElements, capacityBits)) + _alloc(allocatePaddedAndAligned(0, numberOfElements, capacityBits, init_alloc)) { _capacityBits = computeCapacity(_capacityBits, _alloc.size()); init(_alloc.get(), 0, numberOfElements); @@ -104,17 +104,9 @@ AllocatedBitVector::AllocatedBitVector(const BitVector & rhs, std::pair<Index, I AllocatedBitVector::~AllocatedBitVector() = default; void -AllocatedBitVector::cleanup() -{ - init(nullptr, 0, 0); - Alloc().swap(_alloc); - _capacityBits = 0; -} - -void AllocatedBitVector::resize(Index newLength) { - _alloc = allocatePaddedAndAligned(newLength); + _alloc = allocatePaddedAndAligned(0, newLength, newLength, &_alloc); _capacityBits = computeCapacity(newLength, _alloc.size()); init(_alloc.get(), 0, newLength); clear(); @@ -145,7 +137,7 @@ AllocatedBitVector::grow(Index newSize, Index newCapacity) assert(newCapacity >= newSize); GenerationHeldBase::UP ret; if (newCapacity != capacity()) { - AllocatedBitVector tbv(newSize, newCapacity, _alloc.get(), size()); + AllocatedBitVector tbv(newSize, newCapacity, _alloc.get(), size(), &_alloc); if (newSize > size()) { tbv.clearBitAndMaintainCount(size()); // Clear old guard bit. } diff --git a/searchlib/src/vespa/searchlib/common/allocatedbitvector.h b/searchlib/src/vespa/searchlib/common/allocatedbitvector.h index c57ca93ac02..2223c7b4701 100644 --- a/searchlib/src/vespa/searchlib/common/allocatedbitvector.h +++ b/searchlib/src/vespa/searchlib/common/allocatedbitvector.h @@ -35,7 +35,7 @@ public: * Creates a new bitvector with size of numberOfElements bits and at least a capacity of capacity. * Copies what it can from the original vector. This is used for extending vector. */ - AllocatedBitVector(Index numberOfElements, Index capacity, const void * rhsBuf, size_t rhsSize); + AllocatedBitVector(Index numberOfElements, Index capacity, const void * rhsBuf, size_t rhsSize, const Alloc* init_alloc); AllocatedBitVector(const BitVector &other); AllocatedBitVector(const AllocatedBitVector &other); @@ -74,12 +74,6 @@ private: } AllocatedBitVector(const BitVector &other, std::pair<Index, Index> size_capacity); - - /** - * Prepare for potential reuse where new value might be filled in by - * Read method. - */ - void cleanup(); }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/common/bitvector.cpp b/searchlib/src/vespa/searchlib/common/bitvector.cpp index a3a1b9bed66..d2e6d9027ea 100644 --- a/searchlib/src/vespa/searchlib/common/bitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/bitvector.cpp @@ -47,13 +47,13 @@ using vespalib::GenerationHeldAlloc; using vespalib::GenerationHolder; Alloc -BitVector::allocatePaddedAndAligned(Index start, Index end, Index capacity) +BitVector::allocatePaddedAndAligned(Index start, Index end, Index capacity, const Alloc* init_alloc) { assert(capacity >= end); uint32_t words = numActiveWords(start, capacity); words += (-words & 15); // Pad to 64 byte alignment const size_t sz(words * sizeof(Word)); - Alloc alloc = Alloc::alloc(sz, MMAP_LIMIT); + Alloc alloc = (init_alloc != nullptr) ? init_alloc->create(sz) : Alloc::alloc(sz, MMAP_LIMIT); assert(alloc.size()/sizeof(Word) >= words); // Clear padding size_t usedBytes = numBytes(end - start); diff --git a/searchlib/src/vespa/searchlib/common/bitvector.h b/searchlib/src/vespa/searchlib/common/bitvector.h index 184669829cb..c1d447047d1 100644 --- a/searchlib/src/vespa/searchlib/common/bitvector.h +++ b/searchlib/src/vespa/searchlib/common/bitvector.h @@ -281,7 +281,7 @@ protected: static Alloc allocatePaddedAndAligned(Index start, Index end) { return allocatePaddedAndAligned(start, end, end); } - static Alloc allocatePaddedAndAligned(Index start, Index end, Index capacity); + static Alloc allocatePaddedAndAligned(Index start, Index end, Index capacity, const Alloc* init_alloc = nullptr); private: friend PartialBitVector; diff --git a/searchlib/src/vespa/searchlib/common/growablebitvector.cpp b/searchlib/src/vespa/searchlib/common/growablebitvector.cpp index 177266c5618..7c29945292e 100644 --- a/searchlib/src/vespa/searchlib/common/growablebitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/growablebitvector.cpp @@ -10,8 +10,9 @@ using vespalib::GenerationHeldBase; using vespalib::GenerationHolder; GrowableBitVector::GrowableBitVector(Index newSize, Index newCapacity, - GenerationHolder &generationHolder) - : AllocatedBitVector(newSize, newCapacity, nullptr, 0), + GenerationHolder &generationHolder, + const Alloc* init_alloc) + : AllocatedBitVector(newSize, newCapacity, nullptr, 0, init_alloc), _generationHolder(generationHolder) { assert(newSize <= newCapacity); diff --git a/searchlib/src/vespa/searchlib/common/growablebitvector.h b/searchlib/src/vespa/searchlib/common/growablebitvector.h index 08c9fede8f2..71261e130b6 100644 --- a/searchlib/src/vespa/searchlib/common/growablebitvector.h +++ b/searchlib/src/vespa/searchlib/common/growablebitvector.h @@ -9,7 +9,7 @@ namespace search { class GrowableBitVector : public AllocatedBitVector { public: - GrowableBitVector(Index newSize, Index newCapacity, GenerationHolder &generationHolder); + GrowableBitVector(Index newSize, Index newCapacity, GenerationHolder &generationHolder, const Alloc* init_alloc = nullptr); /** Will return true if a a buffer is held */ bool reserve(Index newCapacity); diff --git a/storage/src/tests/distributor/CMakeLists.txt b/storage/src/tests/distributor/CMakeLists.txt index bee7650aebd..501f9a18c47 100644 --- a/storage/src/tests/distributor/CMakeLists.txt +++ b/storage/src/tests/distributor/CMakeLists.txt @@ -9,6 +9,7 @@ vespa_add_executable(storage_distributor_gtest_runner_app TEST bucketdbmetricupdatertest.cpp bucketgctimecalculatortest.cpp bucketstateoperationtest.cpp + distributor_bucket_space_repo_test.cpp distributor_bucket_space_test.cpp distributor_host_info_reporter_test.cpp distributor_message_sender_stub.cpp diff --git a/storage/src/tests/distributor/distributor_bucket_space_repo_test.cpp b/storage/src/tests/distributor/distributor_bucket_space_repo_test.cpp new file mode 100644 index 00000000000..151dcff3d10 --- /dev/null +++ b/storage/src/tests/distributor/distributor_bucket_space_repo_test.cpp @@ -0,0 +1,72 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/document/bucket/fixed_bucket_spaces.h> +#include <vespa/storage/distributor/distributor_bucket_space.h> +#include <vespa/storage/distributor/distributor_bucket_space_repo.h> +#include <vespa/vdslib/state/cluster_state_bundle.h> +#include <vespa/vdslib/state/clusterstate.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <memory> + +namespace storage::distributor { + +using document::FixedBucketSpaces; +using namespace ::testing; + +struct DistributorBucketSpaceRepoTest : Test { + DistributorBucketSpaceRepo _repo; + + DistributorBucketSpaceRepoTest() : _repo(123) {} +}; + +namespace { + +lib::ClusterStateBundle bundle_with_global_merges() { + auto global_state = std::make_shared<lib::ClusterState>("distributor:1 storage:2"); + auto default_state = std::make_shared<lib::ClusterState>("distributor:1 storage:2 .1.s:m"); + return lib::ClusterStateBundle(*global_state, {{FixedBucketSpaces::default_space(), default_state}, + {FixedBucketSpaces::global_space(), global_state}}); +} + +lib::ClusterStateBundle bundle_without_global_merges() { + auto global_state = std::make_shared<lib::ClusterState>("distributor:1 storage:2"); + auto default_state = std::make_shared<lib::ClusterState>("distributor:1 storage:2"); + return lib::ClusterStateBundle(*global_state, {{FixedBucketSpaces::default_space(), default_state}, + {FixedBucketSpaces::global_space(), global_state}}); +} + +} + +TEST_F(DistributorBucketSpaceRepoTest, bucket_spaces_are_initially_not_tagged_as_merge_inhibited) { + EXPECT_FALSE(_repo.get(FixedBucketSpaces::default_space()).merges_inhibited()); + EXPECT_FALSE(_repo.get(FixedBucketSpaces::global_space()).merges_inhibited()); +} + +TEST_F(DistributorBucketSpaceRepoTest, enabled_bundle_with_pending_global_merges_tags_default_space_as_merge_inhibited) { + _repo.enable_cluster_state_bundle(bundle_with_global_merges()); + EXPECT_TRUE(_repo.get(FixedBucketSpaces::default_space()).merges_inhibited()); + EXPECT_FALSE(_repo.get(FixedBucketSpaces::global_space()).merges_inhibited()); +} + +TEST_F(DistributorBucketSpaceRepoTest, enabled_bundle_without_pending_global_merges_unsets_merge_inhibition) { + _repo.enable_cluster_state_bundle(bundle_with_global_merges()); + _repo.enable_cluster_state_bundle(bundle_without_global_merges()); + EXPECT_FALSE(_repo.get(FixedBucketSpaces::default_space()).merges_inhibited()); + EXPECT_FALSE(_repo.get(FixedBucketSpaces::global_space()).merges_inhibited()); +} + +TEST_F(DistributorBucketSpaceRepoTest, pending_bundle_with_pending_global_merges_tags_default_space_as_merge_inhibited) { + _repo.enable_cluster_state_bundle(bundle_without_global_merges()); + _repo.set_pending_cluster_state_bundle(bundle_with_global_merges()); + EXPECT_TRUE(_repo.get(FixedBucketSpaces::default_space()).merges_inhibited()); + EXPECT_FALSE(_repo.get(FixedBucketSpaces::global_space()).merges_inhibited()); +} + +TEST_F(DistributorBucketSpaceRepoTest, pending_bundle_without_pending_global_unsets_merge_inhibition) { + _repo.enable_cluster_state_bundle(bundle_with_global_merges()); + _repo.set_pending_cluster_state_bundle(bundle_without_global_merges()); + EXPECT_FALSE(_repo.get(FixedBucketSpaces::default_space()).merges_inhibited()); + EXPECT_FALSE(_repo.get(FixedBucketSpaces::global_space()).merges_inhibited()); +} + +} diff --git a/storage/src/tests/distributor/distributor_stripe_test.cpp b/storage/src/tests/distributor/distributor_stripe_test.cpp index 8c2ebc983fa..709f2e6cdc5 100644 --- a/storage/src/tests/distributor/distributor_stripe_test.cpp +++ b/storage/src/tests/distributor/distributor_stripe_test.cpp @@ -629,6 +629,19 @@ TEST_F(DistributorStripeTest, max_clock_skew_config_is_propagated_to_distributor EXPECT_EQ(getConfig().getMaxClusterClockSkew(), std::chrono::seconds(5)); } +TEST_F(DistributorStripeTest, inhibit_default_merge_if_global_merges_pending_config_is_propagated) +{ + setup_stripe(Redundancy(2), NodeCount(2), "storage:2 distributor:1"); + ConfigBuilder builder; + builder.inhibitDefaultMergesWhenGlobalMergesPending = true; + configure_stripe(builder); + EXPECT_TRUE(getConfig().inhibit_default_merges_when_global_merges_pending()); + + builder.inhibitDefaultMergesWhenGlobalMergesPending = false; + configure_stripe(builder); + EXPECT_FALSE(getConfig().inhibit_default_merges_when_global_merges_pending()); +} + namespace { auto makeDummyRemoveCommand() { diff --git a/storage/src/tests/distributor/statecheckerstest.cpp b/storage/src/tests/distributor/statecheckerstest.cpp index d481370b2c1..94f913a3325 100644 --- a/storage/src/tests/distributor/statecheckerstest.cpp +++ b/storage/src/tests/distributor/statecheckerstest.cpp @@ -175,6 +175,8 @@ struct StateCheckersTest : Test, DistributorStripeTestUtil { bool _includeSchedulingPriority {false}; bool _merge_operations_disabled {false}; bool _prioritize_global_bucket_merges {true}; + bool _config_enable_default_space_merge_inhibition {false}; + bool _merges_inhibited_in_bucket_space {false}; CheckerParams(); ~CheckerParams(); @@ -222,6 +224,14 @@ struct StateCheckersTest : Test, DistributorStripeTestUtil { _bucket_space = bucket_space; return *this; } + CheckerParams& config_enable_default_space_merge_inhibition(bool enabled) noexcept { + _config_enable_default_space_merge_inhibition = enabled; + return *this; + } + CheckerParams& merges_inhibited_in_bucket_space(bool inhibited) noexcept { + _merges_inhibited_in_bucket_space = inhibited; + return *this; + } }; template <typename CheckerImpl> @@ -236,10 +246,12 @@ struct StateCheckersTest : Test, DistributorStripeTestUtil { vespa::config::content::core::StorDistributormanagerConfigBuilder config; config.mergeOperationsDisabled = params._merge_operations_disabled; config.prioritizeGlobalBucketMerges = params._prioritize_global_bucket_merges; + config.inhibitDefaultMergesWhenGlobalMergesPending = params._config_enable_default_space_merge_inhibition; configure_stripe(config); if (!params._pending_cluster_state.empty()) { simulate_set_pending_cluster_state(params._pending_cluster_state); } + getBucketSpaceRepo().get(params._bucket_space).set_merges_inhibited(params._merges_inhibited_in_bucket_space); NodeMaintenanceStatsTracker statsTracker; StateChecker::Context c(node_context(), operation_context(), @@ -818,6 +830,32 @@ TEST_F(StateCheckersTest, no_merge_operation_generated_if_merges_explicitly_conf .merge_operations_disabled(true)); } +TEST_F(StateCheckersTest, no_merge_operation_generated_if_merges_inhibited_in_default_bucket_space_and_config_allowed) { + // Technically, the state checker doesn't look at global vs. non-global but instead defers + // to the distributor bucket space repo to set the inhibition flag on the correct bucket space. + // This particular logic is tested at a higher repo-level. + runAndVerify<SynchronizeAndMoveStateChecker>( + CheckerParams() + .expect("NO OPERATIONS GENERATED") // Would normally generate a merge op + .bucketInfo("0=1,2=2") + .config_enable_default_space_merge_inhibition(true) + .merges_inhibited_in_bucket_space(true) + .clusterState("distributor:1 storage:3")); +} + +TEST_F(StateCheckersTest, merge_operation_still_generated_if_merges_inhibited_in_default_bucket_space_but_config_disallowed) { + runAndVerify<SynchronizeAndMoveStateChecker>( + CheckerParams() + .expect("[Moving bucket to ideal node 1]" + "[Synchronizing buckets with different checksums " + "node(idx=0,crc=0x1,docs=1/1,bytes=1/1,trusted=false,active=false,ready=false), " + "node(idx=2,crc=0x2,docs=2/2,bytes=2/2,trusted=false,active=false,ready=false)]") + .bucketInfo("0=1,2=2") + .config_enable_default_space_merge_inhibition(false) + .merges_inhibited_in_bucket_space(true) + .clusterState("distributor:1 storage:3")); +} + std::string StateCheckersTest::testDeleteExtraCopies( const std::string& bucketInfo, uint32_t redundancy, diff --git a/storage/src/vespa/storage/config/distributorconfiguration.cpp b/storage/src/vespa/storage/config/distributorconfiguration.cpp index 8a40899165f..b4c23725493 100644 --- a/storage/src/vespa/storage/config/distributorconfiguration.cpp +++ b/storage/src/vespa/storage/config/distributorconfiguration.cpp @@ -51,6 +51,7 @@ DistributorConfiguration::DistributorConfiguration(StorageComponent& component) _enable_revert(true), _implicitly_clear_priority_on_schedule(false), _use_unordered_merge_chaining(false), + _inhibit_default_merges_when_global_merges_pending(false), _minimumReplicaCountingMode(ReplicaCountingMode::TRUSTED) { } @@ -173,6 +174,7 @@ DistributorConfiguration::configure(const vespa::config::content::core::StorDist _enable_revert = config.enableRevert; _implicitly_clear_priority_on_schedule = config.implicitlyClearBucketPriorityOnSchedule; _use_unordered_merge_chaining = config.useUnorderedMergeChaining; + _inhibit_default_merges_when_global_merges_pending = config.inhibitDefaultMergesWhenGlobalMergesPending; _minimumReplicaCountingMode = config.minimumReplicaCountingMode; diff --git a/storage/src/vespa/storage/config/distributorconfiguration.h b/storage/src/vespa/storage/config/distributorconfiguration.h index ea1aca17116..b26f115827e 100644 --- a/storage/src/vespa/storage/config/distributorconfiguration.h +++ b/storage/src/vespa/storage/config/distributorconfiguration.h @@ -273,6 +273,12 @@ public: [[nodiscard]] bool use_unordered_merge_chaining() const noexcept { return _use_unordered_merge_chaining; } + void set_inhibit_default_merges_when_global_merges_pending(bool inhibit) noexcept { + _inhibit_default_merges_when_global_merges_pending = inhibit; + } + [[nodiscard]] bool inhibit_default_merges_when_global_merges_pending() const noexcept { + return _inhibit_default_merges_when_global_merges_pending; + } uint32_t num_distributor_stripes() const noexcept { return _num_distributor_stripes; } @@ -331,6 +337,7 @@ private: bool _enable_revert; bool _implicitly_clear_priority_on_schedule; bool _use_unordered_merge_chaining; + bool _inhibit_default_merges_when_global_merges_pending; DistrConfig::MinimumReplicaCountingMode _minimumReplicaCountingMode; diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp index 7293f9f7acc..5969ccad4cb 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp @@ -28,6 +28,7 @@ DistributorBucketSpace::DistributorBucketSpace(uint16_t node_index) _distribution(), _node_index(node_index), _distribution_bits(1u), + _merges_inhibited(false), _pending_cluster_state(), _available_nodes(), _ownerships(), diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.h b/storage/src/vespa/storage/distributor/distributor_bucket_space.h index 32da78f9972..3dfd1e1ce30 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space.h +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.h @@ -37,6 +37,7 @@ class DistributorBucketSpace { std::shared_ptr<const lib::Distribution> _distribution; uint16_t _node_index; uint16_t _distribution_bits; + bool _merges_inhibited; std::shared_ptr<const lib::ClusterState> _pending_cluster_state; std::vector<bool> _available_nodes; mutable vespalib::hash_map<document::BucketId, BucketOwnershipFlags, document::BucketId::hash> _ownerships; @@ -85,6 +86,13 @@ public: bool has_pending_cluster_state() const noexcept { return static_cast<bool>(_pending_cluster_state); } const lib::ClusterState& get_pending_cluster_state() const noexcept { return *_pending_cluster_state; } + void set_merges_inhibited(bool inhibited) noexcept { + _merges_inhibited = inhibited; + } + [[nodiscard]] bool merges_inhibited() const noexcept { + return _merges_inhibited; + } + /** * Returns true if this distributor owns the given bucket in the * given cluster and current distribution config. diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp index 09468b55430..c88abaf8373 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp @@ -3,6 +3,7 @@ #include "distributor_bucket_space_repo.h" #include "distributor_bucket_space.h" #include <vespa/vdslib/state/cluster_state_bundle.h> +#include <vespa/vdslib/state/clusterstate.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> #include <cassert> @@ -10,14 +11,15 @@ LOG_SETUP(".distributor.distributor_bucket_space_repo"); using document::BucketSpace; +using document::FixedBucketSpaces; namespace storage::distributor { DistributorBucketSpaceRepo::DistributorBucketSpaceRepo(uint16_t node_index) : _map() { - add(document::FixedBucketSpaces::default_space(), std::make_unique<DistributorBucketSpace>(node_index)); - add(document::FixedBucketSpaces::global_space(), std::make_unique<DistributorBucketSpace>(node_index)); + add(FixedBucketSpaces::default_space(), std::make_unique<DistributorBucketSpace>(node_index)); + add(FixedBucketSpaces::global_space(), std::make_unique<DistributorBucketSpace>(node_index)); } DistributorBucketSpaceRepo::~DistributorBucketSpaceRepo() = default; @@ -44,12 +46,54 @@ DistributorBucketSpaceRepo::get(BucketSpace bucketSpace) const return *itr->second; } +namespace { + +bool content_node_is_up(const lib::ClusterState& state, uint16_t index) noexcept { + return (state.getNodeState(lib::Node(lib::NodeType::STORAGE, index)).getState() == lib::State::UP); +} + +bool content_node_is_in_maintenance(const lib::ClusterState& state, uint16_t index) noexcept { + return (state.getNodeState(lib::Node(lib::NodeType::STORAGE, index)).getState() == lib::State::MAINTENANCE); +} + +// Prioritized global bucket merging is taking place if at least one content node is +// marked as Up in the global bucket space state, but Maintenance in the default +// bucket space state. +bool bundle_implies_global_merging_active(const lib::ClusterStateBundle& bundle) noexcept { + auto& default_cs = bundle.getDerivedClusterState(FixedBucketSpaces::default_space()); + auto& global_cs = bundle.getDerivedClusterState(FixedBucketSpaces::global_space()); + if (default_cs.get() == global_cs.get()) { + return false; + } + uint16_t node_count = global_cs->getNodeCount(lib::NodeType::STORAGE); + for (uint16_t i = 0; i < node_count; ++i) { + if (content_node_is_up(*global_cs, i) && content_node_is_in_maintenance(*default_cs, i)) { + return true; + } + } + return false; +} + +} + +void +DistributorBucketSpaceRepo::enable_cluster_state_bundle(const lib::ClusterStateBundle& cluster_state_bundle) +{ + for (auto& entry : _map) { + entry.second->setClusterState(cluster_state_bundle.getDerivedClusterState(entry.first)); + } + get(FixedBucketSpaces::default_space()).set_merges_inhibited( + bundle_implies_global_merging_active(cluster_state_bundle)); +} + void DistributorBucketSpaceRepo::set_pending_cluster_state_bundle(const lib::ClusterStateBundle& cluster_state_bundle) { for (auto& entry : _map) { entry.second->set_pending_cluster_state(cluster_state_bundle.getDerivedClusterState(entry.first)); } + get(FixedBucketSpaces::default_space()).set_merges_inhibited( + bundle_implies_global_merging_active(cluster_state_bundle)); } void diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h index ab1e235ae35..d77a9f37fb0 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h @@ -33,6 +33,7 @@ public: BucketSpaceMap::const_iterator begin() const { return _map.begin(); } BucketSpaceMap::const_iterator end() const { return _map.end(); } void add(document::BucketSpace bucketSpace, std::unique_ptr<DistributorBucketSpace> distributorBucketSpace); + void enable_cluster_state_bundle(const lib::ClusterStateBundle& cluster_state_bundle); void set_pending_cluster_state_bundle(const lib::ClusterStateBundle& cluster_state_bundle); void clear_pending_cluster_state_bundle(); }; diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp index bcba976f2c3..1173a19d729 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp +++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp @@ -481,9 +481,7 @@ void DistributorStripe::propagateClusterStates() { for (auto* repo : {_bucketSpaceRepo.get(), _readOnlyBucketSpaceRepo.get()}) { - for (auto& iter : *repo) { - iter.second->setClusterState(_clusterStateBundle.getDerivedClusterState(iter.first)); - } + repo->enable_cluster_state_bundle(_clusterStateBundle); } } diff --git a/storage/src/vespa/storage/distributor/statechecker.cpp b/storage/src/vespa/storage/distributor/statechecker.cpp index d1241f0bf66..e5a1822d455 100644 --- a/storage/src/vespa/storage/distributor/statechecker.cpp +++ b/storage/src/vespa/storage/distributor/statechecker.cpp @@ -63,9 +63,9 @@ StateChecker::Result::createStoredResult( StateChecker::Context::Context(const DistributorNodeContext& node_ctx_in, const DistributorStripeOperationContext& op_ctx_in, - const DistributorBucketSpace &distributorBucketSpace, + const DistributorBucketSpace& distributorBucketSpace, NodeMaintenanceStatsTracker& statsTracker, - const document::Bucket &bucket_) + const document::Bucket& bucket_) : bucket(bucket_), siblingBucket(op_ctx_in.get_sibling(bucket.getBucketId())), systemState(distributorBucketSpace.getClusterState()), @@ -77,7 +77,8 @@ StateChecker::Context::Context(const DistributorNodeContext& node_ctx_in, node_ctx(node_ctx_in), op_ctx(op_ctx_in), db(distributorBucketSpace.getBucketDatabase()), - stats(statsTracker) + stats(statsTracker), + merges_inhibited_in_bucket_space(distributorBucketSpace.merges_inhibited()) { idealState = distributorBucketSpace.get_ideal_service_layer_nodes_bundle(bucket.getBucketId()).get_available_nonretired_or_maintenance_nodes(); unorderedIdealState.insert(idealState.begin(), idealState.end()); diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h index 3f938238ce7..b72898208e6 100644 --- a/storage/src/vespa/storage/distributor/statechecker.h +++ b/storage/src/vespa/storage/distributor/statechecker.h @@ -82,6 +82,7 @@ public: const DistributorStripeOperationContext& op_ctx; const BucketDatabase& db; NodeMaintenanceStatsTracker& stats; + const bool merges_inhibited_in_bucket_space; const BucketDatabase::Entry& getSiblingEntry() const { return siblingEntry; @@ -97,7 +98,7 @@ public: class ResultImpl { public: - virtual ~ResultImpl() {} + virtual ~ResultImpl() = default; virtual IdealStateOperation::UP createOperation() = 0; virtual MaintenancePriority getPriority() const = 0; virtual MaintenanceOperation::Type getType() const = 0; diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp index d400151b349..71b2da1359a 100644 --- a/storage/src/vespa/storage/distributor/statecheckers.cpp +++ b/storage/src/vespa/storage/distributor/statecheckers.cpp @@ -832,12 +832,20 @@ allCopiesAreInvalid(const StateChecker::Context& c) return true; } +bool +merging_effectively_disabled_for_state_checker(const StateChecker::Context& c) noexcept +{ + return (c.distributorConfig.merge_operations_disabled() + || (c.distributorConfig.inhibit_default_merges_when_global_merges_pending() + && c.merges_inhibited_in_bucket_space)); +} + } StateChecker::Result SynchronizeAndMoveStateChecker::check(StateChecker::Context& c) { - if (c.distributorConfig.merge_operations_disabled()) { + if (merging_effectively_disabled_for_state_checker(c)) { return Result::noMaintenanceNeeded(); } if (isInconsistentlySplit(c)) { diff --git a/vdslib/src/vespa/vdslib/state/cluster_state_bundle.cpp b/vdslib/src/vespa/vdslib/state/cluster_state_bundle.cpp index 0da172fc789..ce00897acdf 100644 --- a/vdslib/src/vespa/vdslib/state/cluster_state_bundle.cpp +++ b/vdslib/src/vespa/vdslib/state/cluster_state_bundle.cpp @@ -62,8 +62,8 @@ ClusterStateBundle::ClusterStateBundle(const ClusterState& baselineClusterState, ClusterStateBundle::ClusterStateBundle(const ClusterStateBundle&) = default; ClusterStateBundle& ClusterStateBundle::operator=(const ClusterStateBundle&) = default; -ClusterStateBundle::ClusterStateBundle(ClusterStateBundle&&) = default; -ClusterStateBundle& ClusterStateBundle::operator=(ClusterStateBundle&&) = default; +ClusterStateBundle::ClusterStateBundle(ClusterStateBundle&&) noexcept = default; +ClusterStateBundle& ClusterStateBundle::operator=(ClusterStateBundle&&) noexcept = default; ClusterStateBundle::~ClusterStateBundle() = default; @@ -90,7 +90,7 @@ ClusterStateBundle::getVersion() const } bool -ClusterStateBundle::operator==(const ClusterStateBundle &rhs) const +ClusterStateBundle::operator==(const ClusterStateBundle &rhs) const noexcept { if (!(*_baselineClusterState == *rhs._baselineClusterState)) { return false; diff --git a/vdslib/src/vespa/vdslib/state/cluster_state_bundle.h b/vdslib/src/vespa/vdslib/state/cluster_state_bundle.h index ad9e07407a0..52a952e30bb 100644 --- a/vdslib/src/vespa/vdslib/state/cluster_state_bundle.h +++ b/vdslib/src/vespa/vdslib/state/cluster_state_bundle.h @@ -65,8 +65,8 @@ public: ClusterStateBundle(const ClusterStateBundle&); ClusterStateBundle& operator=(const ClusterStateBundle&); - ClusterStateBundle(ClusterStateBundle&&); - ClusterStateBundle& operator=(ClusterStateBundle&&); + ClusterStateBundle(ClusterStateBundle&&) noexcept; + ClusterStateBundle& operator=(ClusterStateBundle&&) noexcept; ~ClusterStateBundle(); const std::shared_ptr<const ClusterState> &getBaselineClusterState() const; @@ -74,15 +74,15 @@ public: const BucketSpaceStateMapping& getDerivedClusterStates() const noexcept { return _derivedBucketSpaceStates; } - bool block_feed_in_cluster() const { + [[nodiscard]] bool block_feed_in_cluster() const noexcept { return _feed_block.has_value() && _feed_block->block_feed_in_cluster(); } const std::optional<FeedBlock>& feed_block() const { return _feed_block; } uint32_t getVersion() const; bool deferredActivation() const noexcept { return _deferredActivation; } std::string toString() const; - bool operator==(const ClusterStateBundle &rhs) const; - bool operator!=(const ClusterStateBundle &rhs) const { return !operator==(rhs); } + bool operator==(const ClusterStateBundle &rhs) const noexcept; + bool operator!=(const ClusterStateBundle &rhs) const noexcept { return !operator==(rhs); } }; std::ostream& operator<<(std::ostream&, const ClusterStateBundle&); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java index 548470b0a37..d83eab9e339 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java @@ -9,7 +9,6 @@ import com.yahoo.vespa.athenz.api.AthenzPolicy; import com.yahoo.vespa.athenz.api.AthenzResourceName; import com.yahoo.vespa.athenz.api.AthenzRole; import com.yahoo.vespa.athenz.api.AthenzService; -import com.yahoo.vespa.athenz.api.AthenzUser; import com.yahoo.vespa.athenz.api.OktaAccessToken; import com.yahoo.vespa.athenz.api.OktaIdentityToken; import com.yahoo.vespa.athenz.client.ErrorHandler; @@ -394,8 +393,12 @@ public class DefaultZmsClient extends ClientBase implements ZmsClient { @Override public void createSubdomain(AthenzDomain parent, String name) { URI uri = zmsUrl.resolve(String.format("subdomain/%s", parent.getName())); - StringEntity entity = toJsonStringEntity(Map.of("name", name)); - var request = RequestBuilder.put(uri) + StringEntity entity = toJsonStringEntity( + Map.of("name", name, + "parent", parent.getName(), + "adminUsers", List.of(identity.getFullName())) // TODO: createSubdomain should receive an adminUsers argument + ); + var request = RequestBuilder.post(uri) .setEntity(entity) .build(); execute(request, response -> readEntity(response, Void.class)); diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java index 180ddca2ca5..4d7c2f1cc14 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java @@ -20,6 +20,7 @@ import com.yahoo.document.FixedBucketSpaces; import com.yahoo.document.TestAndSetCondition; import com.yahoo.document.config.DocumentmanagerConfig; import com.yahoo.document.fieldset.AllFields; +import com.yahoo.document.fieldset.DocumentOnly; import com.yahoo.document.fieldset.DocIdOnly; import com.yahoo.document.idstring.IdIdString; import com.yahoo.document.json.DocumentOperationType; @@ -379,6 +380,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { StorageCluster destination = resolveCluster(Optional.of(requireProperty(request, DESTINATION_CLUSTER)), clusters); VisitorParameters parameters = parseParameters(request, path); parameters.setRemoteDataHandler("[Content:cluster=" + destination.name() + "]"); // Bypass indexing. + // TODO Vespa 8: change to DocumentOnly.NAME parameters.setFieldSet(AllFields.NAME); return () -> { visitWithRemote(request, parameters, handler); @@ -1088,6 +1090,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { throw new IllegalArgumentException("Must set 'cluster' parameter to a valid content cluster id when visiting at a root /document/v1/ level"); VisitorParameters parameters = parseCommonParameters(request, path, cluster); + // TODO Vespa 8: change to DocumentOnly.NAME parameters.setFieldSet(getProperty(request, FIELD_SET).orElse(path.documentType().map(type -> type + ":[document]").orElse(AllFields.NAME))); parameters.setMaxTotalHits(wantedDocumentCount); parameters.visitInconsistentBuckets(true); diff --git a/vespaclient-java/src/main/java/com/yahoo/vespaget/CommandLineOptions.java b/vespaclient-java/src/main/java/com/yahoo/vespaget/CommandLineOptions.java index 0eef559da3f..da9f48f44b1 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespaget/CommandLineOptions.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespaget/CommandLineOptions.java @@ -2,6 +2,7 @@ package com.yahoo.vespaget; import com.yahoo.document.fieldset.AllFields; +import com.yahoo.document.fieldset.DocumentOnly; import com.yahoo.document.fieldset.DocIdOnly; import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol; import org.apache.commons.cli.CommandLine; @@ -67,6 +68,7 @@ public class CommandLineOptions { .longOpt(PRINTIDS_OPTION) .build()); + // TODO Vespa 8: change to DocumentOnly.NAME options.addOption(Option.builder("f") .hasArg(true) .desc("Retrieve the specified fields only (see https://docs.vespa.ai/en/documents.html#fieldsets) (default '" + AllFields.NAME + "')") @@ -181,8 +183,9 @@ public class CommandLineOptions { if (printIdsOnly) { fieldSet = DocIdOnly.NAME; - } else if (fieldSet.isEmpty()) { - fieldSet = AllFields.NAME; + } else if (fieldSet.isEmpty()) { + // TODO Vespa 8: change to DocumentOnly.NAME + fieldSet = AllFields.NAME; } if (!cluster.isEmpty() && !route.isEmpty()) { diff --git a/vespaclient-java/src/test/java/com/yahoo/vespaget/CommandLineOptionsTest.java b/vespaclient-java/src/test/java/com/yahoo/vespaget/CommandLineOptionsTest.java index 6af2344e36e..b634e899b74 100644 --- a/vespaclient-java/src/test/java/com/yahoo/vespaget/CommandLineOptionsTest.java +++ b/vespaclient-java/src/test/java/com/yahoo/vespaget/CommandLineOptionsTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespaget; import com.yahoo.document.fieldset.AllFields; +import com.yahoo.document.fieldset.DocumentOnly; import com.yahoo.document.fieldset.DocIdOnly; import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol; import org.junit.Rule; @@ -55,6 +56,7 @@ public class CommandLineOptionsTest { assertFalse(params.help); assertFalse(params.documentIds.hasNext()); assertFalse(params.printIdsOnly); + // TODO Vespa 8: change to DocumentOnly.NAME assertEquals(AllFields.NAME, params.fieldSet); assertEquals("default-get", params.route); assertTrue(params.cluster.isEmpty()); diff --git a/vespajlib/src/main/java/com/yahoo/tensor/TypeResolver.java b/vespajlib/src/main/java/com/yahoo/tensor/TypeResolver.java index dad93734b22..457cfcbfa5f 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/TypeResolver.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/TypeResolver.java @@ -2,13 +2,9 @@ package com.yahoo.tensor; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; import static com.yahoo.tensor.TensorType.Dimension; diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java index a6f71dacf30..09bfb8b996b 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java @@ -121,14 +121,13 @@ public class Slice<NAMETYPE extends Name> extends PrimitiveTensorFunction<NAMETY private TensorType resultType(TensorType argumentType) { List<String> peekDimensions; - // Special case where a single indexed or mapped dimension is sliced if (subspaceAddress.size() == 1 && subspaceAddress.get(0).dimension().isEmpty()) { if (subspaceAddress.get(0).index().isPresent()) { peekDimensions = findDimensions(argumentType.dimensions(), TensorType.Dimension::isIndexed); if (peekDimensions.size() > 1) { throw new IllegalArgumentException(this + " slices a single indexed dimension, cannot be applied " + - "to " + argumentType + ", which has multiple"); + "to " + argumentType + ", which has multiple"); } } else { @@ -141,6 +140,8 @@ public class Slice<NAMETYPE extends Name> extends PrimitiveTensorFunction<NAMETY else { // general slicing peekDimensions = subspaceAddress.stream().map(d -> d.dimension().get()).collect(Collectors.toList()); } + if (peekDimensions.isEmpty()) + throw new IllegalArgumentException(this + " cannot slice " + argumentType + ": No dimensions to slice"); return TypeResolver.peek(argumentType, peekDimensions); } |