diff options
101 files changed, 1738 insertions, 1576 deletions
diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java index 864c8d6fe9c..d5aaa5888b2 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java @@ -181,16 +181,15 @@ public class GenerateOsgiManifestMojo extends AbstractGenerateOsgiManifestMojo { artifact.getId(), artifact.getType()))); } - // TODO: fail the build by throwing a MojoExecutionException private void warnIfInternalContainerArtifactsAreIncluded(Collection<Artifact> includedArtifacts) throws MojoExecutionException { /* In most cases it's sufficient to test for 'component', as it's the lowest level container artifact, * Embedding container artifacts will cause class loading issues at runtime, because the classes will - * not be equal to those seen by the framework (e.g. AbstractComponent). - */ + * not be equal to those seen by the framework (e.g. AbstractComponent). */ if (includedArtifacts.stream().anyMatch(this::isJdiscComponentArtifact)) { - getLog().warn("This project includes the 'com.yahoo.vespa:component' artifact in compile scope." + - " It must be set to scope 'provided' to avoid resource leaks in your application at runtime." + - " The build will fail on a future Vespa version unless this is fixed."); + throw new MojoExecutionException( + "This project includes the 'com.yahoo.vespa:component' artifact in compile scope." + + " It must have scope 'provided' to avoid resource leaks in your application at runtime." + + " Please use 'mvn dependency:tree' to find the root cause."); } } diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index db160c5f6ae..5ca22b695a7 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -98,7 +98,10 @@ public interface ModelContext { // Select sequencer type use while feeding. String feedSequencerType(); - boolean useDistributorBtreeDb(); + // TODO Remove when 7.247 is last + default boolean useDistributorBtreeDb() { return true; } + + boolean useContentNodeBtreeDb(); boolean useThreePhaseUpdates(); diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 181c4ec100a..a31574eea10 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -37,7 +37,7 @@ public class TestProperties implements ModelContext.Properties { private Zone zone; private Set<ContainerEndpoint> endpoints = Collections.emptySet(); private boolean useDedicatedNodeForLogserver = false; - private boolean useDistributorBtreeDb = false; + private boolean useContentNodeBtreeDb = false; private boolean useThreePhaseUpdates = false; private double defaultTermwiseLimit = 1.0; private double threadPoolSizeFactor = 0.0; @@ -70,7 +70,7 @@ public class TestProperties implements ModelContext.Properties { @Override public double queueSizeFactor() { return queueSizeFactor; } - @Override public boolean useDistributorBtreeDb() { return useDistributorBtreeDb; } + @Override public boolean useContentNodeBtreeDb() { return useContentNodeBtreeDb; } @Override public boolean useThreePhaseUpdates() { return useThreePhaseUpdates; } @Override public Optional<AthenzDomain> athenzDomain() { return Optional.ofNullable(athenzDomain); } @Override public Optional<ApplicationRoles> applicationRoles() { return Optional.ofNullable(applicationRoles); } @@ -88,8 +88,8 @@ public class TestProperties implements ModelContext.Properties { return this; } - public TestProperties setUseDistributorBtreeDB(boolean useBtreeDb) { - useDistributorBtreeDb = useBtreeDb; + public TestProperties setUseContentNodeBtreeDB(boolean useBtreeDb) { + useContentNodeBtreeDb = useBtreeDb; return this; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java index 4f3e100ce75..5d790c74f18 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java @@ -96,8 +96,8 @@ public class AttributeChangeValidator { validateAttributeSetting(currAttr, nextAttr, Attribute::densePostingListThreshold, "dense-posting-list-threshold", result); validateAttributeSetting(currAttr, nextAttr, Attribute::isEnabledOnlyBitVector, "rank: filter", result); validateAttributeSetting(currAttr, nextAttr, AttributeChangeValidator::hasHnswIndex, "indexing: index", result); + validateAttributeSetting(currAttr, nextAttr, Attribute::distanceMetric, "distance-metric", result); if (hasHnswIndex(currAttr) && hasHnswIndex(nextAttr)) { - validateAttributeSetting(currAttr, nextAttr, Attribute::distanceMetric, "distance-metric", result); validateAttributeHnswIndexSetting(currAttr, nextAttr, HnswIndexParams::maxLinksPerNode, "max-links-per-node", result); validateAttributeHnswIndexSetting(currAttr, nextAttr, HnswIndexParams::neighborsToExploreAtInsert, "neighbors-to-explore-at-insert", result); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java index 0b6b7154a62..25e3efcb78f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java @@ -41,7 +41,6 @@ public class DistributorCluster extends AbstractConfigProducer<Distributor> impl private final BucketSplitting bucketSplitting; private final GcOptions gc; private final boolean hasIndexedDocumentType; - private final boolean useBtreeDatabase; private final boolean useThreePhaseUpdates; public static class Builder extends VespaDomBuilder.DomConfigProducerBuilder<DistributorCluster> { @@ -103,25 +102,23 @@ public class DistributorCluster extends AbstractConfigProducer<Distributor> impl final ModelElement documentsNode = clusterElement.child("documents"); final GcOptions gc = parseGcOptions(documentsNode); final boolean hasIndexedDocumentType = clusterContainsIndexedDocumentType(documentsNode); - boolean useBtreeDb = deployState.getProperties().useDistributorBtreeDb(); boolean useThreePhaseUpdates = deployState.getProperties().useThreePhaseUpdates(); return new DistributorCluster(parent, new BucketSplitting.Builder().build(new ModelElement(producerSpec)), gc, - hasIndexedDocumentType, useBtreeDb, useThreePhaseUpdates); + hasIndexedDocumentType, useThreePhaseUpdates); } } private DistributorCluster(ContentCluster parent, BucketSplitting bucketSplitting, GcOptions gc, boolean hasIndexedDocumentType, - boolean useBtreeDatabase, boolean useThreePhaseUpdates) + boolean useThreePhaseUpdates) { super(parent, "distributor"); this.parent = parent; this.bucketSplitting = bucketSplitting; this.gc = gc; this.hasIndexedDocumentType = hasIndexedDocumentType; - this.useBtreeDatabase = useBtreeDatabase; this.useThreePhaseUpdates = useThreePhaseUpdates; } @@ -134,7 +131,6 @@ public class DistributorCluster extends AbstractConfigProducer<Distributor> impl } builder.enable_revert(parent.getPersistence().supportRevert()); builder.disable_bucket_activation(hasIndexedDocumentType == false); - builder.use_btree_database(useBtreeDatabase); builder.enable_metadata_only_fetch_phase_for_inconsistent_updates(useThreePhaseUpdates); bucketSplitting.getConfig(builder); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java index 36e4554e610..9e4c9bde1e4 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java @@ -10,32 +10,36 @@ import com.yahoo.vespa.model.builder.xml.dom.ModelElement; */ public class StorServerProducer implements StorServerConfig.Producer { public static class Builder { - StorServerProducer build(ModelElement element) { + StorServerProducer build(ModelElement element, boolean useBtreeDatabase) { ModelElement tuning = element.child("tuning"); if (tuning == null) { - return new StorServerProducer(ContentCluster.getClusterId(element), null, null); + return new StorServerProducer(ContentCluster.getClusterId(element), null, null, useBtreeDatabase); } ModelElement merges = tuning.child("merges"); if (merges == null) { - return new StorServerProducer(ContentCluster.getClusterId(element), null, null); + return new StorServerProducer(ContentCluster.getClusterId(element), null, null, useBtreeDatabase); } return new StorServerProducer(ContentCluster.getClusterId(element), merges.integerAttribute("max-per-node"), - merges.integerAttribute("max-queue-size")); + merges.integerAttribute("max-queue-size"), + useBtreeDatabase); } } - private String clusterName; - private Integer maxMergesPerNode; - private Integer queueSize; + private final String clusterName; + private final Integer maxMergesPerNode; + private final Integer queueSize; + private final boolean useBtreeDatabase; - public StorServerProducer(String clusterName, Integer maxMergesPerNode, Integer queueSize) { + public StorServerProducer(String clusterName, Integer maxMergesPerNode, + Integer queueSize, boolean useBtreeDatabase) { this.clusterName = clusterName; this.maxMergesPerNode = maxMergesPerNode; this.queueSize = queueSize; + this.useBtreeDatabase = useBtreeDatabase; } @Override @@ -52,5 +56,6 @@ public class StorServerProducer implements StorServerConfig.Producer { if (queueSize != null) { builder.max_merge_queue_size(queueSize); } + builder.use_content_node_btree_bucket_db(useBtreeDatabase); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java index 2e5594a001d..0a8abfbd3ad 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java @@ -33,12 +33,13 @@ public class StorageCluster extends AbstractConfigProducer<StorageNode> protected StorageCluster doBuild(DeployState deployState, AbstractConfigProducer ancestor, Element producerSpec) { final ModelElement clusterElem = new ModelElement(producerSpec); final ContentCluster cluster = (ContentCluster)ancestor; + boolean useContentNodeBtreeDb = deployState.getProperties().useContentNodeBtreeDb(); return new StorageCluster(ancestor, ContentCluster.getClusterId(clusterElem), new FileStorProducer.Builder().build(cluster, clusterElem), new IntegrityCheckerProducer.Builder().build(cluster, clusterElem), - new StorServerProducer.Builder().build(clusterElem), + new StorServerProducer.Builder().build(clusterElem, useContentNodeBtreeDb), new StorVisitorProducer.Builder().build(clusterElem), new PersistenceProducer.Builder().build(clusterElem)); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java index 5da36d82a62..e00b34d4a79 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java @@ -168,11 +168,12 @@ public class AttributeChangeValidatorTest { } @Test - public void changing_distance_metric_without_hnsw_index_enabled_is_ok() throws Exception { + public void changing_distance_metric_without_hnsw_index_enabled_requires_restart() throws Exception { new Fixture("field f1 type tensor(x[2]) { indexing: attribute }", "field f1 type tensor(x[2]) { indexing: attribute \n attribute { " + "distance-metric: geodegrees \n } }"). - assertValidation(); + assertValidation(newRestartAction("Field 'f1' changed: change property " + + "'distance-metric' from 'EUCLIDEAN' to 'GEODEGREES'")); } @Test diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java index 5b3c42df869..1f0ec0ea910 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java @@ -949,20 +949,20 @@ public class ContentClusterTest extends ContentBaseTest { verifyTopKProbabilityPropertiesControl(); } - private boolean resolveDistributorBtreeDbConfigWithFeatureFlag(boolean flagEnabledBtreeDb) { - VespaModel model = createEnd2EndOneNode(new TestProperties().setUseDistributorBtreeDB(flagEnabledBtreeDb)); + private boolean resolveContentNodeBtreeDbConfigWithFeatureFlag(boolean flagEnabledBtreeDb) { + VespaModel model = createEnd2EndOneNode(new TestProperties().setUseContentNodeBtreeDB(flagEnabledBtreeDb)); ContentCluster cc = model.getContentClusters().get("storage"); - var builder = new StorDistributormanagerConfig.Builder(); - cc.getDistributorNodes().getConfig(builder); + var builder = new StorServerConfig.Builder(); + cc.getStorageNodes().getConfig(builder); - return (new StorDistributormanagerConfig(builder)).use_btree_database(); + return (new StorServerConfig(builder)).use_content_node_btree_bucket_db(); } @Test - public void default_distributor_btree_usage_controlled_by_properties() { - assertFalse(resolveDistributorBtreeDbConfigWithFeatureFlag(false)); - assertTrue(resolveDistributorBtreeDbConfigWithFeatureFlag(true)); + public void default_content_node_btree_usage_controlled_by_properties() { + assertFalse(resolveContentNodeBtreeDbConfigWithFeatureFlag(false)); + assertTrue(resolveContentNodeBtreeDbConfigWithFeatureFlag(true)); } private boolean resolveThreePhaseUpdateConfigWithFeatureFlag(boolean flagEnableThreePhase) { diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java b/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java index 9710ee607eb..82d02ec89f9 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java @@ -22,9 +22,9 @@ public class ConfigInstanceUtil { * Values that have not been explicitly set in the source builder, will be left unchanged * in the destination. * - * @param destination The builder to copy values into. - * @param source The builder to copy values from. Unset values are not copied. - * @param <BUILDER> The builder class. + * @param destination the builder to copy values into + * @param source the builder to copy values from. Unset values are not copied + * @param <BUILDER> the builder class */ public static<BUILDER extends ConfigBuilder> void setValues(BUILDER destination, BUILDER source) { try { @@ -56,9 +56,9 @@ public class ConfigInstanceUtil { setConfigId(i, configId); } catch (InstantiationException | InvocationTargetException | NoSuchMethodException | - NoSuchFieldException | IllegalAccessException e) { + NoSuchFieldException | IllegalAccessException e) { throw new IllegalArgumentException("Failed creating new instance of '" + type.getCanonicalName() + - "' for config id '" + configId + "': " + Exceptions.toMessageString(e), e); + "' for config id '" + configId + "'", e); } return instance; } diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java index 814222989a1..5fcbd4f8c21 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java @@ -423,21 +423,35 @@ public class ConfigSubscriber implements AutoCloseable { * @return The handle of the config * @see #startConfigThread(Runnable) */ - public <T extends ConfigInstance> ConfigHandle<T> subscribe(final SingleSubscriber<T> singleSubscriber, Class<T> configClass, String configId) { - if (!subscriptionHandles.isEmpty()) - throw new IllegalStateException("Can not start single-subscription because subscriptions were previously opened on this."); - final ConfigHandle<T> handle = subscribe(configClass, configId); - if (!nextConfig()) - throw new ConfigurationRuntimeException("Initial config of " + configClass.getName() + " failed."); + public <T extends ConfigInstance> ConfigHandle<T> subscribe(SingleSubscriber<T> singleSubscriber, Class<T> configClass, String configId) { + if ( ! subscriptionHandles.isEmpty()) + throw new IllegalStateException("Can not start single-subscription because subscriptions were previously opened on this"); + + ConfigHandle<T> handle = subscribe(configClass, configId); + + if ( ! nextConfig()) + throw new ConfigurationRuntimeException("Initial config of " + configClass.getName() + " failed"); + singleSubscriber.configure(handle.getConfig()); startConfigThread(() -> { while (!isClosed()) { + boolean hasNewConfig = false; + try { - if (nextConfig()) { - if (handle.isChanged()) singleSubscriber.configure(handle.getConfig()); - } - } catch (Exception e) { - log.log(SEVERE, "Exception from config system, continuing config thread: " + Exceptions.toMessageString(e)); + hasNewConfig = nextConfig(); + } + catch (Exception e) { + log.log(SEVERE, "Exception on receiving config. Ignoring this change.", e); + } + + try { + if (hasNewConfig) + singleSubscriber.configure(handle.getConfig()); + } + catch (Exception e) { + log.warning("Exception on applying config " + configClass.getName() + + " for config id " + configId + ": Ignoring this change: " + + Exceptions.toMessageString(e)); } } } diff --git a/config/src/test/java/com/yahoo/config/subscription/ConfigInstanceUtilTest.java b/config/src/test/java/com/yahoo/config/subscription/ConfigInstanceUtilTest.java index 3bdaee09eaf..fa24988709a 100644 --- a/config/src/test/java/com/yahoo/config/subscription/ConfigInstanceUtilTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/ConfigInstanceUtilTest.java @@ -14,7 +14,6 @@ import java.io.File; import java.util.Arrays; import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; @@ -165,7 +164,8 @@ public class ConfigInstanceUtilTest { ConfigInstanceUtil.getNewInstance(TestNodefsConfig.class, "id0", ConfigPayload.fromBuilder(payloadBuilder)); assert(false); } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), containsString("Failed creating new instance of 'com.yahoo.foo.TestNodefsConfig' for config id 'id0':")); + assertEquals("Failed creating new instance of 'com.yahoo.foo.TestNodefsConfig' for config id 'id0'", + e.getMessage()); } } } diff --git a/config/src/tests/configgen/CMakeLists.txt b/config/src/tests/configgen/CMakeLists.txt index 533eeb46f70..663e265530f 100644 --- a/config/src/tests/configgen/CMakeLists.txt +++ b/config/src/tests/configgen/CMakeLists.txt @@ -4,11 +4,9 @@ vespa_add_executable(config_configgen_test_app TEST configgen.cpp DEPENDS config_cloudconfig - $<TARGET_OBJECTS:config_tests_configgen_config> - AFTER - config_tests_configgen_config ) vespa_add_test(NAME config_configgen_test_app COMMAND config_configgen_test_app) +vespa_generate_config(config_configgen_test_app ../../test/resources/configdefinitions/motd.def) vespa_add_executable(config_vector_inserter_test_app TEST SOURCES vector_inserter.cpp @@ -28,13 +26,5 @@ vespa_add_executable(config_value_converter_test_app TEST value_converter.cpp DEPENDS config_cloudconfig - $<TARGET_OBJECTS:config_tests_configgen_config> - AFTER - config_tests_configgen_config ) vespa_add_test(NAME config_value_converter_test_app COMMAND config_value_converter_test_app) - -vespa_add_library(config_tests_configgen_config OBJECT -) - -vespa_generate_config(config_tests_configgen_config ../../test/resources/configdefinitions/motd.def) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ReloadHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ReloadHandler.java deleted file mode 100644 index 93af4b1d593..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ReloadHandler.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server; - -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.config.server.application.ApplicationSet; - -import java.util.Set; - -/** - * Interface representing a reload handler. - * - * @author Ulf Lilleengen - */ -public interface ReloadHandler { - - /** - * Reload config with the one contained in the application. - * - * @param applicationSet The set of applications to set as active. - */ - void reloadConfig(ApplicationSet applicationSet); - - /** - * Remove an application and resources related to it. - * - * @param applicationId to be removed - */ - void removeApplication(ApplicationId applicationId); - - /** - * Remove all applications except those specified in argument. - * - * @param applicationIds to be kept - */ - void removeApplicationsExcept(Set<ApplicationId> applicationIds); - -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelController.java b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelController.java index 23baac3d02e..73ee23e5c03 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelController.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelController.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.config.server; import com.yahoo.config.ConfigInstance; import com.yahoo.config.codegen.DefParser; -import com.yahoo.config.codegen.InnerCNode; import com.yahoo.config.model.api.ConfigDefinitionRepo; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.config.ConfigDefinitionKey; @@ -47,22 +46,21 @@ public class SuperModelController { */ public ConfigResponse resolveConfig(GetConfigRequest request) { ConfigKey<?> configKey = request.getConfigKey(); - InnerCNode targetDef = getConfigDefinition(request.getConfigKey(), request.getDefContent()); + validateConfigDefinition(request.getConfigKey(), request.getDefContent()); ConfigPayload payload = model.getConfig(configKey); - return responseFactory.createResponse(payload, targetDef, generation, false); + return responseFactory.createResponse(payload, generation, false); } - private InnerCNode getConfigDefinition(ConfigKey<?> configKey, DefContent defContent) { + private void validateConfigDefinition(ConfigKey<?> configKey, DefContent defContent) { if (defContent.isEmpty()) { ConfigDefinitionKey configDefinitionKey = new ConfigDefinitionKey(configKey.getName(), configKey.getNamespace()); ConfigDefinition configDefinition = configDefinitionRepo.getConfigDefinitions().get(configDefinitionKey); if (configDefinition == null) { throw new UnknownConfigDefinitionException("Unable to find config definition for '" + configKey.getNamespace() + "." + configKey.getName()); } - return configDefinition.getCNode(); } else { DefParser dParser = new DefParser(configKey.getName(), new StringReader(defContent.asString())); - return dParser.getTree(); + dParser.getTree(); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java index 2560badbf43..51213b173dd 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java @@ -134,7 +134,7 @@ public class Application implements ModelResult { throw new ConfigurationRuntimeException("Unable to resolve config " + configKey); } - ConfigResponse configResponse = responseFactory.createResponse(payload, def.getCNode(), appGeneration, internalRedeploy); + ConfigResponse configResponse = responseFactory.createResponse(payload, appGeneration, internalRedeploy); metricUpdater.incrementProcTime(System.currentTimeMillis() - start); if (useCache(req)) { cache.put(cacheKey, configResponse, configResponse.getConfigMd5()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java index 795c6398354..638e3565602 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java @@ -15,7 +15,6 @@ import com.yahoo.vespa.config.GetConfigRequest; import com.yahoo.vespa.config.protocol.ConfigResponse; import com.yahoo.vespa.config.server.GlobalComponentRegistry; import com.yahoo.vespa.config.server.NotFoundException; -import com.yahoo.vespa.config.server.ReloadHandler; import com.yahoo.vespa.config.server.ReloadListener; import com.yahoo.vespa.config.server.RequestHandler; import com.yahoo.vespa.config.server.deploy.TenantFileSystemDirs; @@ -59,7 +58,7 @@ import static java.util.stream.Collectors.toSet; * @author Ulf Lilleengen * @author jonmv */ -public class TenantApplications implements RequestHandler, ReloadHandler, HostValidator<ApplicationId> { +public class TenantApplications implements RequestHandler, HostValidator<ApplicationId> { private static final Logger log = Logger.getLogger(TenantApplications.class.getName()); @@ -258,7 +257,6 @@ public class TenantApplications implements RequestHandler, ReloadHandler, HostVa * * @param applicationSet the {@link ApplicationSet} to be reloaded */ - @Override public void reloadConfig(ApplicationSet applicationSet) { ApplicationId id = applicationSet.getId(); try (Lock lock = lock(id)) { @@ -272,7 +270,6 @@ public class TenantApplications implements RequestHandler, ReloadHandler, HostVa } } - @Override public void removeApplication(ApplicationId applicationId) { try (Lock lock = lock(applicationId)) { if (exists(applicationId)) @@ -288,7 +285,6 @@ public class TenantApplications implements RequestHandler, ReloadHandler, HostVa } } - @Override public void removeApplicationsExcept(Set<ApplicationId> applications) { for (ApplicationId activeApplication : applicationMapper.listApplicationIds()) { if ( ! applications.contains(activeApplication)) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index b252ee3ef56..77cc1075854 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -147,7 +147,7 @@ public class ModelContextImpl implements ModelContext { private final Set<ContainerEndpoint> endpoints; private final boolean isBootstrap; private final boolean isFirstTimeDeployment; - private final boolean useDistributorBtreeDb; + private final boolean useContentNodeBtreeDb; private final boolean useThreePhaseUpdates; private final Optional<EndpointCertificateSecrets> endpointCertificateSecrets; private final double defaultTermwiseLimit; @@ -188,7 +188,7 @@ public class ModelContextImpl implements ModelContext { this.endpointCertificateSecrets = endpointCertificateSecrets; defaultTermwiseLimit = Flags.DEFAULT_TERM_WISE_LIMIT.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); - useDistributorBtreeDb = Flags.USE_DISTRIBUTOR_BTREE_DB.bindTo(flagSource) + useContentNodeBtreeDb = Flags.USE_CONTENT_NODE_BTREE_DB.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); useThreePhaseUpdates = Flags.USE_THREE_PHASE_UPDATES.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); @@ -260,8 +260,8 @@ public class ModelContextImpl implements ModelContext { } @Override - public boolean useDistributorBtreeDb() { - return useDistributorBtreeDb; + public boolean useContentNodeBtreeDb() { + return useContentNodeBtreeDb; } @Override diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java index 54825167d15..88aa41ca735 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.config.server.rpc; import com.yahoo.cloud.config.ConfigserverConfig; -import com.yahoo.config.codegen.InnerCNode; import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.protocol.ConfigResponse; @@ -26,14 +25,12 @@ public interface ConfigResponseFactory { } /** - * Create a {@link ConfigResponse} for a given payload and generation. - * - * @param payload The {@link com.yahoo.vespa.config.ConfigPayload} to put in the response. - * @param defFile The {@link com.yahoo.config.codegen.InnerCNode} def file for this config. - * @param generation The payload generation. @return A {@link ConfigResponse} that can be sent to the client. + * Creates a {@link ConfigResponse} for a given payload and generation. + * @param payload the {@link ConfigPayload} to put in the response. + * @param generation the payload generation. @return A {@link ConfigResponse} that can be sent to the client. * @param internalRedeploy whether this config generation was produced by an internal redeployment, * not a change to the application package */ - ConfigResponse createResponse(ConfigPayload payload, InnerCNode defFile, long generation, boolean internalRedeploy); + ConfigResponse createResponse(ConfigPayload payload, long generation, boolean internalRedeploy); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/LZ4ConfigResponseFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/LZ4ConfigResponseFactory.java index 5235a2bcadd..cba1316a131 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/LZ4ConfigResponseFactory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/LZ4ConfigResponseFactory.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.rpc; -import com.yahoo.config.codegen.InnerCNode; import com.yahoo.text.Utf8Array; import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.LZ4PayloadCompressor; @@ -18,11 +17,10 @@ import com.yahoo.vespa.config.util.ConfigUtils; */ public class LZ4ConfigResponseFactory implements ConfigResponseFactory { - private static LZ4PayloadCompressor compressor = new LZ4PayloadCompressor(); + private static final LZ4PayloadCompressor compressor = new LZ4PayloadCompressor(); @Override public ConfigResponse createResponse(ConfigPayload payload, - InnerCNode defFile, long generation, boolean internalRedeploy) { Utf8Array rawPayload = payload.toUtf8Array(true); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/UncompressedConfigResponseFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/UncompressedConfigResponseFactory.java index bd0b117c3db..2de88ab44cc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/UncompressedConfigResponseFactory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/UncompressedConfigResponseFactory.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.rpc; -import com.yahoo.config.codegen.InnerCNode; import com.yahoo.text.Utf8Array; import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.protocol.CompressionInfo; @@ -19,7 +18,6 @@ public class UncompressedConfigResponseFactory implements ConfigResponseFactory @Override public ConfigResponse createResponse(ConfigPayload payload, - InnerCNode defFile, long generation, boolean internalRedeploy) { Utf8Array rawPayload = payload.toUtf8Array(true); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java index 763c77f2088..66ed721a3e1 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java @@ -8,8 +8,8 @@ import com.yahoo.config.provision.TenantName; import com.yahoo.lang.SettableOptional; import com.yahoo.transaction.Transaction; import com.yahoo.vespa.config.server.GlobalComponentRegistry; -import com.yahoo.vespa.config.server.ReloadHandler; import com.yahoo.vespa.config.server.application.ApplicationSet; +import com.yahoo.vespa.config.server.application.TenantApplications; import com.yahoo.vespa.config.server.modelfactory.ActivatedModelsBuilder; import com.yahoo.vespa.curator.Curator; import org.apache.zookeeper.KeeperException; @@ -81,12 +81,12 @@ public class RemoteSession extends Session { return sessionZooKeeperClient.createWriteStatusTransaction(Status.DELETE); } - void makeActive(ReloadHandler reloadHandler) { + void makeActive(TenantApplications tenantApplications) { Curator.CompletionWaiter waiter = sessionZooKeeperClient.getActiveWaiter(); log.log(Level.FINE, () -> logPre() + "Getting session from repo: " + getSessionId()); ApplicationSet app = ensureApplicationLoaded(); log.log(Level.FINE, () -> logPre() + "Reloading config for " + getSessionId()); - reloadHandler.reloadConfig(app); + tenantApplications.reloadConfig(app); log.log(Level.FINE, () -> logPre() + "Notifying " + waiter); notifyCompletion(waiter); log.log(Level.INFO, logPre() + "Session activated: " + getSessionId()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java index c35fcb3cf21..8a7be7ef176 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java @@ -17,7 +17,6 @@ import com.yahoo.transaction.AbstractTransaction; import com.yahoo.transaction.NestedTransaction; import com.yahoo.transaction.Transaction; import com.yahoo.vespa.config.server.GlobalComponentRegistry; -import com.yahoo.vespa.config.server.ReloadHandler; import com.yahoo.vespa.config.server.TimeoutBudget; import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.application.TenantApplications; @@ -73,8 +72,6 @@ public class SessionRepository { private static final FilenameFilter sessionApplicationsFilter = (dir, name) -> name.matches("\\d+"); private static final long nonExistingActiveSession = 0; - - private final SessionCache<LocalSession> localSessionCache = new SessionCache<>(); private final SessionCache<RemoteSession> remoteSessionCache = new SessionCache<>(); private final Map<Long, SessionStateWatcher> sessionStateWatchers = new HashMap<>(); @@ -84,7 +81,6 @@ public class SessionRepository { private final Executor zkWatcherExecutor; private final TenantFileSystemDirs tenantFileSystemDirs; private final BooleanFlag distributeApplicationPackage; - private final ReloadHandler reloadHandler; private final MetricUpdater metrics; private final Curator.DirectoryCache directoryCache; private final TenantApplications applicationRepo; @@ -97,7 +93,6 @@ public class SessionRepository { public SessionRepository(TenantName tenantName, GlobalComponentRegistry componentRegistry, TenantApplications applicationRepo, - ReloadHandler reloadHandler, FlagSource flagSource, SessionPreparer sessionPreparer) { this.tenantName = tenantName; @@ -111,7 +106,6 @@ public class SessionRepository { this.applicationRepo = applicationRepo; this.sessionPreparer = sessionPreparer; this.distributeApplicationPackage = Flags.CONFIGSERVER_DISTRIBUTE_APPLICATION_PACKAGE.bindTo(flagSource); - this.reloadHandler = reloadHandler; this.metrics = componentRegistry.getMetrics().getOrCreateMetricUpdater(Metrics.createDimensions(tenantName)); this.locksPath = TenantRepository.getLocksPath(tenantName); @@ -351,7 +345,7 @@ public class SessionRepository { for (ApplicationId applicationId : applicationRepo.activeApplications()) { if (applicationRepo.requireActiveSessionOf(applicationId) == session.getSessionId()) { log.log(Level.FINE, () -> "Found active application for session " + session.getSessionId() + " , loading it"); - reloadHandler.reloadConfig(session.ensureApplicationLoaded()); + applicationRepo.reloadConfig(session.ensureApplicationLoaded()); log.log(Level.INFO, session.logPre() + "Application activated successfully: " + applicationId + " (generation " + session.getSessionId() + ")"); return; } @@ -626,7 +620,7 @@ public class SessionRepository { if (sessionStateWatchers.containsKey(sessionId)) { localSession.ifPresent(session -> sessionStateWatchers.get(sessionId).addLocalSession(session)); } else { - sessionStateWatchers.put(sessionId, new SessionStateWatcher(fileCache, reloadHandler, remoteSession, + sessionStateWatchers.put(sessionId, new SessionStateWatcher(fileCache, applicationRepo, remoteSession, localSession, metrics, zkWatcherExecutor, this)); } } @@ -636,8 +630,6 @@ public class SessionRepository { return getLocalSessions().toString(); } - public ReloadHandler getReloadHandler() { return reloadHandler; } - /** Returns the lock for session operations for the given session id. */ public Lock lock(long sessionId) { return curator.lock(lockPath(sessionId), Duration.ofMinutes(1)); // These locks shouldn't be held for very long. diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java index 1cbab9be1a8..65c62a392b7 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java @@ -2,7 +2,7 @@ package com.yahoo.vespa.config.server.session; import com.yahoo.text.Utf8; -import com.yahoo.vespa.config.server.ReloadHandler; +import com.yahoo.vespa.config.server.application.TenantApplications; import com.yahoo.vespa.config.server.monitoring.MetricUpdater; import com.yahoo.vespa.curator.Curator; import org.apache.curator.framework.recipes.cache.ChildData; @@ -26,7 +26,7 @@ public class SessionStateWatcher { private static final Logger log = Logger.getLogger(SessionStateWatcher.class.getName()); private final Curator.FileCache fileCache; - private final ReloadHandler reloadHandler; + private final TenantApplications tenantApplications; private final RemoteSession remoteSession; private final MetricUpdater metrics; private final Executor zkWatcherExecutor; @@ -34,14 +34,14 @@ public class SessionStateWatcher { private Optional<LocalSession> localSession; SessionStateWatcher(Curator.FileCache fileCache, - ReloadHandler reloadHandler, + TenantApplications tenantApplications, RemoteSession remoteSession, Optional<LocalSession> localSession, MetricUpdater metrics, Executor zkWatcherExecutor, SessionRepository sessionRepository) { this.fileCache = fileCache; - this.reloadHandler = reloadHandler; + this.tenantApplications = tenantApplications; this.remoteSession = remoteSession; this.localSession = localSession; this.metrics = metrics; @@ -60,7 +60,7 @@ public class SessionStateWatcher { remoteSession.loadPrepared(); } else if (newStatus.equals(Status.ACTIVATE)) { createLocalSession(sessionId); - remoteSession.makeActive(reloadHandler); + remoteSession.makeActive(tenantApplications); } else if (newStatus.equals(Status.DEACTIVATE)) { remoteSession.deactivate(); } else if (newStatus.equals(Status.DELETE)) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java index 4316f03272a..33ce8f52834 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java @@ -9,8 +9,6 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.path.Path; import com.yahoo.vespa.config.server.GlobalComponentRegistry; -import com.yahoo.vespa.config.server.ReloadHandler; -import com.yahoo.vespa.config.server.RequestHandler; import com.yahoo.vespa.config.server.application.TenantApplications; import com.yahoo.vespa.config.server.deploy.TenantFileSystemDirs; import com.yahoo.vespa.config.server.monitoring.MetricUpdater; @@ -142,13 +140,8 @@ public class TenantRepository { } public synchronized void addTenant(TenantName tenantName) { - addTenant(tenantName, null, null); - } - - public synchronized void addTenant(TenantName tenantName, RequestHandler requestHandler, - ReloadHandler reloadHandler) { writeTenantPath(tenantName); - createTenant(tenantName, componentRegistry.getClock().instant(), requestHandler, reloadHandler); + createTenant(tenantName, componentRegistry.getClock().instant()); } private static Set<TenantName> readTenantsFromZooKeeper(Curator curator) { @@ -201,7 +194,7 @@ public class TenantRepository { // Use when bootstrapping an existing tenant based on ZooKeeper data protected void bootstrapTenant(TenantName tenantName) { - createTenant(tenantName, readCreatedTimeFromZooKeeper(tenantName), null, null); + createTenant(tenantName, readCreatedTimeFromZooKeeper(tenantName)); } public Instant readCreatedTimeFromZooKeeper(TenantName tenantName) { @@ -213,7 +206,7 @@ public class TenantRepository { } // Creates tenant and all its dependencies. This also includes loading active applications - private void createTenant(TenantName tenantName, Instant created, RequestHandler requestHandler, ReloadHandler reloadHandler) { + private void createTenant(TenantName tenantName, Instant created) { if (tenants.containsKey(tenantName)) return; TenantApplications applicationRepo = @@ -226,16 +219,13 @@ public class TenantRepository { componentRegistry.getConfigserverConfig(), componentRegistry.getHostRegistries().createApplicationHostRegistry(tenantName), new TenantFileSystemDirs(componentRegistry.getConfigServerDB(), tenantName)); - if (requestHandler == null) - requestHandler = applicationRepo; - if (reloadHandler == null) - reloadHandler = applicationRepo; - SessionRepository sessionRepository = new SessionRepository(tenantName, componentRegistry, - applicationRepo, reloadHandler, + SessionRepository sessionRepository = new SessionRepository(tenantName, + componentRegistry, + applicationRepo, componentRegistry.getFlagSource(), componentRegistry.getSessionPreparer()); log.log(Level.INFO, "Adding tenant '" + tenantName + "'" + ", created " + created); - Tenant tenant = new Tenant(tenantName, sessionRepository, requestHandler, applicationRepo, created); + Tenant tenant = new Tenant(tenantName, sessionRepository, applicationRepo, applicationRepo, created); notifyNewTenant(tenant); tenants.putIfAbsent(tenantName, tenant); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index c6d2c6b6438..a4b2bfb8902 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -31,17 +31,16 @@ import com.yahoo.vespa.config.protocol.ConfigResponse; import com.yahoo.vespa.config.protocol.DefContent; import com.yahoo.vespa.config.protocol.VespaVersion; import com.yahoo.vespa.config.server.application.OrchestratorMock; -import com.yahoo.vespa.config.server.application.TenantApplications; import com.yahoo.vespa.config.server.deploy.DeployTester; import com.yahoo.vespa.config.server.deploy.TenantFileSystemDirs; import com.yahoo.vespa.config.server.http.InternalServerException; import com.yahoo.vespa.config.server.http.SessionHandlerTest; import com.yahoo.vespa.config.server.http.v2.PrepareResult; import com.yahoo.vespa.config.server.session.LocalSession; -import com.yahoo.vespa.config.server.session.SessionRepository; import com.yahoo.vespa.config.server.session.PrepareParams; import com.yahoo.vespa.config.server.session.RemoteSession; import com.yahoo.vespa.config.server.session.Session; +import com.yahoo.vespa.config.server.session.SessionRepository; import com.yahoo.vespa.config.server.session.SilentDeployLogger; import com.yahoo.vespa.config.server.tenant.ApplicationRolesStore; import com.yahoo.vespa.config.server.tenant.Tenant; @@ -54,7 +53,6 @@ import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.model.VespaModelFactory; import org.hamcrest.core.Is; -import org.jetbrains.annotations.NotNull; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -134,7 +132,7 @@ public class ApplicationRepositoryTest { .flagSource(flagSource) .clock(clock) .build(); - tenantRepository = new TenantRepository(componentRegistry, false); + tenantRepository = new TenantRepository(componentRegistry); tenantRepository.addTenant(TenantRepository.HOSTED_VESPA_TENANT); tenantRepository.addTenant(tenant1); tenantRepository.addTenant(tenant2); @@ -573,8 +571,7 @@ public class ApplicationRepositoryTest { .vespaVersion(vespaVersion) .build()); - // TODO: Need to reload config before resolving works - RequestHandler requestHandler = reloadConfig(applicationId()); + RequestHandler requestHandler = getRequestHandler(applicationId()); SimpletypesConfig config = resolve(SimpletypesConfig.class, requestHandler, applicationId(), vespaVersion); assertEquals(1337 , config.intval()); } @@ -597,13 +594,11 @@ public class ApplicationRepositoryTest { .vespaVersion(vespaVersion) .build()); - // TODO: Need to reload config before resolving works - RequestHandler requestHandler = reloadConfig(applicationId()); + RequestHandler requestHandler = getRequestHandler(applicationId()); SimpletypesConfig config = resolve(SimpletypesConfig.class, requestHandler, applicationId(), vespaVersion); assertEquals(1337, config.intval()); - // TODO: Need to reload config before resolving works - RequestHandler requestHandler2 = reloadConfig(appId2); + RequestHandler requestHandler2 = getRequestHandler(appId2); SimpletypesConfig config2 = resolve(SimpletypesConfig.class, requestHandler2, appId2, vespaVersion); assertEquals(1330, config2.intval()); @@ -622,8 +617,7 @@ public class ApplicationRepositoryTest { .vespaVersion(vespaVersion) .build()); - // TODO: Need to reload config before resolving works - RequestHandler requestHandler = reloadConfig(applicationId()); + RequestHandler requestHandler = getRequestHandler(applicationId()); SimpletypesConfig config = resolve(SimpletypesConfig.class, requestHandler, applicationId(), vespaVersion); assertEquals(1337, config.intval()); @@ -640,8 +634,7 @@ public class ApplicationRepositoryTest { .vespaVersion(vespaVersion) .build()); - // TODO: Need to reload config before resolving works - RequestHandler requestHandler = reloadConfig(applicationId()); + RequestHandler requestHandler = getRequestHandler(applicationId()); SimpletypesConfig config = resolve(SimpletypesConfig.class, requestHandler, applicationId(), vespaVersion); assertEquals(1337 , config.intval()); @@ -770,11 +763,8 @@ public class ApplicationRepositoryTest { }, Optional.empty()); } - @NotNull - private RequestHandler reloadConfig(ApplicationId applicationId) { - RequestHandler requestHandler = tenantRepository.getTenant(applicationId.tenant()).getRequestHandler(); - ((TenantApplications) requestHandler).reloadConfig(applicationRepository.getActiveSession(applicationId).ensureApplicationLoaded()); - return requestHandler; + private RequestHandler getRequestHandler(ApplicationId applicationId) { + return tenantRepository.getTenant(applicationId.tenant()).getRequestHandler(); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/TestConfigDefinitionRepo.java b/configserver/src/test/java/com/yahoo/vespa/config/server/TestConfigDefinitionRepo.java index 5878b250bc8..ea093542354 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/TestConfigDefinitionRepo.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/TestConfigDefinitionRepo.java @@ -15,7 +15,9 @@ import java.util.Map; * @author Ulf Lilleengen */ public class TestConfigDefinitionRepo implements ConfigDefinitionRepo { + private final Map<ConfigDefinitionKey, ConfigDefinition> repo = new LinkedHashMap<>(); + public TestConfigDefinitionRepo() { repo.put(new ConfigDefinitionKey(SimpletypesConfig.CONFIG_DEF_NAME, SimpletypesConfig.CONFIG_DEF_NAMESPACE), new ConfigDefinition(SimpletypesConfig.CONFIG_DEF_NAME, SimpletypesConfig.CONFIG_DEF_SCHEMA)); @@ -32,4 +34,5 @@ public class TestConfigDefinitionRepo implements ConfigDefinitionRepo { @Override public ConfigDefinition get(ConfigDefinitionKey key) { return null; } + } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java index 32ce2c6f509..237a6547057 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java @@ -1,62 +1,84 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.http; -import com.yahoo.config.SimpletypesConfig; +import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.ConfigPayload; -import com.yahoo.vespa.config.protocol.SlimeConfigResponse; -import com.yahoo.vespa.config.server.rpc.MockRequestHandler; +import com.yahoo.vespa.config.server.ApplicationRepository; +import com.yahoo.vespa.config.server.TestComponentRegistry; +import com.yahoo.vespa.config.server.TestConfigDefinitionRepo; +import com.yahoo.vespa.config.server.application.OrchestratorMock; +import com.yahoo.vespa.config.server.session.PrepareParams; +import com.yahoo.vespa.config.server.tenant.TenantRepository; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import java.io.File; import java.io.IOException; +import java.time.Clock; import java.util.Collections; -import java.util.HashSet; import static com.yahoo.jdisc.http.HttpRequest.Method.GET; import static com.yahoo.jdisc.http.HttpResponse.Status.BAD_REQUEST; import static com.yahoo.jdisc.http.HttpResponse.Status.NOT_FOUND; -import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen + * @author hmusum */ public class HttpGetConfigHandlerTest { - private static final String configUri = "http://yahoo.com:8080/config/v1/foo.bar/myid"; - private MockRequestHandler mockRequestHandler; + private static final TenantName tenant = TenantName.from("default"); + private static final String expected = + "{\"port\":{\"telnet\":19098,\"rpc\":19097},\"application\":{\"tenant\":\"default\",\"name\":\"default\""; + private static final String baseUri = "http://yahoo.com:8080/config/v1/"; + private static final String configUri = baseUri + "cloud.config.sentinel/hosts/localhost/sentinel"; + private final static File testApp = new File("src/test/resources/deploy/validapp"); + private static final ApplicationId applicationId = ApplicationId.defaultId(); + private HttpGetConfigHandler handler; + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Before - public void setUp() { - mockRequestHandler = new MockRequestHandler(ApplicationId.defaultId()); - mockRequestHandler.setAllConfigs(new HashSet<>() {{ - add(new ConfigKey<>("bar", "myid", "foo")); - }} ); - handler = new HttpGetConfigHandler(HttpGetConfigHandler.testOnlyContext(), mockRequestHandler); + public void setUp() throws IOException { + TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder() + .configDefinitionRepo(new TestConfigDefinitionRepo()) + .configServerConfig(new ConfigserverConfig.Builder() + .configServerDBDir(temporaryFolder.newFolder().getAbsolutePath()) + .configDefinitionsDir(temporaryFolder.newFolder().getAbsolutePath()) + .build()) + .build(); + TenantRepository tenantRepository = new TenantRepository(componentRegistry); + tenantRepository.addTenant(tenant); + ApplicationRepository applicationRepository = + new ApplicationRepository(tenantRepository, + new SessionHandlerTest.MockProvisioner(), + new OrchestratorMock(), + Clock.systemUTC()); + handler = new HttpGetConfigHandler(HttpGetConfigHandler.testOnlyContext(), tenantRepository); + applicationRepository.deploy(testApp, prepareParams()); } @Test public void require_that_handler_can_be_created() throws IOException { - // Define config response for mock handler - final long generation = 1L; - ConfigPayload payload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); - mockRequestHandler.responses.put(ApplicationId.defaultId(), SlimeConfigResponse.fromConfigPayload(payload, generation, false, "mymd5")); HttpResponse response = handler.handle(HttpRequest.createTestRequest(configUri, GET)); - assertThat(SessionHandlerTest.getRenderedString(response), is("{\"boolval\":false,\"doubleval\":0.0,\"enumval\":\"VAL1\",\"intval\":0,\"longval\":0,\"stringval\":\"s\"}")); + String renderedString = SessionHandlerTest.getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith(expected)); } @Test public void require_correct_error_response() throws IOException { - final String nonExistingConfigNameUri = "http://yahoo.com:8080/config/v1/nonexisting.config/myid"; - final String nonExistingConfigUri = "http://yahoo.com:8080/config/v1/foo.bar/myid/nonexisting/id"; - final String illegalConfigNameUri = "http://yahoo.com:8080/config/v1/foobar/myid"; + final String nonExistingConfigNameUri = baseUri + "nonexisting.config/myid"; + final String nonExistingConfigUri = baseUri + "cloud.config.sentinel/myid/nonexisting/id"; + final String illegalConfigNameUri = baseUri + "/foobar/myid"; HttpResponse response = handler.handle(HttpRequest.createTestRequest(nonExistingConfigNameUri, GET)); HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(response, NOT_FOUND, HttpErrorResponse.errorCodes.NOT_FOUND, "No such config: nonexisting.config"); @@ -71,12 +93,14 @@ public class HttpGetConfigHandlerTest { @Test public void require_that_nocache_property_works() throws IOException { - long generation = 1L; - ConfigPayload payload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); - mockRequestHandler.responses.put(ApplicationId.defaultId(), SlimeConfigResponse.fromConfigPayload(payload, generation, false, "mymd5")); - final HttpRequest request = HttpRequest.createTestRequest(configUri, GET, null, Collections.singletonMap("nocache", "true")); + HttpRequest request = HttpRequest.createTestRequest(configUri, GET, null, Collections.singletonMap("nocache", "true")); HttpResponse response = handler.handle(request); - assertThat(SessionHandlerTest.getRenderedString(response), is("{\"boolval\":false,\"doubleval\":0.0,\"enumval\":\"VAL1\",\"intval\":0,\"longval\":0,\"stringval\":\"s\"}")); + String renderedString = SessionHandlerTest.getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith(expected)); + } + + private PrepareParams prepareParams() { + return new PrepareParams.Builder().applicationId(applicationId).build(); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java index dea9196c949..86ea4309839 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java @@ -1,57 +1,96 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.http; +import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.server.rpc.MockRequestHandler; +import com.yahoo.vespa.config.server.ApplicationRepository; +import com.yahoo.vespa.config.server.TestComponentRegistry; +import com.yahoo.vespa.config.server.TestConfigDefinitionRepo; +import com.yahoo.vespa.config.server.application.OrchestratorMock; import com.yahoo.vespa.config.server.http.HttpListConfigsHandler.ListConfigsResponse; - +import com.yahoo.vespa.config.server.session.PrepareParams; +import com.yahoo.vespa.config.server.tenant.TenantRepository; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import java.io.File; import java.io.IOException; -import java.util.*; - -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.*; +import java.time.Clock; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; -import static com.yahoo.jdisc.http.HttpResponse.Status.*; +import static com.yahoo.container.jdisc.HttpRequest.createTestRequest; import static com.yahoo.jdisc.http.HttpRequest.Method.GET; +import static com.yahoo.jdisc.http.HttpResponse.Status.BAD_REQUEST; +import static com.yahoo.jdisc.http.HttpResponse.Status.NOT_FOUND; +import static com.yahoo.vespa.config.server.http.SessionHandlerTest.getRenderedString; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen + * @author hmusum */ public class HttpListConfigsHandlerTest { - - private MockRequestHandler mockRequestHandler; + + private static final TenantName tenant = TenantName.from("default"); + private static final String baseUri = "http://foo.com:8080/config/v1/"; + private final static File testApp = new File("src/test/resources/deploy/validapp"); + private static final ApplicationId applicationId = ApplicationId.defaultId(); + private HttpListConfigsHandler handler; private HttpListNamedConfigsHandler namedHandler; + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Before - public void setUp() { - mockRequestHandler = new MockRequestHandler(ApplicationId.defaultId()); - mockRequestHandler.setAllConfigs(new HashSet<>() {{ - add(new ConfigKey<>("bar", "conf/id/", "foo")); - }} ); + public void setUp() throws IOException { + TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder() + .configDefinitionRepo(new TestConfigDefinitionRepo()) + .configServerConfig(new ConfigserverConfig.Builder() + .configServerDBDir(temporaryFolder.newFolder().getAbsolutePath()) + .configDefinitionsDir(temporaryFolder.newFolder().getAbsolutePath()) + .build()) + .build(); + TenantRepository tenantRepository = new TenantRepository(componentRegistry); + tenantRepository.addTenant(tenant); + ApplicationRepository applicationRepository = + new ApplicationRepository(tenantRepository, + new SessionHandlerTest.MockProvisioner(), + new OrchestratorMock(), + Clock.systemUTC()); + applicationRepository.deploy(testApp, prepareParams()); + HttpListConfigsHandler.Context ctx = HttpListConfigsHandler.testOnlyContext(); - handler = new HttpListConfigsHandler(ctx, mockRequestHandler); - namedHandler = new HttpListNamedConfigsHandler(ctx, mockRequestHandler); + handler = new HttpListConfigsHandler(ctx, tenantRepository); + namedHandler = new HttpListNamedConfigsHandler(ctx, tenantRepository); } @Test public void require_that_handler_can_be_created() throws IOException { HttpResponse response = handler.handle(HttpRequest.createTestRequest("/config/v1/", GET)); - assertThat(SessionHandlerTest.getRenderedString(response), is("{\"children\":[],\"configs\":[]}")); + String renderedString = getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith("{\"children\":[")); + assertTrue(renderedString, renderedString.contains(",\"configs\":[")); } @Test public void require_that_named_handler_can_be_created() throws IOException { - HttpRequest req = HttpRequest.createTestRequest("http://foo.com:8080/config/v1/foo.bar/conf/id/", GET); - req.getJDiscRequest().parameters().put("http.path", List.of("foo.bar")); + HttpRequest req = createTestRequest(baseUri + "cloud.config.sentinel/hosts/localhost/sentinel", GET); + req.getJDiscRequest().parameters().put("http.path", List.of("cloud.config.sentinel")); HttpResponse response = namedHandler.handle(req); - assertThat(SessionHandlerTest.getRenderedString(response), is("{\"children\":[],\"configs\":[]}")); + String renderedString = getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith("{\"children\":[")); + assertTrue(renderedString, renderedString.contains(",\"configs\":[")); } @Test @@ -76,29 +115,23 @@ public class HttpListConfigsHandlerTest { assertEquals(resp.toUrl(new ConfigKey<>("myconfig", "my/id", "mynamespace"), true), "http://foo.com/config/v1/mynamespace.myconfig/my/id"); assertEquals(resp.toUrl(new ConfigKey<>("myconfig", "my/id", "mynamespace"), false), "http://foo.com/config/v1/mynamespace.myconfig/my/id/"); assertEquals(resp.getContentType(), "application/json"); - } @Test public void require_error_on_bad_request() throws IOException { - HttpRequest req = HttpRequest.createTestRequest("http://foo.com:8080/config/v1/foobar/conf/id/", GET); + HttpRequest req = createTestRequest(baseUri + "foobar/hosts/localhost/sentinel/", GET); HttpResponse resp = namedHandler.handle(req); HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(resp, BAD_REQUEST, HttpErrorResponse.errorCodes.BAD_REQUEST, "Illegal config, must be of form namespace.name."); - req = HttpRequest.createTestRequest("http://foo.com:8080/config/v1/foo.barNOPE/conf/id/", GET); + req = createTestRequest(baseUri + "foo.barNOPE/conf/id/", GET); resp = namedHandler.handle(req); HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(resp, NOT_FOUND, HttpErrorResponse.errorCodes.NOT_FOUND, "No such config: foo.barNOPE"); - req = HttpRequest.createTestRequest("http://foo.com:8080/config/v1/foo.bar/conf/id/NOPE/", GET); + req = createTestRequest(baseUri + "cloud.config.sentinel/conf/id/NOPE/", GET); resp = namedHandler.handle(req); - HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(resp, NOT_FOUND, HttpErrorResponse.errorCodes.NOT_FOUND, "No such config id: conf/id/NOPE/"); + HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(resp, NOT_FOUND, HttpErrorResponse.errorCodes.NOT_FOUND, "No such config id: conf/id/NOPE"); } - - @Test - public void require_correct_error_response_on_no_model() throws IOException { - mockRequestHandler.setAllConfigs(new HashSet<>()); - HttpResponse response = namedHandler.handle(HttpRequest.createTestRequest("http://yahoo.com:8080/config/v1/foo.bar/myid/", GET)); - HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(response, NOT_FOUND, - HttpErrorResponse.errorCodes.NOT_FOUND, - "Config not available, verify that an application package has been deployed and activated."); + + private PrepareParams prepareParams() { + return new PrepareParams.Builder().applicationId(applicationId).build(); } - + } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java index 97789caeb4b..a1ec73c44dc 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java @@ -1,27 +1,32 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.http.v2; -import com.yahoo.config.SimpletypesConfig; +import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.ConfigPayload; -import com.yahoo.vespa.config.protocol.SlimeConfigResponse; +import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.TestComponentRegistry; +import com.yahoo.vespa.config.server.TestConfigDefinitionRepo; +import com.yahoo.vespa.config.server.application.OrchestratorMock; import com.yahoo.vespa.config.server.http.HandlerTest; import com.yahoo.vespa.config.server.http.HttpConfigRequest; import com.yahoo.vespa.config.server.http.HttpErrorResponse; import com.yahoo.vespa.config.server.http.SessionHandlerTest; -import com.yahoo.vespa.config.server.rpc.MockRequestHandler; +import com.yahoo.vespa.config.server.session.PrepareParams; import com.yahoo.vespa.config.server.tenant.TenantRepository; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import java.io.File; import java.io.IOException; +import java.time.Clock; import java.util.Collections; -import java.util.HashSet; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; import static com.yahoo.jdisc.Response.Status.NOT_FOUND; @@ -31,54 +36,62 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +/** + * @author hmusum + */ public class HttpGetConfigHandlerTest { private static final TenantName tenant = TenantName.from("mytenant"); - private static final String EXPECTED_RENDERED_STRING = "{\"boolval\":false,\"doubleval\":0.0,\"enumval\":\"VAL1\",\"intval\":0,\"longval\":0,\"stringval\":\"s\"}"; - private static final String configUri = "http://yahoo.com:8080/config/v2/tenant/" + tenant.value() + "/application/myapplication/foo.bar/myid"; - private MockRequestHandler mockRequestHandler; + private static final ApplicationName applicationName = ApplicationName.from("myapplication"); + private static final String expected = + "{\"port\":{\"telnet\":19098,\"rpc\":19097},\"application\":{\"tenant\":\"mytenant\",\"name\":\"myapplication\""; + private static final String baseUri = "http://yahoo.com:8080/config/v2/tenant/mytenant/application/myapplication/"; + private static final String configUri = baseUri + "cloud.config.sentinel/hosts/localhost/sentinel"; + private final static File testApp = new File("src/test/resources/deploy/validapp"); + private static final ApplicationId applicationId = ApplicationId.from(tenant, applicationName, InstanceName.defaultName()); + private HttpGetConfigHandler handler; + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Before - public void setUp() { - mockRequestHandler = new MockRequestHandler(ApplicationId.defaultId()); - mockRequestHandler.setAllConfigs(new HashSet<>() {{ - add(new ConfigKey<>("bar", "myid", "foo")); - }} ); - TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder().build(); - TenantRepository tenantRepository = new TenantRepository(componentRegistry, false); - tenantRepository.addTenant(tenant, mockRequestHandler, mockRequestHandler); + public void setUp() throws IOException { + TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder() + .configDefinitionRepo(new TestConfigDefinitionRepo()) + .configServerConfig(new ConfigserverConfig.Builder() + .configServerDBDir(temporaryFolder.newFolder().getAbsolutePath()) + .configDefinitionsDir(temporaryFolder.newFolder().getAbsolutePath()) + .build()) + .build(); + TenantRepository tenantRepository = new TenantRepository(componentRegistry); + tenantRepository.addTenant(tenant); + ApplicationRepository applicationRepository = + new ApplicationRepository(tenantRepository, + new SessionHandlerTest.MockProvisioner(), + new OrchestratorMock(), + Clock.systemUTC()); handler = new HttpGetConfigHandler(HttpGetConfigHandler.testOnlyContext(), tenantRepository); + applicationRepository.deploy(testApp, prepareParams()); } @Test public void require_that_handler_can_be_created() throws IOException { - // Define config response for mock handler - final long generation = 1L; - ConfigPayload payload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); - mockRequestHandler.responses.put(new ApplicationId.Builder().tenant(tenant).applicationName("myapplication").build(), - SlimeConfigResponse.fromConfigPayload(payload, generation, false, "mymd5")); HttpResponse response = handler.handle(HttpRequest.createTestRequest(configUri, GET)); - assertThat(SessionHandlerTest.getRenderedString(response), is(EXPECTED_RENDERED_STRING)); + assertTrue(SessionHandlerTest.getRenderedString(response).startsWith(expected)); } @Test public void require_that_handler_can_handle_long_appid_request_with_configid() throws IOException { - String uriLongAppId = "http://yahoo.com:8080/config/v2/tenant/" + tenant.value() + - "/application/myapplication/environment/staging/region/myregion/instance/myinstance/foo.bar/myid"; - final long generation = 1L; - ConfigPayload payload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); - mockRequestHandler.responses.put(new ApplicationId.Builder() - .tenant(tenant) - .applicationName("myapplication").instanceName("myinstance").build(), - SlimeConfigResponse.fromConfigPayload(payload, generation, false, "mymd5")); - HttpResponse response = handler.handle(HttpRequest.createTestRequest(uriLongAppId, GET)); - assertThat(SessionHandlerTest.getRenderedString(response), is(EXPECTED_RENDERED_STRING)); + String configUri = baseUri + "/environment/staging/region/myregion/instance/default/cloud.config.sentinel/hosts/localhost/sentinel"; + HttpResponse response = handler.handle(HttpRequest.createTestRequest(configUri, GET)); + String renderedString = SessionHandlerTest.getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith(expected)); } @Test public void require_that_request_gets_correct_fields_with_full_appid() { - String uriLongAppId = "http://yahoo.com:8080/config/v2/tenant/bill/application/sookie/environment/dev/region/bellefleur/instance/sam/foo.bar/myid"; + String uriLongAppId = "http://foo.com:8080/config/v2/tenant/bill/application/sookie/environment/dev/region/bellefleur/instance/sam/foo.bar/myid"; HttpRequest r = HttpRequest.createTestRequest(uriLongAppId, GET); HttpConfigRequest req = HttpConfigRequest.createFromRequestV2(r); assertThat(req.getApplicationId().tenant().value(), is("bill")); @@ -88,7 +101,7 @@ public class HttpGetConfigHandlerTest { @Test public void require_that_request_gets_correct_fields_with_short_appid() { - String uriShortAppId = "http://yahoo.com:8080/config/v2/tenant/jason/application/alcide/foo.bar/myid"; + String uriShortAppId = "http://foo.com:8080/config/v2/tenant/jason/application/alcide/foo.bar/myid"; HttpRequest r = HttpRequest.createTestRequest(uriShortAppId, GET); HttpConfigRequest req = HttpConfigRequest.createFromRequestV2(r); assertThat(req.getApplicationId().tenant().value(), is("jason")); @@ -98,9 +111,9 @@ public class HttpGetConfigHandlerTest { @Test public void require_correct_error_response() throws IOException { - final String nonExistingConfigNameUri = "http://yahoo.com:8080/config/v2/tenant/mytenant/application/myapplication/nonexisting.config/myid"; - final String nonExistingConfigUri = "http://yahoo.com:8080/config/v2/tenant/mytenant/application/myapplication//foo.bar/myid/nonexisting/id"; - final String illegalConfigNameUri = "http://yahoo.com:8080/config/v2/tenant/mytenant/application/myapplication//foobar/myid"; + String nonExistingConfigNameUri = baseUri + "nonexisting.config/myid"; + String nonExistingConfigUri = baseUri + "cloud.config.sentinel/myid/nonexisting/id"; + String illegalConfigNameUri = baseUri + "/foobar/myid"; HttpResponse response = handler.handle(HttpRequest.createTestRequest(nonExistingConfigNameUri, GET)); HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(response, NOT_FOUND, HttpErrorResponse.errorCodes.NOT_FOUND, "No such config: nonexisting.config"); @@ -115,13 +128,14 @@ public class HttpGetConfigHandlerTest { @Test public void require_that_nocache_property_works() throws IOException { - long generation = 1L; - ConfigPayload payload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); - mockRequestHandler.responses.put(new ApplicationId.Builder().tenant(tenant).applicationName("myapplication").build(), - SlimeConfigResponse.fromConfigPayload(payload, generation, false, "mymd5")); - final HttpRequest request = HttpRequest.createTestRequest(configUri, GET, null, Collections.singletonMap("nocache", "true")); + HttpRequest request = HttpRequest.createTestRequest(configUri, GET, null, Collections.singletonMap("nocache", "true")); HttpResponse response = handler.handle(request); - assertThat(SessionHandlerTest.getRenderedString(response), is(EXPECTED_RENDERED_STRING)); + String renderedString = SessionHandlerTest.getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith(expected)); + } + + private PrepareParams prepareParams() { + return new PrepareParams.Builder().applicationId(applicationId).build(); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpListConfigsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpListConfigsHandlerTest.java index d91d41173b2..64fc1d118c2 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpListConfigsHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpListConfigsHandlerTest.java @@ -1,52 +1,82 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.http.v2; +import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.vespa.config.ConfigKey; +import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.TestComponentRegistry; -import com.yahoo.vespa.config.server.rpc.MockRequestHandler; -import com.yahoo.vespa.config.server.tenant.TenantRepository; +import com.yahoo.vespa.config.server.TestConfigDefinitionRepo; +import com.yahoo.vespa.config.server.application.OrchestratorMock; import com.yahoo.vespa.config.server.http.HandlerTest; import com.yahoo.vespa.config.server.http.HttpErrorResponse; import com.yahoo.vespa.config.server.http.SessionHandlerTest; import com.yahoo.vespa.config.server.http.v2.HttpListConfigsHandler.ListConfigsResponse; - +import com.yahoo.vespa.config.server.session.PrepareParams; +import com.yahoo.vespa.config.server.tenant.TenantRepository; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import java.io.File; import java.io.IOException; -import java.util.*; - -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.*; +import java.time.Clock; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; -import static com.yahoo.jdisc.http.HttpResponse.Status.*; +import static com.yahoo.container.jdisc.HttpRequest.createTestRequest; import static com.yahoo.jdisc.http.HttpRequest.Method.GET; +import static com.yahoo.jdisc.http.HttpResponse.Status.BAD_REQUEST; +import static com.yahoo.jdisc.http.HttpResponse.Status.NOT_FOUND; +import static com.yahoo.vespa.config.server.http.SessionHandlerTest.getRenderedString; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen + * @author hmusum */ public class HttpListConfigsHandlerTest { - private final TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder().build(); + private static final TenantName tenant = TenantName.from("mytenant"); + private static final ApplicationName applicationName = ApplicationName.from("myapplication"); + private static final String baseUri = "http://foo.com:8080/config/v2/tenant/mytenant/application/myapplication/"; + private final static File testApp = new File("src/test/resources/deploy/validapp"); + private static final ApplicationId applicationId = ApplicationId.from(tenant, applicationName, InstanceName.defaultName()); - private MockRequestHandler mockRequestHandler; private HttpListConfigsHandler handler; private HttpListNamedConfigsHandler namedHandler; + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Before - public void setUp() { - mockRequestHandler = new MockRequestHandler(ApplicationId.defaultId()); - mockRequestHandler.setAllConfigs(new HashSet<>() {{ - add(new ConfigKey<>("bar", "conf/id", "foo")); - }} ); - TenantName tenantName = TenantName.from("mytenant"); - TenantRepository tenantRepository = new TenantRepository(componentRegistry, false); - tenantRepository.addTenant(tenantName, mockRequestHandler, mockRequestHandler); + public void setUp() throws IOException { + TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder() + .configDefinitionRepo(new TestConfigDefinitionRepo()) + .configServerConfig(new ConfigserverConfig.Builder() + .configServerDBDir(temporaryFolder.newFolder().getAbsolutePath()) + .configDefinitionsDir(temporaryFolder.newFolder().getAbsolutePath()) + .build()) + .build(); + TenantRepository tenantRepository = new TenantRepository(componentRegistry); + tenantRepository.addTenant(tenant); + ApplicationRepository applicationRepository = + new ApplicationRepository(tenantRepository, + new SessionHandlerTest.MockProvisioner(), + new OrchestratorMock(), + Clock.systemUTC()); + applicationRepository.deploy(testApp, prepareParams()); handler = new HttpListConfigsHandler(HttpListConfigsHandler.testOnlyContext(), tenantRepository, Zone.defaultZone()); @@ -57,31 +87,41 @@ public class HttpListConfigsHandlerTest { @Test public void require_that_handler_can_be_created() throws IOException { - HttpResponse response = handler.handle(HttpRequest.createTestRequest("http://yahoo.com:8080/config/v2/tenant/mytenant/application/myapplication/", GET)); - assertThat(SessionHandlerTest.getRenderedString(response), is("{\"children\":[],\"configs\":[]}")); + HttpResponse response = handler.handle(createTestRequest(baseUri, GET)); + String renderedString = getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith("{\"children\":[")); + assertTrue(renderedString, renderedString.contains(",\"configs\":[")); } - + @Test public void require_that_request_can_be_created_from_full_appid() throws IOException { - HttpResponse response = handler.handle(HttpRequest.createTestRequest( - "http://yahoo.com:8080/config/v2/tenant/mytenant/application/myapplication/environment/test/region/myregion/instance/myinstance/", GET)); - assertThat(SessionHandlerTest.getRenderedString(response), is("{\"children\":[],\"configs\":[]}")); + HttpResponse response = handler.handle( + createTestRequest(baseUri + + "environment/test/region/myregion/instance/default/", GET)); + String renderedString = getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith("{\"children\":[")); + assertTrue(renderedString, renderedString.contains(",\"configs\":[")); } @Test public void require_that_named_handler_can_be_created() throws IOException { - HttpRequest req = HttpRequest.createTestRequest("http://foo.com:8080/config/v2/tenant/mytenant/application/myapplication/foo.bar/conf/id/", GET); - req.getJDiscRequest().parameters().put("http.path", List.of("foo.bar")); + HttpRequest req = createTestRequest(baseUri + "cloud.config.sentinel/hosts/localhost/sentinel/", GET); + req.getJDiscRequest().parameters().put("http.path", List.of("cloud.config.sentinel")); HttpResponse response = namedHandler.handle(req); - assertThat(SessionHandlerTest.getRenderedString(response), is("{\"children\":[],\"configs\":[]}")); + String renderedString = getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith("{\"children\":[")); + assertTrue(renderedString, renderedString.contains(",\"configs\":[")); } @Test public void require_that_named_handler_can_be_created_from_full_appid() throws IOException { - HttpRequest req = HttpRequest.createTestRequest("http://foo.com:8080/config/v2/tenant/mytenant/application/myapplication/environment/prod/region/myregion/instance/myinstance/foo.bar/conf/id/", GET); + HttpRequest req = createTestRequest(baseUri + + "environment/prod/region/myregion/instance/default/cloud.config.sentinel/hosts/localhost/sentinel/", GET); req.getJDiscRequest().parameters().put("http.path", List.of("foo.bar")); HttpResponse response = namedHandler.handle(req); - assertThat(SessionHandlerTest.getRenderedString(response), is("{\"children\":[],\"configs\":[]}")); + String renderedString = getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith("{\"children\":[")); + assertTrue(renderedString, renderedString.contains(",\"configs\":[")); } @Test @@ -108,31 +148,21 @@ public class HttpListConfigsHandlerTest { assertEquals(resp.toUrl(new ConfigKey<>("myconfig", "", "mynamespace"), false), "http://foo.com/config/v2/tenant/mytenant/application/mya/mynamespace.myconfig"); assertEquals(resp.toUrl(new ConfigKey<>("myconfig", "", "mynamespace"), true), "http://foo.com/config/v2/tenant/mytenant/application/mya/mynamespace.myconfig"); assertEquals(resp.getContentType(), "application/json"); - } @Test public void require_error_on_bad_request() throws IOException { - HttpRequest req = HttpRequest.createTestRequest("http://foo.com:8080/config/v2/tenant/mytenant/application/myapplication/foobar/conf/id/", GET); + HttpRequest req = createTestRequest(baseUri + "foobar/hosts/localhost/sentinel/", GET); HttpResponse resp = namedHandler.handle(req); HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(resp, BAD_REQUEST, HttpErrorResponse.errorCodes.BAD_REQUEST, "Illegal config, must be of form namespace.name."); - req = HttpRequest.createTestRequest("http://foo.com:8080/config/v2/tenant/mytenant/application/myapplication/foo.barNOPE/conf/id/", GET); + req = createTestRequest(baseUri + "foo.barNOPE/conf/id/", GET); resp = namedHandler.handle(req); HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(resp, NOT_FOUND, HttpErrorResponse.errorCodes.NOT_FOUND, "No such config: foo.barNOPE"); - req = HttpRequest.createTestRequest("http://foo.com:8080/config/v2/tenant/mytenant/application/myapplication/foo.bar/conf/id/NOPE/", GET); + req = createTestRequest(baseUri + "cloud.config.sentinel/conf/id/NOPE/", GET); resp = namedHandler.handle(req); HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(resp, NOT_FOUND, HttpErrorResponse.errorCodes.NOT_FOUND, "No such config id: conf/id/NOPE"); } - - @Test - public void require_correct_error_response_on_no_model() throws IOException { - mockRequestHandler.setAllConfigs(new HashSet<>()); - HttpResponse response = namedHandler.handle(HttpRequest.createTestRequest("http://yahoo.com:8080/config/v2/tenant/mytenant/application/myapplication/foo.bar/myid/", GET)); - HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(response, NOT_FOUND, - HttpErrorResponse.errorCodes.NOT_FOUND, - "Config not available, verify that an application package has been deployed and activated."); - } - + @Test public void require_correct_configid_parent() { assertNull(ListConfigsResponse.parentConfigId(null)); @@ -142,6 +172,10 @@ public class HttpListConfigsHandlerTest { assertEquals(ListConfigsResponse.parentConfigId("foo/bar"), "foo"); assertEquals(ListConfigsResponse.parentConfigId("foo/bar/baz"), "foo/bar"); assertEquals(ListConfigsResponse.parentConfigId("foo/bar/"), "foo/bar"); + } + private PrepareParams prepareParams() { + return new PrepareParams.Builder().applicationId(applicationId).build(); } + } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java index 2eccf6b5643..6fa4a421d5a 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java @@ -32,7 +32,7 @@ public class ConfigResponseFactoryTest { @Test public void testUncompressedFactory() { UncompressedConfigResponseFactory responseFactory = new UncompressedConfigResponseFactory(); - ConfigResponse response = responseFactory.createResponse(ConfigPayload.empty(), def, 3, false); + ConfigResponse response = responseFactory.createResponse(ConfigPayload.empty(), 3, false); assertEquals(CompressionType.UNCOMPRESSED, response.getCompressionInfo().getCompressionType()); assertEquals(3L,response.getGeneration()); assertEquals(2, response.getPayload().getByteLength()); @@ -41,7 +41,7 @@ public class ConfigResponseFactoryTest { @Test public void testLZ4CompressedFactory() { LZ4ConfigResponseFactory responseFactory = new LZ4ConfigResponseFactory(); - ConfigResponse response = responseFactory.createResponse(ConfigPayload.empty(), def, 3, false); + ConfigResponse response = responseFactory.createResponse(ConfigPayload.empty(), 3, false); assertEquals(CompressionType.LZ4, response.getCompressionInfo().getCompressionType()); assertEquals(3L, response.getGeneration()); assertEquals(3, response.getPayload().getByteLength()); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java deleted file mode 100644 index e800faf78a4..00000000000 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.rpc; - -import com.yahoo.config.FileReference; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.component.Version; -import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.GetConfigRequest; -import com.yahoo.vespa.config.protocol.ConfigResponse; -import com.yahoo.vespa.config.server.application.ApplicationSet; -import com.yahoo.vespa.config.server.ReloadHandler; -import com.yahoo.vespa.config.server.RequestHandler; - -import java.util.Collections; -import java.util.HashSet; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.Optional; -import java.util.Set; - -/** - * Test utility class - * - * @author Ulf Lilleengen - */ -public class MockRequestHandler implements RequestHandler, ReloadHandler { - - private Set<ConfigKey<?>> allConfigs = new HashSet<>(); - public Map<ApplicationId, ConfigResponse> responses = new LinkedHashMap<>(); - private final ApplicationId applicationId; - - public MockRequestHandler(ApplicationId applicationId) { - this.applicationId = applicationId; - } - - @Override - public ConfigResponse resolveConfig(ApplicationId appId, GetConfigRequest req, Optional<Version> vespaVersion) { - return responses.get(appId); - } - - @Override - public Set<ConfigKey<?>> listConfigs(ApplicationId appId, Optional<Version> vespaVersion, boolean recursive) { - return Collections.emptySet(); - } - - @Override - public void removeApplication(ApplicationId applicationId) { } - - @Override - public void removeApplicationsExcept(Set<ApplicationId> applicationIds) { } - - @Override - public void reloadConfig(ApplicationSet application) { } - - @Override - public Set<ConfigKey<?>> listNamedConfigs(ApplicationId appId, Optional<Version> vespaVersion, ConfigKey<?> key, boolean recursive) { - return Collections.emptySet(); - } - - @Override - public Set<String> allConfigIds(ApplicationId appId, Optional<Version> vespaVersion) { - Set<String> ret = new HashSet<>(); - for (ConfigKey<?> k : allConfigs) { - ret.add(k.getConfigId()); - } - return ret; - } - - @Override - public Set<ConfigKey<?>> allConfigsProduced(ApplicationId appId, Optional<Version> vespaVersion) { - return allConfigs; - } - - public void setAllConfigs(Set<ConfigKey<?>> allConfigs) { - this.allConfigs = allConfigs; - } - - @Override - public boolean hasApplication(ApplicationId appId, Optional<Version> vespaVersion) { - return responses.containsKey(appId); - } - - @Override - public ApplicationId resolveApplicationId(String hostName) { - return applicationId; - } - - @Override - public Set<FileReference> listFileReferences(ApplicationId applicationId) { - return Set.of(); - } - - public RequestHandler getRequestHandler() { return this; } - -} diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java index 8d0285ac12b..f372b21b065 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java @@ -15,6 +15,7 @@ import com.yahoo.vespa.config.server.ServerCache; import com.yahoo.vespa.config.server.TestComponentRegistry; import com.yahoo.vespa.config.server.application.Application; import com.yahoo.vespa.config.server.application.ApplicationSet; +import com.yahoo.vespa.config.server.application.TenantApplications; import com.yahoo.vespa.config.server.application.TenantApplicationsTest; import com.yahoo.vespa.config.server.monitoring.MetricUpdater; import com.yahoo.vespa.curator.Curator; @@ -82,10 +83,10 @@ public class TenantRepositoryTest { @Test public void testListenersAdded() throws IOException, SAXException { - Tenant tenant = tenantRepository.getTenant(tenant1); - tenant.getApplicationRepo().createApplication(ApplicationId.defaultId()); - tenant.getApplicationRepo().createPutTransaction(ApplicationId.defaultId(), 4).commit(); - tenant.getSessionRepository().getReloadHandler().reloadConfig(ApplicationSet.fromSingle( + TenantApplications applicationRepo = tenantRepository.getTenant(tenant1).getApplicationRepo(); + applicationRepo.createApplication(ApplicationId.defaultId()); + applicationRepo.createPutTransaction(ApplicationId.defaultId(), 4).commit(); + applicationRepo.reloadConfig(ApplicationSet.fromSingle( new Application(new VespaModel(MockApplicationPackage.createEmpty()), new ServerCache(), 4L, diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index dac33d2d431..aa2e5ccfa5f 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -36,7 +36,7 @@ ], "methods": [ "public void <init>()", - "public void setContainerHasClusters(boolean)", + "public void setClusters(java.util.Set)", "public void setReceiveTrafficByDefault(boolean)", "public void setUp(java.lang.Object)", "public void setDown(java.lang.Object)", diff --git a/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java b/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java index 0ed0daa2141..e339db2f084 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java +++ b/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java @@ -5,7 +5,9 @@ import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; +import java.util.Set; /** * A component which tracks the up/down status of any clusters which should influence @@ -37,11 +39,15 @@ public class ClustersStatus extends AbstractComponent { /** The status of clusters, when known. Note that clusters may exist for which there is no knowledge yet. */ private final Map<String, Boolean> clusterStatus = new HashMap<>(); - public void setContainerHasClusters(boolean containerHasClusters) { + /** Sets the current clusters of this container */ + public void setClusters(Set<String> clusters) { synchronized (mutex) { - this.containerHasClusters = containerHasClusters; - if ( ! containerHasClusters) - clusterStatus.clear(); // forget container clusters which was configured away + this.containerHasClusters = clusters.size() > 0; + for (Iterator<String> i = clusterStatus.keySet().iterator(); i.hasNext(); ) { + String existingCluster = i.next(); + if ( ! clusters.contains(existingCluster)) + i.remove(); // forget clusters which was configured away + } } } @@ -78,9 +84,11 @@ public class ClustersStatus extends AbstractComponent { public boolean containerShouldReceiveTraffic() { return containerShouldReceiveTraffic(Require.ONE); } + /** * Returns whether this container should receive traffic based on the state of this - * @param require Requirement for being up, ALL or ONE. + * + * @param require requirement for being up, ALL or ONE. */ public boolean containerShouldReceiveTraffic(Require require) { synchronized (mutex) { diff --git a/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java b/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java index 0bf86e8f440..e1b5b769906 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java +++ b/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java @@ -6,6 +6,8 @@ import com.yahoo.container.QrSearchersConfig; import com.yahoo.container.core.VipStatusConfig; import com.yahoo.container.jdisc.state.StateMonitor; +import java.util.stream.Collectors; + /** * A component which keeps track of whether or not this container instance should receive traffic * and respond that it is in good health. @@ -59,8 +61,7 @@ public class VipStatus { this.clustersStatus = clustersStatus; this.healthState = healthState; initiallyInRotation = vipStatusConfig.initiallyInRotation(); - healthState.status(StateMonitor.Status.initializing); - clustersStatus.setContainerHasClusters(! dispatchers.searchcluster().isEmpty()); + clustersStatus.setClusters(dispatchers.searchcluster().stream().map(c -> c.name()).collect(Collectors.toSet())); updateCurrentlyInRotation(); } diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java index 78b65622150..0018dd22dd9 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java @@ -47,11 +47,13 @@ public class StateMonitor extends AbstractComponent { @Inject public StateMonitor(HealthMonitorConfig config, Timer timer) { - this(config, timer, runnable -> { - Thread thread = new Thread(runnable, "StateMonitor"); - thread.setDaemon(true); - return thread; - }); + this(config, + timer, + runnable -> { + Thread thread = new Thread(runnable, "StateMonitor"); + thread.setDaemon(true); + return thread; + }); } StateMonitor(HealthMonitorConfig config, Timer timer, ThreadFactory threadFactory) { @@ -59,7 +61,8 @@ public class StateMonitor extends AbstractComponent { Status.valueOf(config.initialStatus()), timer, threadFactory); } - /* For Testing */ + + /* Public for testing only */ public StateMonitor(long snapshotIntervalMS, Status status, Timer timer, ThreadFactory threadFactory) { this.timer = timer; this.snapshotIntervalMs = snapshotIntervalMS; diff --git a/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java b/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java index e13debcddda..e7a9a1442f3 100644 --- a/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java +++ b/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java @@ -10,74 +10,24 @@ import com.yahoo.jdisc.core.SystemTimer; import org.junit.Test; /** - * Smoke test that VipStatus has the right basic logic. - * - * @author steinar + * @author bratseth */ public class VipStatusTestCase { - private static QrSearchersConfig getSearchersConfig(String[] clusters) { - var b = new QrSearchersConfig.Builder(); - if (clusters.length > 0) { - var searchClusterB = new QrSearchersConfig.Searchcluster.Builder(); - for (String cluster : clusters) { - searchClusterB.name(cluster); - } - b.searchcluster(searchClusterB); - } - return b.build(); - } - - private static VipStatus getVipStatus(String[] clusters, StateMonitor.Status startState, boolean initiallyInRotation) { - return new VipStatus(getSearchersConfig(clusters), - new VipStatusConfig.Builder().initiallyInRotation(initiallyInRotation).build(), - new ClustersStatus(), - new StateMonitor(1000, startState, new SystemTimer(), runnable -> { - Thread thread = new Thread(runnable, "StateMonitor"); - thread.setDaemon(true); - return thread; - })); - } - - private static void remove(String[] clusters, VipStatus v) { - for (String s : clusters) { - v.removeFromRotation(s); - } - } - - private static void add(String[] clusters, VipStatus v) { - for (String s : clusters) { - v.addToRotation(s); - } - } - - private static void verifyUpOrDown(String[] clusters, StateMonitor.Status status) { - VipStatus v = getVipStatus(clusters, status, true); - remove(clusters, v); - // initial state - assertFalse(v.isInRotation()); - v.addToRotation(clusters[0]); - assertFalse(v.isInRotation()); - v.addToRotation(clusters[1]); - assertFalse(v.isInRotation()); - v.addToRotation(clusters[2]); - assertTrue(v.isInRotation()); - } - @Test public void testInitializingOrDownRequireAllUp() { String[] clusters = {"cluster1", "cluster2", "cluster3"}; - verifyUpOrDown(clusters, StateMonitor.Status.initializing); - verifyUpOrDown(clusters, StateMonitor.Status.down); + verifyStatus(clusters, StateMonitor.Status.initializing); + verifyStatus(clusters, StateMonitor.Status.down); } @Test public void testUpRequireAllDown() { String[] clusters = {"cluster1", "cluster2", "cluster3"}; - VipStatus v = getVipStatus(clusters, StateMonitor.Status.initializing, true); + VipStatus v = createVipStatus(clusters, StateMonitor.Status.initializing, true, new ClustersStatus()); assertFalse(v.isInRotation()); - add(clusters, v); + addToRotation(clusters, v); assertTrue(v.isInRotation()); v.removeFromRotation(clusters[0]); @@ -102,14 +52,119 @@ public class VipStatusTestCase { @Test public void testNoClustersConfiguringInitiallyInRotationFalse() { String[] clusters = {}; - VipStatus v = getVipStatus(clusters, StateMonitor.Status.initializing, false); + VipStatus v = createVipStatus(clusters, StateMonitor.Status.initializing, false, new ClustersStatus()); assertFalse(v.isInRotation()); } @Test public void testNoClustersConfiguringInitiallyInRotationTrue() { String[] clusters = {}; - VipStatus v = getVipStatus(clusters, StateMonitor.Status.initializing, true); + VipStatus v = createVipStatus(clusters, StateMonitor.Status.initializing, true, new ClustersStatus()); + assertTrue(v.isInRotation()); + } + + @Test + public void testClusterRemovalRemovedIsDown() { + assertClusterRemoval(true, false); + } + + @Test + public void testClusterRemovalRemovedIsUp() { + assertClusterRemoval(false, false); + } + + @Test + public void testClusterRemovalAnotherIsDown() { + assertClusterRemoval(false, true); + } + + private void assertClusterRemoval(boolean removedIsDown, boolean anotherIsDown) { + ClustersStatus clustersStatus = new ClustersStatus(); + StateMonitor stateMonitor = createStateMonitor(StateMonitor.Status.initializing); + + String[] clusters = {"cluster1", "cluster2", "cluster3"}; + + VipStatus v = createVipStatus(clusters, true, clustersStatus, stateMonitor); + assertFalse(v.isInRotation()); + assertEquals(StateMonitor.Status.initializing, stateMonitor.status()); + + addToRotation(clusters, v); + assertTrue(v.isInRotation()); + assertEquals(StateMonitor.Status.up, stateMonitor.status()); + + String[] newClusters = {"cluster2", "cluster3"}; + if (removedIsDown) + v.removeFromRotation("cluster1"); + if (anotherIsDown) + v.removeFromRotation("cluster3"); + v = createVipStatus(newClusters, true, clustersStatus, stateMonitor); + assertTrue(v.isInRotation()); + assertEquals(StateMonitor.Status.up, stateMonitor.status()); + + v.removeFromRotation(newClusters[0]); + if ( ! anotherIsDown) + assertTrue(v.isInRotation()); + + v.removeFromRotation(newClusters[1]); + assertFalse(v.isInRotation()); // Both remaining clusters are out + assertEquals(StateMonitor.Status.down, stateMonitor.status()); + } + + private static QrSearchersConfig createSearchersConfig(String[] clusters) { + var b = new QrSearchersConfig.Builder(); + for (String cluster : clusters) { + var searchCluster = new QrSearchersConfig.Searchcluster.Builder(); + searchCluster.name(cluster); + b.searchcluster(searchCluster); + } + return b.build(); + } + + private static VipStatus createVipStatus(String[] clusters, + StateMonitor.Status startState, + boolean initiallyInRotation, + ClustersStatus clustersStatus) { + return createVipStatus(clusters, initiallyInRotation, clustersStatus, createStateMonitor(startState)); + } + + private static VipStatus createVipStatus(String[] clusters, + boolean initiallyInRotation, + ClustersStatus clustersStatus, + StateMonitor stateMonitor) { + return new VipStatus(createSearchersConfig(clusters), + new VipStatusConfig.Builder().initiallyInRotation(initiallyInRotation).build(), + clustersStatus, + stateMonitor); + } + + private static StateMonitor createStateMonitor(StateMonitor.Status startState) { + return new StateMonitor(1000, startState, new SystemTimer(), runnable -> { + Thread thread = new Thread(runnable, "StateMonitor"); + thread.setDaemon(true); + return thread; + }); + } + + private static void removeFromRotation(String[] clusters, VipStatus v) { + for (String s : clusters) + v.removeFromRotation(s); + } + + private static void addToRotation(String[] clusters, VipStatus v) { + for (String s : clusters) + v.addToRotation(s); + } + + private static void verifyStatus(String[] clusters, StateMonitor.Status status) { + VipStatus v = createVipStatus(clusters, status, true, new ClustersStatus()); + removeFromRotation(clusters, v); + // initial state + assertFalse(v.isInRotation()); + v.addToRotation(clusters[0]); + assertFalse(v.isInRotation()); + v.addToRotation(clusters[1]); + assertFalse(v.isInRotation()); + v.addToRotation(clusters[2]); assertTrue(v.isInRotation()); } diff --git a/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java b/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java index bd3d146cfec..1133363be8e 100644 --- a/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java +++ b/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java @@ -103,29 +103,27 @@ public class CloudSubscriberFactory implements SubscriberFactory { @Override public long waitNextGeneration() { - if (handles.isEmpty()) { + if (handles.isEmpty()) throw new IllegalStateException("No config keys registered"); - } - /* Catch and just log config exceptions due to missing config values for parameters that do - * not have a default value. These exceptions occur when the user has removed a component - * from services.xml, and the component takes a config that has parameters without a - * default value in the def-file. There is a new 'components' config underway, where the - * component is removed, so this old config generation will soon be replaced by a new one. */ + // Catch and just log config exceptions due to missing config values for parameters that do + // not have a default value. These exceptions occur when the user has removed a component + // from services.xml, and the component takes a config that has parameters without a + // default value in the def-file. There is a new 'components' config underway, where the + // component is removed, so this old config generation will soon be replaced by a new one. boolean gotNextGen = false; int numExceptions = 0; while ( ! gotNextGen) { try { - if (subscriber.nextGeneration()) { + if (subscriber.nextGeneration()) gotNextGen = true; - } - } catch (IllegalArgumentException e) { + } + catch (IllegalArgumentException e) { numExceptions++; - log.log(Level.WARNING, "Got exception from the config system (please ignore the exception if you just removed " - + "a component from your application that used the mentioned config): ", e); - if (numExceptions >= 5) { - throw new IllegalArgumentException("Failed retrieving the next config generation.", e); - } + log.log(Level.WARNING, "Got exception from the config system (ignore if you just removed a " + + "component from your application that used the mentioned config): ", e); + if (numExceptions >= 5) + throw new IllegalArgumentException("Failed retrieving the next config generation", e); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java b/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java index 2b61dc4c0a6..3c336c80d37 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java @@ -47,7 +47,11 @@ final class AllValuesQueryProfileVisitor extends PrefixQueryProfileVisitor { DimensionValues variant, DimensionBinding binding) { CompoundName fullName = currentPrefix.append(key); - if (values.containsKey(fullName.toString())) return; // The first value encountered has priority + + ValueWithSource existing = values.get(fullName.toString()); + + // The first value encountered has priority and values have priority over profiles + if (existing != null && (existing.value() != null || value == null)) return; Boolean isOverridable = owner != null ? owner.isLocalOverridable(key, binding) : null; diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java index 06434da2478..e85940278e3 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java @@ -14,6 +14,7 @@ import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry; import com.yahoo.search.query.profile.config.QueryProfileXMLReader; import com.yahoo.search.query.profile.types.FieldDescription; import com.yahoo.search.query.profile.types.QueryProfileType; +import org.junit.Ignore; import org.junit.Test; import java.util.HashMap; @@ -31,6 +32,17 @@ import static org.junit.Assert.fail; public class XmlReadingTestCase { @Test + public void testInheritance() { + QueryProfileRegistry registry = + new QueryProfileXMLReader().read("src/test/java/com/yahoo/search/query/profile/config/test/inheritance"); + + CompiledQueryProfile cProfile = registry.getComponent("child").compile(null); + Query q = new Query("?query=foo", cProfile); + assertEquals("a.b-parent", q.properties().getString("a.b")); + assertEquals("d-parent", q.properties().getString("d")); + } + + @Test public void testValid() { QueryProfileRegistry registry= new QueryProfileXMLReader().read("src/test/java/com/yahoo/search/query/profile/config/test/validxml"); diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/inheritance/child.xml b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/inheritance/child.xml new file mode 100644 index 00000000000..64dd3b787ac --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/inheritance/child.xml @@ -0,0 +1,6 @@ +<!-- Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> +<query-profile id="child" inherits="parent"> + <field name="a.b.c">a.b.c-child</field> + <field name="d.e.f">d.e.f-child</field> +</query-profile> + diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/inheritance/parent.xml b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/inheritance/parent.xml new file mode 100644 index 00000000000..b3443fab646 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/inheritance/parent.xml @@ -0,0 +1,7 @@ +<!-- Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> +<query-profile id="parent"> + <field name="a.b">a.b-parent</field> + <field name="a.b.c">a.b.c-parent</field> + <field name="d">d-parent</field> + <field name="d.e.f">d.e.f-parent</field> +</query-profile> diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/types/test/MandatoryTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/types/test/MandatoryTestCase.java index 7dc6eb3d8aa..b875c66735b 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/types/test/MandatoryTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/profile/types/test/MandatoryTestCase.java @@ -230,7 +230,6 @@ public class MandatoryTestCase { defaultProfile.setType(fixture.rootType); QueryProfile mandatoryProfile = new QueryProfile("mandatory"); - mandatoryProfile.setType(fixture.rootType); mandatoryProfile.setType(fixture.mandatoryType); fixture.registry.register(defaultProfile); @@ -249,7 +248,6 @@ public class MandatoryTestCase { defaultProfile.setType(fixture.rootType); QueryProfile mandatoryProfile = new QueryProfile("mandatory"); - mandatoryProfile.setType(fixture.rootType); mandatoryProfile.addInherited(defaultProfile); // The single difference from the test above mandatoryProfile.setType(fixture.mandatoryType); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java index 24864c03530..2a167ee2962 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java @@ -18,7 +18,7 @@ public interface BillingController { /** * @return String containing error message if something went wrong. Empty otherwise */ - PlanResult setPlan(TenantName tenant, PlanId planId, boolean hasApplications); + PlanResult setPlan(TenantName tenant, PlanId planId, boolean hasDeployments); Invoice.Id createInvoiceForPeriod(TenantName tenant, ZonedDateTime startTime, ZonedDateTime endTime, String agent); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java index 4f367d6498e..523f20eaef8 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java @@ -30,7 +30,7 @@ public class MockBillingController implements BillingController { } @Override - public PlanResult setPlan(TenantName tenant, PlanId planId, boolean hasApplications) { + public PlanResult setPlan(TenantName tenant, PlanId planId, boolean hasDeployments) { plans.put(tenant, planId); return PlanResult.success(); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/WeightedAliasTarget.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/WeightedAliasTarget.java index 8f81c94257e..d3d60ecd64d 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/WeightedAliasTarget.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/WeightedAliasTarget.java @@ -21,6 +21,7 @@ public class WeightedAliasTarget extends AliasTarget { public WeightedAliasTarget(HostName name, String dnsZone, ZoneId zone, long weight) { super(name, dnsZone, zone.value()); this.weight = weight; + if (weight < 0) throw new IllegalArgumentException("Weight cannot be negative"); } /** The weight of this target */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index 15ab14e3241..9e4600a1bdb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -18,6 +18,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.ServiceRegistry; import com.yahoo.vespa.hosted.controller.api.integration.maven.MavenRepository; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; import com.yahoo.vespa.hosted.controller.auditlog.AuditLogger; +import com.yahoo.vespa.hosted.controller.config.ControllerConfig; import com.yahoo.vespa.hosted.controller.deployment.JobController; import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder; import com.yahoo.vespa.hosted.controller.metric.ConfigServerMetrics; @@ -76,6 +77,7 @@ public class Controller extends AbstractComponent { private final MavenRepository mavenRepository; private final Metric metric; private final RoutingController routingController; + private final ControllerConfig controllerConfig; /** * Creates a controller @@ -84,14 +86,15 @@ public class Controller extends AbstractComponent { */ @Inject public Controller(CuratorDb curator, RotationsConfig rotationsConfig, AccessControl accessControl, FlagSource flagSource, - MavenRepository mavenRepository, ServiceRegistry serviceRegistry, Metric metric, SecretStore secretStore) { + MavenRepository mavenRepository, ServiceRegistry serviceRegistry, Metric metric, SecretStore secretStore, + ControllerConfig controllerConfig) { this(curator, rotationsConfig, accessControl, com.yahoo.net.HostName::getLocalhost, flagSource, - mavenRepository, serviceRegistry, metric, secretStore); + mavenRepository, serviceRegistry, metric, secretStore, controllerConfig); } public Controller(CuratorDb curator, RotationsConfig rotationsConfig, AccessControl accessControl, Supplier<String> hostnameSupplier, FlagSource flagSource, MavenRepository mavenRepository, - ServiceRegistry serviceRegistry, Metric metric, SecretStore secretStore) { + ServiceRegistry serviceRegistry, Metric metric, SecretStore secretStore, ControllerConfig controllerConfig) { this.hostnameSupplier = Objects.requireNonNull(hostnameSupplier, "HostnameSupplier cannot be null"); this.curator = Objects.requireNonNull(curator, "Curator cannot be null"); @@ -110,6 +113,7 @@ public class Controller extends AbstractComponent { routingController = new RoutingController(this, Objects.requireNonNull(rotationsConfig, "RotationsConfig cannot be null")); auditLogger = new AuditLogger(curator, clock); jobControl = new JobControl(curator); + this.controllerConfig = controllerConfig; // Record the version of this controller curator().writeControllerVersion(this.hostname(), ControllerVersion.CURRENT); @@ -149,6 +153,8 @@ public class Controller extends AbstractComponent { public MavenRepository mavenRepository() { return mavenRepository; } + public ControllerConfig controllerConfig() { return controllerConfig; } + public ApplicationView getApplicationView(String tenantName, String applicationName, String instanceName, String environment, String region) { return serviceRegistry.configServer().getApplicationView(tenantName, applicationName, instanceName, environment, region); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index f08cce57dcb..0ef25d0f613 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -45,6 +45,7 @@ import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.Endpoint; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.certificate.EndpointCertificateException; +import com.yahoo.vespa.hosted.controller.config.ControllerConfig; import com.yahoo.vespa.hosted.controller.maintenance.JobRunner; import com.yahoo.vespa.hosted.controller.routing.RoutingPolicyId; import com.yahoo.yolean.Exceptions; @@ -793,10 +794,13 @@ public class InternalStepRunner implements StepRunner { ZoneId zone = id.type().zone(controller.system()); boolean useTesterCertificate = controller.system().isPublic() && id.type().environment().isTest(); + boolean useOsgiBasedTestRuntime = testerPlatformVersion(id).isAfter(new Version(7, 247, 11)); byte[] servicesXml = servicesXml(! controller.system().isPublic(), useTesterCertificate, - testerResourcesFor(zone, spec.requireInstance(id.application().instance()))); + useOsgiBasedTestRuntime, + testerResourcesFor(zone, spec.requireInstance(id.application().instance())), + controller.controllerConfig().steprunner().testerapp()); byte[] testPackage = controller.applications().applicationStore().getTester(id.application().tenant(), id.application().application(), version); byte[] deploymentXml = deploymentXml(id.tester(), spec.athenzDomain(), @@ -845,7 +849,9 @@ public class InternalStepRunner implements StepRunner { } /** Returns the generated services.xml content for the tester application. */ - static byte[] servicesXml(boolean systemUsesAthenz, boolean useTesterCertificate, NodeResources resources) { + static byte[] servicesXml( + boolean systemUsesAthenz, boolean useTesterCertificate, boolean useOsgiBasedTestRuntime, + NodeResources resources, ControllerConfig.Steprunner.Testerapp config) { int jdiscMemoryGb = 2; // 2Gb memory for tester application (excessive?). int jdiscMemoryPct = (int) Math.ceil(100 * jdiscMemoryGb / resources.memoryGb()); @@ -856,6 +862,23 @@ public class InternalStepRunner implements StepRunner { "<resources vcpu=\"%.2f\" memory=\"%.2fGb\" disk=\"%.2fGb\" disk-speed=\"%s\" storage-type=\"%s\"/>", resources.vcpu(), resources.memoryGb(), resources.diskGb(), resources.diskSpeed().name(), resources.storageType().name()); + String runtimeProviderClass = config.runtimeProviderClass(); + String tenantCdBundle = config.tenantCdBundle(); + + String handlerAndExtraComponents = useOsgiBasedTestRuntime + ? + " <component id=\"" + runtimeProviderClass + "\" bundle=\"" + tenantCdBundle + "\" />\n" + + "\n" + + " <component id=\"com.yahoo.vespa.testrunner.JunitRunner\" bundle=\"vespa-osgi-testrunner\" />\n" + + "\n" + + " <handler id=\"com.yahoo.vespa.testrunner.TestRunnerHandler\" bundle=\"vespa-osgi-testrunner\">\n" + + " <binding>http://*/tester/v1/*</binding>\n" + + " </handler>\n" + : + " <handler id=\"com.yahoo.vespa.hosted.testrunner.TestRunnerHandler\" bundle=\"vespa-testrunner-components\">\n" + + " <binding>http://*/tester/v1/*</binding>\n" + + " </handler>\n"; + String servicesXml = "<?xml version='1.0' encoding='UTF-8'?>\n" + "<services xmlns:deploy='vespa' version='1.0'>\n" + @@ -870,9 +893,7 @@ public class InternalStepRunner implements StepRunner { " </config>\n" + " </component>\n" + "\n" + - " <handler id=\"com.yahoo.vespa.hosted.testrunner.TestRunnerHandler\" bundle=\"vespa-testrunner-components\">\n" + - " <binding>http://*/tester/v1/*</binding>\n" + - " </handler>\n" + + handlerAndExtraComponents + "\n" + " <nodes count=\"1\" allocated-memory=\"" + jdiscMemoryPct + "%\">\n" + " " + resourceString + "\n" + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java index 0e6f856b115..8fad0db4368 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java @@ -19,6 +19,7 @@ import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.hosted.controller.ApplicationController; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.billing.PaymentInstrument; import com.yahoo.vespa.hosted.controller.api.integration.billing.Invoice; @@ -137,9 +138,9 @@ public class BillingApiHandler extends LoggingRequestHandler { var tenantName = TenantName.from(tenant); var slime = inspectorOrThrow(request); var planId = PlanId.from(slime.field("plan").asString()); - var hasApplications = applicationController.asList(tenantName).size() > 0; - var result = billingController.setPlan(tenantName, planId, hasApplications); + var hasDeployments = hasDeployments(tenantName); + var result = billingController.setPlan(tenantName, planId, hasDeployments); if (result.isSuccess()) return new StringResponse("Plan: " + planId.value()); @@ -380,4 +381,14 @@ public class BillingApiHandler extends LoggingRequestHandler { return LocalDate.parse(until); } + private boolean hasDeployments(TenantName tenantName) { + return applicationController.asList(tenantName) + .stream() + .flatMap(app -> app.instances().values() + .stream() + .flatMap(instance -> instance.deployments().values().stream()) + ) + .count() > 0; + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index 2fa931f2219..e5a99c2e69d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -177,51 +177,65 @@ public class RoutingPolicies { private void updateGlobalDnsOf(Collection<RoutingPolicy> routingPolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Lock lock) { Map<RoutingId, List<RoutingPolicy>> routingTable = routingTableFrom(routingPolicies); for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) { - Map<RegionEndpoint, Set<AliasTarget>> targets = computeRegionEndpoints(routeEntry.getValue(), inactiveZones); + Collection<RegionEndpoint> regionEndpoints = computeRegionEndpoints(routeEntry.getValue(), inactiveZones); // Create a weighted ALIAS per region, pointing to all zones within the same region - targets.forEach(((regionEndpoint, weightedTargets) -> { - controller.nameServiceForwarder().createAlias(RecordName.from(regionEndpoint.dnsName), weightedTargets, + regionEndpoints.forEach(regionEndpoint -> { + controller.nameServiceForwarder().createAlias(RecordName.from(regionEndpoint.target().name().value()), + Collections.unmodifiableSet(regionEndpoint.zoneTargets()), Priority.normal); - })); + }); + // Create global latency-based ALIAS pointing to each per-region weighted ALIAS + Set<AliasTarget> latencyTargets = new LinkedHashSet<>(); + Set<AliasTarget> inactiveLatencyTargets = new LinkedHashSet<>(); + for (var regionEndpoint : regionEndpoints) { + if (regionEndpoint.active()) { + latencyTargets.add(regionEndpoint.target()); + } else { + inactiveLatencyTargets.add(regionEndpoint.target()); + } + } + // If all targets are configured out, all targets are set in. We do this because otherwise removing 100% of + // the ALIAS records would cause the global endpoint to stop resolving entirely (NXDOMAIN). + if (latencyTargets.isEmpty() && !inactiveLatencyTargets.isEmpty()) { + latencyTargets.addAll(inactiveLatencyTargets); + inactiveLatencyTargets.clear(); + } var endpoints = controller.routing().endpointsOf(routeEntry.getKey().application()) .named(routeEntry.getKey().endpointId()) .not().requiresRotation(); - Set<AliasTarget> latencyTargets = targets.keySet().stream() - .map(regionEndpoint -> new LatencyAliasTarget(HostName.from(regionEndpoint.dnsName), - regionEndpoint.dnsZone, - regionEndpoint.zone)) - .collect(Collectors.toSet()); endpoints.forEach(endpoint -> controller.nameServiceForwarder().createAlias(RecordName.from(endpoint.dnsName()), latencyTargets, Priority.normal)); + inactiveLatencyTargets.forEach(t -> controller.nameServiceForwarder() + .removeRecords(Record.Type.ALIAS, + RecordData.fqdn(t.name().value()), + Priority.normal)); } } /** Compute region endpoints and their targets from given policies */ - private Map<RegionEndpoint, Set<AliasTarget>> computeRegionEndpoints(List<RoutingPolicy> policies, Set<ZoneId> inactiveZones) { - Map<RegionEndpoint, Set<AliasTarget>> targets = new LinkedHashMap<>(); + private Collection<RegionEndpoint> computeRegionEndpoints(List<RoutingPolicy> policies, Set<ZoneId> inactiveZones) { + Map<Endpoint, RegionEndpoint> endpoints = new LinkedHashMap<>(); RoutingMethod routingMethod = RoutingMethod.exclusive; for (var policy : policies) { if (policy.dnsZone().isEmpty()) continue; if (!controller.zoneRegistry().routingMethods(policy.id().zone()).contains(routingMethod)) continue; Endpoint weighted = policy.weightedEndpointIn(controller.system(), routingMethod); - // Do not route to zone if global routing status is set out at: - // - zone level (ZoneRoutingPolicy) - // - deployment level (RoutingPolicy) - // - application package level (deployment.xml) - long weight = 1; var zonePolicy = db.readZoneRoutingPolicy(policy.id().zone()); + long weight = 1; if (isConfiguredOut(policy, zonePolicy, inactiveZones)) { weight = 0; // A record with 0 weight will not received traffic. If all records within a group have 0 // weight, traffic is routed to all records with equal probability. } - var regionEndpoint = new RegionEndpoint(weighted, policy.dnsZone().get(), policy.id().zone()); var weightedTarget = new WeightedAliasTarget(policy.canonicalName(), policy.dnsZone().get(), policy.id().zone(), weight); - targets.computeIfAbsent(regionEndpoint, (k) -> new LinkedHashSet<>()) - .add(weightedTarget); + endpoints.computeIfAbsent(weighted, (k) -> new RegionEndpoint(new LatencyAliasTarget(HostName.from(weighted.dnsName()), + policy.dnsZone().get(), + policy.id().zone()))) + .zoneTargets() + .add(weightedTarget); } - return Collections.unmodifiableMap(targets); + return endpoints.values(); } /** Store routing policies for given load balancers */ @@ -335,18 +349,26 @@ public class RoutingPolicies { return false; } - /** Represents a region-wide endpoint */ + /** Represents records for a region-wide endpoint */ private static class RegionEndpoint { - private final String dnsName; - private final String dnsZone; - private final ZoneId zone; + private final LatencyAliasTarget target; + private final Set<WeightedAliasTarget> zoneTargets = new LinkedHashSet<>(); + + public RegionEndpoint(LatencyAliasTarget target) { + this.target = Objects.requireNonNull(target); + } + + public LatencyAliasTarget target() { + return target; + } + + public Set<WeightedAliasTarget> zoneTargets() { + return zoneTargets; + } - public RegionEndpoint(Endpoint endpoint, String dnsZone, ZoneId zone) { - this.dnsName = Objects.requireNonNull(endpoint).dnsName(); - this.dnsZone = Objects.requireNonNull(dnsZone); - this.zone = Objects.requireNonNull(zone); - if (endpoint.scope() != Endpoint.Scope.weighted) throw new IllegalArgumentException("Region endpoint must be weighted"); + public boolean active() { + return zoneTargets.stream().anyMatch(target -> target.weight() > 0); } @Override @@ -354,12 +376,12 @@ public class RoutingPolicies { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; RegionEndpoint that = (RegionEndpoint) o; - return dnsName.equals(that.dnsName); + return target.name().equals(that.target.name()); } @Override public int hashCode() { - return Objects.hash(dnsName); + return Objects.hash(target.name()); } } diff --git a/controller-server/src/main/resources/configdefinitions/controller.def b/controller-server/src/main/resources/configdefinitions/controller.def new file mode 100644 index 00000000000..069deaf276d --- /dev/null +++ b/controller-server/src/main/resources/configdefinitions/controller.def @@ -0,0 +1,7 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +# Generic config for controller +namespace=vespa.hosted.controller.config + +steprunner.testerapp.tenantCdBundle string default="cloud-tenant-cd" + +steprunner.testerapp.runtimeProviderClass string default="ai.vespa.hosted.cd.cloud.impl.VespaTestRuntimeProvider"
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index 35093c22f42..c0244b9ea17 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -31,6 +31,7 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; +import com.yahoo.vespa.hosted.controller.config.ControllerConfig; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.integration.MetricsMock; import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock; @@ -367,7 +368,8 @@ public final class ControllerTester { new InMemoryFlagSource(), new MockMavenRepository(), serviceRegistry, - new MetricsMock(), new SecretStoreMock()); + new MetricsMock(), new SecretStoreMock(), + new ControllerConfig.Builder().build()); // Calculate initial versions controller.updateVersionStatus(VersionStatus.compute(controller)); return controller; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index 07c643070a0..02640cf8486 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -27,6 +27,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.SystemApplication; +import com.yahoo.vespa.hosted.controller.config.ControllerConfig; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; import org.junit.Before; import org.junit.Test; @@ -485,12 +486,24 @@ public class InternalStepRunnerTest { } @Test - public void generates_correct_services_xml_test() { - assertFile("test_runner_services.xml-cd", - new String(InternalStepRunner.servicesXml( - true, - false, - new NodeResources(2, 12, 75, 1, NodeResources.DiskSpeed.fast, NodeResources.StorageType.local)))); + public void generates_correct_services_xml_using_osgi_based_runtime() { + generates_correct_services_xml("test_runner_services.xml-cd-osgi", true); + } + + @Test + public void generates_correct_services_xml_using_legacy_runtime() { + generates_correct_services_xml("test_runner_services.xml-cd-legacy", false); + } + + private void generates_correct_services_xml(String filenameExpectedOutput, boolean useOsgiBasedRuntime) { + ControllerConfig.Steprunner.Testerapp config = new ControllerConfig.Steprunner.Testerapp.Builder().build(); + assertFile(filenameExpectedOutput, + new String(InternalStepRunner.servicesXml( + true, + false, + useOsgiBasedRuntime, + new NodeResources(2, 12, 75, 1, NodeResources.DiskSpeed.fast, NodeResources.StorageType.local), + config))); } private void assertFile(String resourceName, String actualContent) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index ca4bcf1a354..3ee4c6961a4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -1,6 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.routing; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationId; @@ -27,7 +28,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; -import com.yahoo.vespa.hosted.controller.api.integration.dns.WeightedAliasTarget; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Endpoint; import com.yahoo.vespa.hosted.controller.application.EndpointId; @@ -48,6 +48,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -55,6 +56,7 @@ import java.util.Set; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; /** @@ -147,7 +149,7 @@ public class RoutingPoliciesTest { @Test public void global_routing_policies_with_duplicate_region() { var tester = new RoutingPoliciesTester(); - var context1 = tester.newDeploymentContext("tenant1", "app1", "default"); + var context = tester.newDeploymentContext("tenant1", "app1", "default"); int clustersPerZone = 2; int numberOfDeployments = 3; var applicationPackage = applicationPackageBuilder() @@ -157,15 +159,42 @@ public class RoutingPoliciesTest { .endpoint("r0", "c0") .endpoint("r1", "c1") .build(); - tester.provisionLoadBalancers(clustersPerZone, context1.instanceId(), zone1, zone3, zone4); + tester.provisionLoadBalancers(clustersPerZone, context.instanceId(), zone1, zone3, zone4); // Creates alias records - context1.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - tester.assertTargets(context1.instanceId(), EndpointId.of("r0"), 0, zone1, zone3, zone4); - tester.assertTargets(context1.instanceId(), EndpointId.of("r1"), 1, zone1, zone3, zone4); + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone3, zone4); + tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 1, zone1, zone3, zone4); assertEquals("Routing policy count is equal to cluster count", numberOfDeployments * clustersPerZone, - tester.policiesOf(context1.instance().id()).size()); + tester.policiesOf(context.instance().id()).size()); + + // A zone in shared region is set out + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone4), GlobalRouting.Status.out, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + + // Weight of inactive zone is set to zero + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L, + zone3, 1L, + zone4, 0L)); + + // Other zone in shared region is set out. Entire record group for the region is removed as all zones in the + // region are out (weight sum = 0) + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone3), GlobalRouting.Status.out, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L)); + + // Everything is set back in + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone3), GlobalRouting.Status.in, + GlobalRouting.Agent.tenant); + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone4), GlobalRouting.Status.in, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L, + zone3, 1L, + zone4, 1L)); } @Test @@ -395,9 +424,9 @@ public class RoutingPoliciesTest { GlobalRouting.Agent.tenant); context.flushDnsUpdates(); - // Inactive zone is given zero weight - tester.assertWeight(0, context.instanceId(), 0, zone1); - tester.assertWeight(1, context.instanceId(), 0, zone2); + // Inactive zone is removed from global DNS record + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2); + tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone2); // Status details is stored in policy var policy1 = tester.routingPolicies().get(context.deploymentIdIn(zone1)).values().iterator().next(); @@ -414,15 +443,16 @@ public class RoutingPoliciesTest { // Next deployment does not affect status context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); context.flushDnsUpdates(); - tester.assertWeight(0, context.instanceId(), 0, zone1); - tester.assertWeight(1, context.instanceId(), 0, zone2); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2); + tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone2); // Deployment is set back in tester.controllerTester().clock().advance(Duration.ofHours(1)); changedAt = tester.controllerTester().clock().instant(); tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.in, GlobalRouting.Agent.tenant); context.flushDnsUpdates(); - tester.assertWeight(1, context.instanceId(), 0, zone1, zone2); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); + tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1, zone2); policy1 = tester.routingPolicies().get(context.deploymentIdIn(zone1)).values().iterator().next(); assertEquals(GlobalRouting.Status.in, policy1.status().globalRouting().status()); @@ -437,8 +467,8 @@ public class RoutingPoliciesTest { .endpoint("r1", "c0", zone1.region().value(), zone2.region().value()) .build(); context.submit(applicationPackage2).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - tester.assertWeight(1, context.instanceId(), 0, zone1); - tester.assertWeight(0, context.instanceId(), 0, zone2); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1); + tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1); // ... back in var applicationPackage3 = applicationPackageBuilder() @@ -448,7 +478,8 @@ public class RoutingPoliciesTest { .endpoint("r1", "c0", zone1.region().value(), zone2.region().value()) .build(); context.submit(applicationPackage3).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - tester.assertWeight(1, context.instanceId(), 0, zone1, zone2); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); + tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1, zone2); } @Test @@ -468,14 +499,13 @@ public class RoutingPoliciesTest { tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2); context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); tester.assertTargets(context.instanceId(), EndpointId.defaultId(), 0, zone1, zone2); - tester.assertWeight(1, context.instanceId(), 0, zone1, zone2); } // Set zone out tester.routingPolicies().setGlobalRoutingStatus(zone2, GlobalRouting.Status.out); context1.flushDnsUpdates(); - tester.assertWeight(1, context1.instanceId(), 0, zone1); - tester.assertWeight(0, context1.instanceId(), 0, zone2); + tester.assertTargets(context1.instanceId(), EndpointId.defaultId(), 0, zone1); + tester.assertTargets(context2.instanceId(), EndpointId.defaultId(), 0, zone1); for (var context : contexts) { var policies = tester.routingPolicies().get(context.instanceId()); assertTrue("Global routing status for policy remains " + GlobalRouting.Status.in, @@ -494,8 +524,8 @@ public class RoutingPoliciesTest { // Setting status per deployment does not affect status as entire zone is out tester.routingPolicies().setGlobalRoutingStatus(context1.deploymentIdIn(zone2), GlobalRouting.Status.in, GlobalRouting.Agent.tenant); context1.flushDnsUpdates(); - tester.assertWeight(0, context1.instanceId(), 0, zone2); - tester.assertWeight(0, context2.instanceId(), 0, zone2); + tester.assertTargets(context1.instanceId(), EndpointId.defaultId(), 0, zone1); + tester.assertTargets(context2.instanceId(), EndpointId.defaultId(), 0, zone1); // Set single deployment out tester.routingPolicies().setGlobalRoutingStatus(context1.deploymentIdIn(zone2), GlobalRouting.Status.out, GlobalRouting.Agent.tenant); @@ -504,9 +534,8 @@ public class RoutingPoliciesTest { // Set zone back in. Deployment set explicitly out, remains out, the rest are in tester.routingPolicies().setGlobalRoutingStatus(zone2, GlobalRouting.Status.in); context1.flushDnsUpdates(); - tester.assertWeight(1, context1.instanceId(), 0, zone1); - tester.assertWeight(0, context1.instanceId(), 0, zone2); - tester.assertWeight(1, context2.instanceId(), 0, zone1, zone2); + tester.assertTargets(context1.instanceId(), EndpointId.defaultId(), 0, zone1); + tester.assertTargets(context2.instanceId(), EndpointId.defaultId(), 0, zone1, zone2); } @Test @@ -549,7 +578,64 @@ public class RoutingPoliciesTest { // Deployment completes context.completeRollout(); - tester.assertTargets(context.instanceId(), endpointId, ClusterSpec.Id.from("default"), 0, prodZone); + tester.assertTargets(context.instanceId(), endpointId, ClusterSpec.Id.from("default"), 0, Map.of(prodZone, 1L)); + } + + @Test + public void changing_global_routing_status_never_removes_all_members() { + var tester = new RoutingPoliciesTester(); + var context = tester.newDeploymentContext("tenant1", "app1", "default"); + + // Provision load balancers and deploy application + tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2); + var applicationPackage = applicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region()) + .endpoint("r0", "c0", zone1.region().value(), zone2.region().value()) + .build(); + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); + + // Global DNS record is created, pointing to all configured zones + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); + + // Global routing status is overridden for one deployment + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.out, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2); + + // Setting other deployment out implicitly sets all deployments in. Weight is set to zero, but that has no + // impact on routing decisions when the weight sum is zero + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone2), GlobalRouting.Status.out, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 0L, zone2, 0L)); + + // One inactive deployment is put back in. Global DNS record now points to the only active deployment + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.in, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1); + + // Setting zone (containing active deployment) out puts all deployments in + tester.routingPolicies().setGlobalRoutingStatus(zone1, GlobalRouting.Status.out); + context.flushDnsUpdates(); + assertEquals(GlobalRouting.Status.out, tester.routingPolicies().get(zone1).globalRouting().status()); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 0L, zone2, 0L)); + + // Setting zone back in removes the currently inactive deployment + tester.routingPolicies().setGlobalRoutingStatus(zone1, GlobalRouting.Status.in); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1); + + // Inactive deployment is set in + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone2), GlobalRouting.Status.in, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + for (var policy : tester.routingPolicies().get(context.instanceId()).values()) { + assertSame(GlobalRouting.Status.in, policy.status().globalRouting().status()); + } + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); } @Test @@ -584,7 +670,7 @@ public class RoutingPoliciesTest { lbHostname = HostName.from("shared-lb--" + zone.value()); } else { lbHostname = HostName.from("lb-" + i + "--" + application.serializedForm() + - "--" + zone.value()); + "--" + zone.value()); } loadBalancers.add( new LoadBalancer("LB-" + i + "-Z-" + zone.value(), @@ -656,11 +742,11 @@ public class RoutingPoliciesTest { .collect(Collectors.toSet()); } - private List<String> aliasDataOf(String name) { + private Set<String> aliasDataOf(String name) { return tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from(name)).stream() .map(Record::data) .map(RecordData::asString) - .collect(Collectors.toList()); + .collect(Collectors.toSet()); } private List<String> cnameDataOf(String name) { @@ -670,27 +756,10 @@ public class RoutingPoliciesTest { .collect(Collectors.toList()); } - private void assertWeight(long expected, ApplicationId application, int loadBalancerId, ZoneId... zones) { - for (var zone : zones) { - Endpoint weighted = tester.controller().routing().endpointsOf(new DeploymentId(application, zone)) - .scope(Endpoint.Scope.weighted) - .named(EndpointId.of("c" + loadBalancerId)) - .asList() - .get(0); - List<Record> records = tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, - RecordName.from(weighted.dnsName())); - assertEquals(1, records.size()); - assertEquals("Record " + weighted.dnsName() + " has expected weight", - expected, - WeightedAliasTarget.unpack(records.get(0).data()) - .weight()); - } - } - - private void assertTargets(ApplicationId application, EndpointId endpointId, ClusterSpec.Id clusterId, int loadBalancerId, ZoneId... zones) { + private void assertTargets(ApplicationId application, EndpointId endpointId, ClusterSpec.Id clusterId, int loadBalancerId, Map<ZoneId, Long> zoneWeights) { Set<String> latencyTargets = new HashSet<>(); Map<String, List<ZoneId>> zonesByRegionEndpoint = new HashMap<>(); - for (var zone : zones) { + for (var zone : zoneWeights.keySet()) { Endpoint weighted = tester.controller().routing().endpointsOf(new DeploymentId(application, zone)) .scope(Endpoint.Scope.weighted) .named(EndpointId.of(clusterId.value())) @@ -700,11 +769,11 @@ public class RoutingPoliciesTest { .add(zone); } zonesByRegionEndpoint.forEach((regionEndpoint, zonesInRegion) -> { - List<String> weightedTargets = zonesInRegion.stream() - .map(z -> "weighted/lb-" + loadBalancerId + "--" + - application.serializedForm() + "--" + z.value() + - "/dns-zone-1/" + z.value() + "/1") - .collect(Collectors.toList()); + Set<String> weightedTargets = zonesInRegion.stream() + .map(z -> "weighted/lb-" + loadBalancerId + "--" + + application.serializedForm() + "--" + z.value() + + "/dns-zone-1/" + z.value() + "/" + zoneWeights.get(z)) + .collect(Collectors.toSet()); assertEquals("Weighted endpoint " + regionEndpoint + " points to load balancer", weightedTargets, aliasDataOf(regionEndpoint)); @@ -714,7 +783,7 @@ public class RoutingPoliciesTest { }); String globalEndpoint = tester.controller().routing().endpointsOf(application) .named(endpointId) - .targets(List.of(zones)) + .targets(List.copyOf(zoneWeights.keySet())) .primary() .map(Endpoint::dnsName) .orElse("<none>"); @@ -724,7 +793,15 @@ public class RoutingPoliciesTest { } private void assertTargets(ApplicationId application, EndpointId endpointId, int loadBalancerId, ZoneId... zones) { - assertTargets(application, endpointId, ClusterSpec.Id.from("c" + loadBalancerId), loadBalancerId, zones); + Map<ZoneId, Long> zoneWeights = new LinkedHashMap<>(); + for (var zone : zones) { + zoneWeights.put(zone, 1L); + } + assertTargets(application, endpointId, ClusterSpec.Id.from("c" + loadBalancerId), loadBalancerId, zoneWeights); + } + + private void assertTargets(ApplicationId application, EndpointId endpointId, int loadBalancerId, Map<ZoneId, Long> zoneWeights) { + assertTargets(application, endpointId, ClusterSpec.Id.from("c" + loadBalancerId), loadBalancerId, zoneWeights); } } diff --git a/controller-server/src/test/resources/test_runner_services.xml-cd b/controller-server/src/test/resources/test_runner_services.xml-cd-legacy index 125c5004d25..125c5004d25 100644 --- a/controller-server/src/test/resources/test_runner_services.xml-cd +++ b/controller-server/src/test/resources/test_runner_services.xml-cd-legacy diff --git a/controller-server/src/test/resources/test_runner_services.xml-cd-osgi b/controller-server/src/test/resources/test_runner_services.xml-cd-osgi new file mode 100644 index 00000000000..03277628156 --- /dev/null +++ b/controller-server/src/test/resources/test_runner_services.xml-cd-osgi @@ -0,0 +1,26 @@ +<?xml version='1.0' encoding='UTF-8'?> +<services xmlns:deploy='vespa' version='1.0'> + <container version='1.0' id='tester'> + + <component id="com.yahoo.vespa.hosted.testrunner.TestRunner" bundle="vespa-testrunner-components"> + <config name="com.yahoo.vespa.hosted.testrunner.test-runner"> + <artifactsPath>artifacts</artifactsPath> + <surefireMemoryMb>5120</surefireMemoryMb> + <useAthenzCredentials>true</useAthenzCredentials> + <useTesterCertificate>false</useTesterCertificate> + </config> + </component> + + <component id="ai.vespa.hosted.cd.cloud.impl.VespaTestRuntimeProvider" bundle="cloud-tenant-cd" /> + + <component id="com.yahoo.vespa.testrunner.JunitRunner" bundle="vespa-osgi-testrunner" /> + + <handler id="com.yahoo.vespa.testrunner.TestRunnerHandler" bundle="vespa-osgi-testrunner"> + <binding>http://*/tester/v1/*</binding> + </handler> + + <nodes count="1" allocated-memory="17%"> + <resources vcpu="2.00" memory="12.00Gb" disk="75.00Gb" disk-speed="fast" storage-type="local"/> + </nodes> + </container> +</services> diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 4ab477dbaad..60c02b3fd60 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -152,10 +152,10 @@ public class Flags { "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); - public static final UnboundBooleanFlag USE_DISTRIBUTOR_BTREE_DB = defineFeatureFlag( - "use-distributor-btree-db", true, - "Whether to use the new B-tree bucket database in the distributors.", - "Takes effect at restart of distributor process", + public static final UnboundBooleanFlag USE_CONTENT_NODE_BTREE_DB = defineFeatureFlag( + "use-content-node-btree-db", false, + "Whether to use the new B-tree bucket database on the content node.", + "Takes effect at restart of content node process", ZONE_ID, APPLICATION_ID); public static final UnboundBooleanFlag USE_THREE_PHASE_UPDATES = defineFeatureFlag( @@ -295,17 +295,29 @@ public class Flags { APPLICATION_ID ); + public static final UnboundBooleanFlag WEIGHTED_DNS_PER_REGION = defineFeatureFlag( + "weighted-dns-per-region", false, + "Whether to create weighted DNS records per region in global endpoints", + "Takes effect on next deployment through controller", + APPLICATION_ID + ); + public static final UnboundBooleanFlag ONLY_PUBLIC_ACCESS = defineFeatureFlag( "enable-public-only", false, "Only access public hosts from container", "Takes effect on next tick" ); - public static final UnboundBooleanFlag WEIGHTED_DNS_PER_REGION = defineFeatureFlag( - "weighted-dns-per-region", false, - "Whether to create weighted DNS records per region in global endpoints", - "Takes effect on next deployment through controller", - APPLICATION_ID + public static final UnboundListFlag<String> OUTBOUND_BLOCKED_IPV4 = defineListFlag( + "container-outbound-blocked-ipv4", List.of(), String.class, + "List of IPs or CIDRs that are blocked for outbound connections", + "Takes effect on next tick" + ); + + public static final UnboundListFlag<String> OUTBOUND_BLOCKED_IPV6 = defineListFlag( + "container-outbound-blocked-ipv6", List.of(), String.class, + "List of IPs or CIDRs that are blocked for outbound connections", + "Takes effect on next tick" ); /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ diff --git a/hosted-api/src/main/java/ai/vespa/hosted/api/TestDescriptor.java b/hosted-api/src/main/java/ai/vespa/hosted/api/TestDescriptor.java index 08cd3932ae7..6074bd73a20 100644 --- a/hosted-api/src/main/java/ai/vespa/hosted/api/TestDescriptor.java +++ b/hosted-api/src/main/java/ai/vespa/hosted/api/TestDescriptor.java @@ -26,6 +26,7 @@ public class TestDescriptor { private static final String JSON_FIELD_CONFIGURED_TESTS = "configuredTests"; private static final String JSON_FIELD_SYSTEM_TESTS = "systemTests"; private static final String JSON_FIELD_STAGING_TESTS = "stagingTests"; + private static final String JSON_FIELD_STAGING_SETUP_TESTS = "stagingSetupTests"; private static final String JSON_FIELD_PRODUCTION_TESTS = "productionTests"; private final Map<TestCategory, List<String>> configuredTestClasses; @@ -43,20 +44,22 @@ public class TestDescriptor { var testRoot = root.field(JSON_FIELD_CONFIGURED_TESTS); var systemTests = getJsonArray(testRoot, JSON_FIELD_SYSTEM_TESTS); var stagingTests = getJsonArray(testRoot, JSON_FIELD_STAGING_TESTS); + var stagingSetupTests = getJsonArray(testRoot, JSON_FIELD_STAGING_SETUP_TESTS); var productionTests = getJsonArray(testRoot, JSON_FIELD_PRODUCTION_TESTS); - return new TestDescriptor(version, toMap(systemTests, stagingTests, productionTests)); + return new TestDescriptor(version, toMap(systemTests, stagingTests, stagingSetupTests, productionTests)); } public static TestDescriptor from( - String version, List<String> systemTests, List<String> stagingTests, List<String> productionTests) { - return new TestDescriptor(version, toMap(systemTests, stagingTests, productionTests)); + String version, List<String> systemTests, List<String> stagingTests, List<String> stagingSetupTests, List<String> productionTests) { + return new TestDescriptor(version, toMap(systemTests, stagingTests, stagingSetupTests, productionTests)); } private static Map<TestCategory, List<String>> toMap( - List<String> systemTests, List<String> stagingTests, List<String> productionTests) { + List<String> systemTests, List<String> stagingTests, List<String> stagingSetupTests, List<String> productionTests) { return Map.of( TestCategory.systemtest, systemTests, TestCategory.stagingtest, stagingTests, + TestCategory.stagingsetuptest, stagingSetupTests, TestCategory.productiontest, productionTests ); } @@ -81,6 +84,7 @@ public class TestDescriptor { addJsonArrayForTests(tests, JSON_FIELD_SYSTEM_TESTS, TestCategory.systemtest); addJsonArrayForTests(tests, JSON_FIELD_STAGING_TESTS, TestCategory.stagingtest); addJsonArrayForTests(tests, JSON_FIELD_PRODUCTION_TESTS, TestCategory.productiontest); + addJsonArrayForTests(tests, JSON_FIELD_STAGING_SETUP_TESTS, TestCategory.stagingsetuptest); ByteArrayOutputStream out = new ByteArrayOutputStream(); uncheck(() -> new JsonFormat(/*compact*/false).encode(out, slime)); return out.toString(); @@ -100,5 +104,5 @@ public class TestDescriptor { '}'; } - public enum TestCategory {systemtest, stagingtest, productiontest} + public enum TestCategory {systemtest, stagingsetuptest, stagingtest, productiontest} } diff --git a/hosted-api/src/test/java/ai/vespa/hosted/api/TestDescriptorTest.java b/hosted-api/src/test/java/ai/vespa/hosted/api/TestDescriptorTest.java index 7e59af9ced8..d78526c500b 100644 --- a/hosted-api/src/test/java/ai/vespa/hosted/api/TestDescriptorTest.java +++ b/hosted-api/src/test/java/ai/vespa/hosted/api/TestDescriptorTest.java @@ -33,6 +33,9 @@ public class TestDescriptorTest { var stagingTests = testClassDescriptor.getConfiguredTests(TestDescriptor.TestCategory.stagingtest); Assertions.assertIterableEquals(Collections.emptyList(), stagingTests); + var stagingSetupTests = testClassDescriptor.getConfiguredTests(TestDescriptor.TestCategory.stagingtest); + Assertions.assertIterableEquals(Collections.emptyList(), stagingSetupTests); + var productionTests = testClassDescriptor.getConfiguredTests(TestDescriptor.TestCategory.productiontest); Assertions.assertIterableEquals(Collections.emptyList(), productionTests); } @@ -40,7 +43,8 @@ public class TestDescriptorTest { @Test public void parsesDescriptorFile() { String testDescriptor = "{\n" + - " \"version\": \"1.0\",\n" + + " \"" + + "version\": \"1.0\",\n" + " \"configuredTests\": {\n" + " \"systemTests\": [\n" + " \"ai.vespa.test.SystemTest1\",\n" + @@ -50,6 +54,10 @@ public class TestDescriptorTest { " \"ai.vespa.test.StagingTest1\",\n" + " \"ai.vespa.test.StagingTest2\"\n" + " ],\n" + + " \"stagingSetupTests\": [\n" + + " \"ai.vespa.test.StagingSetupTest1\",\n" + + " \"ai.vespa.test.StagingSetupTest2\"\n" + + " ],\n" + " \"productionTests\": [\n" + " \"ai.vespa.test.ProductionTest1\",\n" + " \"ai.vespa.test.ProductionTest2\"\n" + @@ -65,8 +73,13 @@ public class TestDescriptorTest { var stagingTests = testClassDescriptor.getConfiguredTests(TestDescriptor.TestCategory.stagingtest); Assertions.assertIterableEquals(List.of("ai.vespa.test.StagingTest1", "ai.vespa.test.StagingTest2"), stagingTests); + var stagingSetupTests = testClassDescriptor.getConfiguredTests(TestDescriptor.TestCategory.stagingsetuptest); + Assertions.assertIterableEquals(List.of("ai.vespa.test.StagingSetupTest1", "ai.vespa.test.StagingSetupTest2"), stagingSetupTests); + var productionTests = testClassDescriptor.getConfiguredTests(TestDescriptor.TestCategory.productiontest); Assertions.assertIterableEquals(List.of("ai.vespa.test.ProductionTest1", "ai.vespa.test.ProductionTest2"), productionTests); + + JsonTestHelper.assertJsonEquals(testClassDescriptor.toJson(), testDescriptor); } @Test diff --git a/hosted-tenant-base/pom.xml b/hosted-tenant-base/pom.xml index 03466255362..d453da74b74 100644 --- a/hosted-tenant-base/pom.xml +++ b/hosted-tenant-base/pom.xml @@ -255,7 +255,10 @@ </goals> <configuration> <tasks> - <copy file="target/${project.artifactId}-tests.jar" todir="target/application-test/artifacts/" /> + <!-- Creating a dummy file to support running tests with old test runner. Remove when it is no longer in use --> + <mkdir dir="target/application-test/artifacts" /> + <touch file="target/application-test/artifacts/.ignore" /> + <copy file="target/${project.artifactId}-tests.jar" todir="target/application-test/components/" /> <zip destfile="target/application-test.zip" basedir="target/application-test/" /> </tasks> </configuration> diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index f1645727b7c..30ca2e0d218 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.maintenance; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; +import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.NodeType; import java.util.logging.Level; import com.yahoo.vespa.hosted.dockerapi.Container; @@ -37,6 +38,7 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.function.Function; import java.util.logging.Logger; +import java.util.stream.Stream; import static com.yahoo.vespa.hosted.node.admin.maintenance.disk.DiskCleanupRule.Priority; import static com.yahoo.yolean.Exceptions.uncheck; @@ -158,7 +160,7 @@ public class StorageMaintainer { attributes.put("kernel_version", System.getProperty("os.version")); attributes.put("cpu_microcode_version", getMicrocodeVersion()); - container.map(c -> c.image).ifPresent(image -> attributes.put("docker_image", image.asString())); + attributes.put("docker_image", getDockerImage(context, container)); context.node().parentHostname().ifPresent(parent -> attributes.put("parent_hostname", parent)); context.node().currentVespaVersion().ifPresent(version -> attributes.put("vespa_version", version.toFullString())); context.node().owner().ifPresent(owner -> { @@ -199,4 +201,12 @@ public class StorageMaintainer { return results[1].trim(); } + + private String getDockerImage(NodeAgentContext context, Optional<Container> container) { + return container.map(c -> c.image.asString()) + .orElse(context.node().currentDockerImage() + .map(DockerImage::asString) + .orElse("<none>") + ); + } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index df8a7e45917..e8639561599 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -260,7 +260,6 @@ class NodeAllocation { node = node.unretire(); } else { ++wasRetiredJustNow; - // Retire nodes which are of an unwanted flavor, retired flavor or have an overlapping parent host node = node.retire(nodeRepository.clock().instant()); } if ( ! node.allocation().get().membership().cluster().equals(cluster)) { diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributesconfigscout.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributesconfigscout.cpp index 986571d07e1..7f6d47f4ae2 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributesconfigscout.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributesconfigscout.cpp @@ -30,6 +30,7 @@ AttributesConfigScout::adjust(AttributesConfig::Attribute &attr, attr.huge = liveAttr.huge; // Note: Predicate attributes only handle changes for the dense-posting-list-threshold config. attr.densepostinglistthreshold = liveAttr.densepostinglistthreshold; + attr.distancemetric = liveAttr.distancemetric; attr.index = liveAttr.index; } diff --git a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp index 3e87749c794..407129199e3 100644 --- a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp @@ -179,12 +179,15 @@ AdaptiveSequencedExecutor::exchange_strand(Worker &worker, std::unique_lock<std: return true; } -AdaptiveSequencedExecutor::Task::UP -AdaptiveSequencedExecutor::next_task(Worker &worker) +AdaptiveSequencedExecutor::TaggedTask +AdaptiveSequencedExecutor::next_task(Worker &worker, std::optional<uint32_t> prev_token) { - Task::UP task; + TaggedTask task; Worker *worker_to_wake = nullptr; auto guard = std::unique_lock(_mutex); + if (prev_token.has_value()) { + _barrier.completeEvent(prev_token.value()); + } if (exchange_strand(worker, guard)) { assert(worker.state == Worker::State::RUNNING); assert(worker.strand != nullptr); @@ -212,8 +215,10 @@ void AdaptiveSequencedExecutor::worker_main() { Worker worker; - while (Task::UP my_task = next_task(worker)) { - my_task->run(); + std::optional<uint32_t> prev_token = std::nullopt; + while (TaggedTask my_task = next_task(worker, prev_token)) { + my_task.task->run(); + prev_token = my_task.token; } _thread_tools->allow_worker_exit.await(); } @@ -267,9 +272,9 @@ AdaptiveSequencedExecutor::executeTask(ExecutorId id, Task::UP task) assert(id.getId() < _strands.size()); Strand &strand = _strands[id.getId()]; auto guard = std::unique_lock(_mutex); - maybe_block_self(guard); assert(_self.state != Self::State::CLOSED); - strand.queue.push(std::move(task)); + maybe_block_self(guard); + strand.queue.push(TaggedTask(std::move(task), _barrier.startEvent())); _stats.queueSize.add(++_self.pending_tasks); ++_stats.acceptedTasks; if (strand.state == Strand::State::WAITING) { @@ -297,11 +302,14 @@ AdaptiveSequencedExecutor::executeTask(ExecutorId id, Task::UP task) void AdaptiveSequencedExecutor::sync() { - vespalib::CountDownLatch latch(_strands.size()); - for (size_t i = 0; i < _strands.size(); ++i) { - execute(ExecutorId(i), [&](){ latch.countDown(); }); + BarrierCompletion barrierCompletion; + { + auto guard = std::lock_guard(_mutex); + if (!_barrier.startBarrier(barrierCompletion)) { + return; + } } - latch.await(); + barrierCompletion.gate.await(); } void diff --git a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h index a4d3ac97758..c52b9b22245 100644 --- a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h +++ b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h @@ -5,9 +5,12 @@ #include "isequencedtaskexecutor.h" #include <vespa/vespalib/util/arrayqueue.hpp> #include <vespa/vespalib/util/gate.h> +#include <vespa/vespalib/util/eventbarrier.hpp> +#include <vespa/vespalib/util/gate.h> #include <vespa/fastos/thread.h> #include <mutex> #include <condition_variable> +#include <optional> #include <cassert> namespace vespalib { @@ -23,6 +26,24 @@ private: using Stats = vespalib::ExecutorStats; using Task = vespalib::Executor::Task; + struct TaggedTask { + Task::UP task; + uint32_t token; + TaggedTask() : task(nullptr), token(0) {} + TaggedTask(Task::UP task_in, uint32_t token_in) + : task(std::move(task_in)), token(token_in) {} + TaggedTask(TaggedTask &&rhs) = default; + TaggedTask(const TaggedTask &rhs) = delete; + TaggedTask &operator=(const TaggedTask &rhs) = delete; + TaggedTask &operator=(TaggedTask &&rhs) { + assert(task.get() == nullptr); // no overwrites + task = std::move(rhs.task); + token = rhs.token; + return *this; + } + operator bool() const { return bool(task); } + }; + /** * Values used to configure the executor. **/ @@ -51,7 +72,7 @@ private: struct Strand { enum class State { IDLE, WAITING, ACTIVE }; State state; - vespalib::ArrayQueue<Task::UP> queue; + vespalib::ArrayQueue<TaggedTask> queue; Strand(); ~Strand(); }; @@ -96,11 +117,17 @@ private: void close(); }; + struct BarrierCompletion { + Gate gate; + void completeBarrier() { gate.countDown(); } + }; + std::unique_ptr<ThreadTools> _thread_tools; std::mutex _mutex; std::vector<Strand> _strands; vespalib::ArrayQueue<Strand*> _wait_queue; vespalib::ArrayQueue<Worker*> _worker_stack; + EventBarrier<BarrierCompletion> _barrier; Self _self; Stats _stats; Config _cfg; @@ -111,7 +138,7 @@ private: Worker *get_worker_to_wake(const std::unique_lock<std::mutex> &lock); bool obtain_strand(Worker &worker, std::unique_lock<std::mutex> &lock); bool exchange_strand(Worker &worker, std::unique_lock<std::mutex> &lock); - Task::UP next_task(Worker &worker); + TaggedTask next_task(Worker &worker, std::optional<uint32_t> prev_token); void worker_main(); public: AdaptiveSequencedExecutor(size_t num_strands, size_t num_threads, diff --git a/storage/src/tests/distributor/bucketdatabasetest.cpp b/storage/src/tests/distributor/bucketdatabasetest.cpp index a0354c8ad4e..0b832699364 100644 --- a/storage/src/tests/distributor/bucketdatabasetest.cpp +++ b/storage/src/tests/distributor/bucketdatabasetest.cpp @@ -610,7 +610,7 @@ struct InsertBeforeBucketMergingProcessor : BucketDatabase::MergingProcessor { Result merge(Merger& m) override { if (m.bucket_id() == _before_bucket) { // Assumes _before_bucket is > the inserted bucket - m.insert_before_current(BucketDatabase::Entry(document::BucketId(16, 2), BI(2))); + m.insert_before_current(document::BucketId(16, 2), BucketDatabase::Entry(document::BucketId(16, 2), BI(2))); } return Result::KeepUnchanged; } @@ -622,7 +622,7 @@ struct InsertAtEndMergingProcessor : BucketDatabase::MergingProcessor { } void insert_remaining_at_end(TrailingInserter& inserter) override { - inserter.insert_at_end(BucketDatabase::Entry(document::BucketId(16, 3), BI(3))); + inserter.insert_at_end(document::BucketId(16, 3), BucketDatabase::Entry(document::BucketId(16, 3), BI(3))); } }; diff --git a/storage/src/vespa/storage/bucketdb/abstract_bucket_map.h b/storage/src/vespa/storage/bucketdb/abstract_bucket_map.h index 6c669beab1c..a6855532e30 100644 --- a/storage/src/vespa/storage/bucketdb/abstract_bucket_map.h +++ b/storage/src/vespa/storage/bucketdb/abstract_bucket_map.h @@ -151,7 +151,6 @@ public: static constexpr uint32_t DEFAULT_CHUNK_SIZE = 1000; - /** * Iterate over the entire database contents, holding the global database * mutex for `chunkSize` processed entries at a time, yielding the current diff --git a/storage/src/vespa/storage/bucketdb/btree_bucket_database.cpp b/storage/src/vespa/storage/bucketdb/btree_bucket_database.cpp index 42bd3a247bb..9634d6d0953 100644 --- a/storage/src/vespa/storage/bucketdb/btree_bucket_database.cpp +++ b/storage/src/vespa/storage/bucketdb/btree_bucket_database.cpp @@ -1,22 +1,10 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "btree_bucket_database.h" -#include <vespa/vespalib/btree/btreebuilder.h> -#include <vespa/vespalib/btree/btreenodeallocator.hpp> -#include <vespa/vespalib/btree/btreenode.hpp> -#include <vespa/vespalib/btree/btreenodestore.hpp> -#include <vespa/vespalib/btree/btreeiterator.hpp> -#include <vespa/vespalib/btree/btreeroot.hpp> -#include <vespa/vespalib/btree/btreebuilder.hpp> -#include <vespa/vespalib/btree/btree.hpp> -#include <vespa/vespalib/btree/btreestore.hpp> +#include "generic_btree_bucket_database.hpp" #include <vespa/vespalib/datastore/array_store.hpp> #include <iostream> -// TODO remove once this impl uses the generic bucket B-tree code! -#include "generic_btree_bucket_database.h" -#include <vespa/vespalib/datastore/datastore.h> - /* * Buckets in our tree are represented by their 64-bit numeric key, in what's known as * "reversed bit order with appended used-bits" form. I.e. a bucket ID (16, 0xcafe), which @@ -74,232 +62,53 @@ uint64_t value_from(uint32_t gc_timestamp, EntryRef ref) { return ((uint64_t(gc_timestamp) << 32u) | ref.ref()); } -// TODO dedupe and unify common code -uint8_t -getMinDiffBits(uint16_t minBits, const document::BucketId& a, const document::BucketId& b) { - for (uint32_t i = minBits; i <= std::min(a.getUsedBits(), b.getUsedBits()); i++) { - document::BucketId a1(i, a.getRawId()); - document::BucketId b1(i, b.getRawId()); - if (b1.getId() != a1.getId()) { - return i; - } - } - return minBits; } -uint8_t next_parent_bit_seek_level(uint8_t minBits, const document::BucketId& a, const document::BucketId& b) { - const uint8_t min_used = std::min(a.getUsedBits(), b.getUsedBits()); - assert(min_used >= minBits); // Always monotonically descending towards leaves - for (uint32_t i = minBits; i <= min_used; i++) { - document::BucketId a1(i, a.getRawId()); - document::BucketId b1(i, b.getRawId()); - if (b1.getId() != a1.getId()) { - return i; - } - } - // The bit prefix is equal, which means that one node is a parent of the other. In this - // case we have to force the seek to continue from the next level in the tree. - return std::max(min_used, minBits) + 1; -} +struct BTreeBucketDatabase::ReplicaValueTraits { + using ValueType = Entry; + using ConstValueRef = ConstEntryRef; + using DataStoreType = vespalib::datastore::ArrayStore<BucketCopy>; -// TODO getMinDiffBits is hoisted from lockablemap.cpp, could probably be rewritten in terms of xor and MSB bit scan instr -/* - * 63 -------- ... -> 0 - * a: 1101111111 ... 0010 - * b: 1101110010 ... 0011 - * a ^ b: 0000001101 ... 0001 - * ^- diff bit = 57 - * - * 63 - vespalib::Optimized::msbIdx(a ^ b) ==> 6 - * - * what if a == b? special case? not a problem if we can prove this never happens. - */ + static ValueType make_invalid_value() { + return Entry::createInvalid(); + } + static uint64_t wrap_and_store_value(DataStoreType& store, const Entry& entry) noexcept { + auto replicas_ref = store.add(entry.getBucketInfo().getRawNodes()); + return value_from(entry.getBucketInfo().getLastGarbageCollectionTime(), replicas_ref); + } + static void remove_by_wrapped_value(DataStoreType& store, uint64_t value) noexcept { + store.remove(entry_ref_from_value(value)); + } + static ValueType unwrap_from_key_value(const DataStoreType& store, uint64_t key, uint64_t value) { + const auto replicas_ref = store.get(entry_ref_from_value(value)); + const auto bucket = BucketId(BucketId::keyToBucketId(key)); + return entry_from_replica_array_ref(bucket, gc_timestamp_from_value(value), replicas_ref); + } + static ConstValueRef unwrap_const_ref_from_key_value(const DataStoreType& store, uint64_t key, uint64_t value) { + const auto replicas_ref = store.get(entry_ref_from_value(value)); + const auto bucket = BucketId(BucketId::keyToBucketId(key)); + return const_entry_ref_from_replica_array_ref(bucket, gc_timestamp_from_value(value), replicas_ref); + } +}; -} +template class bucketdb::GenericBTreeBucketDatabase<BTreeBucketDatabase::ReplicaValueTraits>; BTreeBucketDatabase::BTreeBucketDatabase() - : _tree(), - _store(make_default_array_store_config<ReplicaStore>()), - _generation_handler() + : _impl(std::make_unique<ImplType>(make_default_array_store_config<ReplicaValueTraits::DataStoreType>())) { } BTreeBucketDatabase::~BTreeBucketDatabase() = default; -void BTreeBucketDatabase::commit_tree_changes() { - // TODO break up and refactor - // TODO verify semantics and usage - // TODO make BTree wrapping API which abstracts away all this stuff via reader/writer interfaces - _tree.getAllocator().freeze(); - - auto current_gen = _generation_handler.getCurrentGeneration(); - _store.transferHoldLists(current_gen); - _tree.getAllocator().transferHoldLists(current_gen); - - _generation_handler.incGeneration(); - - auto used_gen = _generation_handler.getFirstUsedGeneration(); - _store.trimHoldLists(used_gen); - _tree.getAllocator().trimHoldLists(used_gen); -} - -Entry BTreeBucketDatabase::entry_from_value(uint64_t bucket_key, uint64_t value) const { - const auto replicas_ref = _store.get(entry_ref_from_value(value)); - const auto bucket = BucketId(BucketId::keyToBucketId(bucket_key)); - return entry_from_replica_array_ref(bucket, gc_timestamp_from_value(value), replicas_ref); -} - -Entry BTreeBucketDatabase::entry_from_iterator(const BTree::ConstIterator& iter) const { - if (!iter.valid()) { - return Entry::createInvalid(); - } - const auto value = iter.getData(); - std::atomic_thread_fence(std::memory_order_acquire); - return entry_from_value(iter.getKey(), value); -} - -ConstEntryRef BTreeBucketDatabase::const_entry_ref_from_iterator(const BTree::ConstIterator& iter) const { - if (!iter.valid()) { - return ConstEntryRef::createInvalid(); - } - const auto value = iter.getData(); - std::atomic_thread_fence(std::memory_order_acquire); - const auto replicas_ref = _store.get(entry_ref_from_value(value)); - const auto bucket = BucketId(BucketId::keyToBucketId(iter.getKey())); - return const_entry_ref_from_replica_array_ref(bucket, gc_timestamp_from_value(value), replicas_ref); -} - -BucketId BTreeBucketDatabase::bucket_from_valid_iterator(const BTree::ConstIterator& iter) const { - return BucketId(BucketId::keyToBucketId(iter.getKey())); -} - Entry BTreeBucketDatabase::get(const BucketId& bucket) const { - return entry_from_iterator(_tree.find(bucket.toKey())); + return _impl->get(bucket); } void BTreeBucketDatabase::remove(const BucketId& bucket) { - auto iter = _tree.find(bucket.toKey()); - if (!iter.valid()) { - return; - } - const auto value = iter.getData(); - _store.remove(entry_ref_from_value(value)); - _tree.remove(iter); - commit_tree_changes(); + _impl->remove(bucket); } -/* - * Finding the complete set of parents of a given bucket is not obvious how to - * do efficiently, as we only know that the parents are ordered before their - * children, but we do not a-priori know if any exist at all. The Judy DB impl - * does O(b) explicit point lookups (where b is the number of used bits in the - * bucket), starting at the leaf bit and working towards the root. To avoid - * having to re-create iterators and perform a full tree search every time, we - * turn this on its head and start from the root, progressing towards the leaf. - * This allows us to reuse a single iterator and to continue seeking forwards - * from its current position. - * - * To speed up the process of converging on the target bucket without needing - * to check many unrelated subtrees, we let the underlying B-tree automatically - * aggregate the min/max range of the used-bits of all contained bucket keys. - * If we e.g. know that the minimum number of used bits in the DB is 16, we can - * immediately seek to this level in the tree instead of working our way down - * one bit at a time. By definition, no parents can exist above this level. - * This is a very important optimization, as bucket trees are usually very well - * balanced due to randomized distribution of data (combined with a cluster-wide - * minimum tree level imposed by distribution bits). It is common that the minimum - * number of used bits == max number of used bits, i.e. a totally even split. - * This means that for a system without inconsistently split buckets (i.e. no - * parents) we're highly likely to converge on the target bucket in a single seek. - * - * Algorithm: - * - * Core invariant: every subsequent iterator seek performed in this algorithm - * is for a key that is strictly higher than the one the iterator is currently at. - * - * 1. Lbound seek to the lowest key that is known to exclude all already visited - * parents. On the first iteration we use a bit count equal to the minimum number - * of key used-bits in the entire DB, allowing us to potentially skip most subtrees. - * 2. If the current node's key is greater than that of the requested bucket's key, - * we've either descended to--or beyond--it in its own subtree or we've entered - * a disjoint subtree. Since we know that all parents must sort before any given - * child bucket, no more parents may be found at this point. Algorithm terminates. - * 3. As the main body of the loop is entered, we know one of following must hold: - * 3.1 The current node is an explicitly present parent of our bucket. - * 3.2 The current node is contained in a left subtree branch of a parent that - * does not have a bucket explicitly present in the tree. It cannot be in - * a right subtree of any parent, as that would imply the node is ordered - * _after_ our own bucket in an in-order traversal, which would contradict - * the check in step 2 above. - * 4. If the current node contains the requested bucket, we're at a parent - * node of the bucket; add it to the result set. - * If this is _not_ the case, we're in a different subtree. Example: the - * requested bucket has a key whose MSB is 1 but the first bucket in the - * tree has a key with an MSB of 0. Either way we need to update our search - * key to home in on the target subtree where more parents may be found; - * 5. Update the seek key to find the next possible parent. To ensure this key is - * strictly greater than the iterator's current key we find the largest shared - * prefix of bits in common between the current node's key and the requested - * bucket's key. The prefix length + 1 is then the depth in the tree at which the - * two subtrees branch off and diverge. - * The new key is then the MSB prefix length + 1 requested bucket's key with a - * matching number of used-bits set. Forward lbound-seek the iterator to this key. - * `--> TODO elaborate on prefix semantics when they are equal wrt. min used bits - * 6. Iff iterator is still valid, go to step 2 - * - * This algorithm is able to skip through large parts of the tree in a sparsely populated - * tree, but the number of seeks will trend towards O(b - min_bits) as with the legacy - * implementation when a tree is densely populated (where `b` is the used-bits count of the - * most specific node in the tree for the target bucket, and min_bits is the minimum number - * of used-bits for any key in the database). This because all logical inner nodes in the tree - * will have subtrees under them. Even in the worst case we should be more efficient than the - * legacy Judy-based implementation since we've cut any dense search space in half for each - * invocation of seek() on the iterator. - */ -BTreeBucketDatabase::BTree::ConstIterator -BTreeBucketDatabase::find_parents_internal(const BTree::FrozenView& frozen_view, - const document::BucketId& bucket, - std::vector<Entry>& entries) const -{ - const uint64_t bucket_key = bucket.toKey(); - if (frozen_view.empty()) { - return frozen_view.begin(); // Will be invalid. - } - const auto min_db_bits = frozen_view.getAggregated().getMin(); - assert(min_db_bits >= static_cast<int32_t>(BucketId::minNumBits)); - assert(min_db_bits <= static_cast<int32_t>(BucketId::maxNumBits)); - // Start at the lowest possible tree level no parents can exist above, - // descending towards the bucket itself. - // Note: important to use getId() rather than getRawId(), as min_db_bits may be - // greater than the used bits of the queried bucket. If we used the raw ID, we'd - // end up looking at undefined bits. - const auto first_key = BucketId(min_db_bits, bucket.getId()).toKey(); - auto iter = frozen_view.lowerBound(first_key); - // Try skipping as many levels of the tree as possible as we go. - uint32_t bits = min_db_bits; - while (iter.valid() && (iter.getKey() < bucket_key)) { - auto candidate = BucketId(BucketId::keyToBucketId(iter.getKey())); - if (candidate.contains(bucket)) { - assert(candidate.getUsedBits() >= bits); - entries.emplace_back(entry_from_iterator(iter)); - } - bits = next_parent_bit_seek_level(bits, candidate, bucket); - const auto parent_key = BucketId(bits, bucket.getRawId()).toKey(); - assert(parent_key > iter.getKey()); - iter.seek(parent_key); - } - return iter; -} - -void BTreeBucketDatabase::find_parents_and_self_internal(const BTree::FrozenView& frozen_view, - const document::BucketId& bucket, - std::vector<Entry>& entries) const -{ - auto iter = find_parents_internal(frozen_view, bucket, entries); - if (iter.valid() && iter.getKey() == bucket.toKey()) { - entries.emplace_back(entry_from_iterator(iter)); - } -} +using bucketdb::ByValue; /* * Note: due to legacy API reasons, iff the requested bucket itself exists in the @@ -309,212 +118,53 @@ void BTreeBucketDatabase::find_parents_and_self_internal(const BTree::FrozenView void BTreeBucketDatabase::getParents(const BucketId& bucket, std::vector<Entry>& entries) const { - auto view = _tree.getFrozenView(); - find_parents_and_self_internal(view, bucket, entries); + _impl->find_parents_and_self<ByValue>(bucket, [&entries]([[maybe_unused]] uint64_t key, Entry entry){ + entries.emplace_back(std::move(entry)); + }); } void BTreeBucketDatabase::getAll(const BucketId& bucket, std::vector<Entry>& entries) const { - auto view = _tree.getFrozenView(); - auto iter = find_parents_internal(view, bucket, entries); - // `iter` is already pointing at, or beyond, one of the bucket's subtrees. - for (; iter.valid(); ++iter) { - auto candidate = BucketId(BucketId::keyToBucketId(iter.getKey())); - if (bucket.contains(candidate)) { - entries.emplace_back(entry_from_iterator(iter)); - } else { - break; - } - } + _impl->find_parents_self_and_children<ByValue>(bucket, [&entries]([[maybe_unused]] uint64_t key, Entry entry){ + entries.emplace_back(std::move(entry)); + }); } void BTreeBucketDatabase::update(const Entry& newEntry) { assert(newEntry.valid()); - auto replicas_ref = _store.add(newEntry.getBucketInfo().getRawNodes()); - const auto new_value = value_from(newEntry.getBucketInfo().getLastGarbageCollectionTime(), replicas_ref); - const auto bucket_key = newEntry.getBucketId().toKey(); - auto iter = _tree.lowerBound(bucket_key); - if (iter.valid() && (iter.getKey() == bucket_key)) { - _store.remove(entry_ref_from_value(iter.getData())); - // In-place update of value; does not require tree structure modification - std::atomic_thread_fence(std::memory_order_release); // Must ensure visibility when new array ref is observed - iter.writeData(new_value); - } else { - _tree.insert(iter, bucket_key, new_value); - } - commit_tree_changes(); // TODO does publishing a new root imply an implicit memory fence? + _impl->update(newEntry.getBucketId(), newEntry); } // TODO need snapshot read with guarding // FIXME semantics of for-each in judy and bit tree DBs differ, former expects lbound, latter ubound..! // FIXME but bit-tree code says "lowerBound" in impl and "after" in declaration??? void BTreeBucketDatabase::forEach(EntryProcessor& proc, const BucketId& after) const { - for (auto iter = _tree.upperBound(after.toKey()); iter.valid(); ++iter) { - if (!proc.process(const_entry_ref_from_iterator(iter))) { + for (auto iter = _impl->upper_bound(after.toKey()); iter.valid(); ++iter) { + if (!proc.process(_impl->const_value_ref_from_valid_iterator(iter))) { break; } } } -struct BTreeBuilderMerger final : BucketDatabase::Merger { - BTreeBucketDatabase& _db; - BTreeBucketDatabase::BTree::Builder& _builder; - uint64_t _current_key; - uint64_t _current_value; - Entry _cached_entry; - bool _valid_cached_entry; - - BTreeBuilderMerger(BTreeBucketDatabase& db, - BTreeBucketDatabase::BTree::Builder& builder) - : _db(db), - _builder(builder), - _current_key(0), - _current_value(0), - _cached_entry(), - _valid_cached_entry(false) - {} - ~BTreeBuilderMerger() override = default; - - uint64_t bucket_key() const noexcept override { - return _current_key; - } - BucketId bucket_id() const noexcept override { - return BucketId(BucketId::keyToBucketId(_current_key)); - } - Entry& current_entry() override { - if (!_valid_cached_entry) { - _cached_entry = _db.entry_from_value(_current_key, _current_value); - _valid_cached_entry = true; - } - return _cached_entry; - } - void insert_before_current(const Entry& e) override { - const uint64_t bucket_key = e.getBucketId().toKey(); - assert(bucket_key < _current_key); - - auto replicas_ref = _db._store.add(e.getBucketInfo().getRawNodes()); - const auto new_value = value_from(e.getBucketInfo().getLastGarbageCollectionTime(), replicas_ref); - - _builder.insert(bucket_key, new_value); - } - - void update_iteration_state(uint64_t key, uint64_t value) { - _current_key = key; - _current_value = value; - _valid_cached_entry = false; - } -}; - -struct BTreeTrailingInserter final : BucketDatabase::TrailingInserter { - BTreeBucketDatabase& _db; - BTreeBucketDatabase::BTree::Builder& _builder; - - BTreeTrailingInserter(BTreeBucketDatabase& db, - BTreeBucketDatabase::BTree::Builder& builder) - : _db(db), - _builder(builder) - {} - - ~BTreeTrailingInserter() override = default; - - void insert_at_end(const Entry& e) override { - const uint64_t bucket_key = e.getBucketId().toKey(); - const auto replicas_ref = _db._store.add(e.getBucketInfo().getRawNodes()); - const auto new_value = value_from(e.getBucketInfo().getLastGarbageCollectionTime(), replicas_ref); - _builder.insert(bucket_key, new_value); - } -}; - -// TODO lbound arg? void BTreeBucketDatabase::merge(MergingProcessor& proc) { - BTreeBucketDatabase::BTree::Builder builder(_tree.getAllocator()); - BTreeBuilderMerger merger(*this, builder); - - // TODO for_each instead? - for (auto iter = _tree.begin(); iter.valid(); ++iter) { - const uint64_t key = iter.getKey(); - const uint64_t value = iter.getData(); - merger.update_iteration_state(key, value); - - auto result = proc.merge(merger); - - if (result == MergingProcessor::Result::KeepUnchanged) { - builder.insert(key, value); // Reuse array store ref with no changes - } else if (result == MergingProcessor::Result::Update) { - assert(merger._valid_cached_entry); // Must actually have been touched - assert(merger._cached_entry.valid()); - _store.remove(entry_ref_from_value(value)); - auto new_replicas_ref = _store.add(merger._cached_entry.getBucketInfo().getRawNodes()); - const auto new_value = value_from(merger._cached_entry.getBucketInfo().getLastGarbageCollectionTime(), new_replicas_ref); - builder.insert(key, new_value); - } else if (result == MergingProcessor::Result::Skip) { - _store.remove(entry_ref_from_value(value)); - } else { - abort(); - } - } - BTreeTrailingInserter inserter(*this, builder); - proc.insert_remaining_at_end(inserter); - - _tree.assign(builder); - commit_tree_changes(); + _impl->merge(proc); } Entry BTreeBucketDatabase::upperBound(const BucketId& bucket) const { - return entry_from_iterator(_tree.upperBound(bucket.toKey())); + return _impl->entry_from_iterator(_impl->upper_bound(bucket.toKey())); } uint64_t BTreeBucketDatabase::size() const { - return _tree.size(); + return _impl->size(); } void BTreeBucketDatabase::clear() { - _tree.clear(); - commit_tree_changes(); + _impl->clear(); } -/* - * Returns the bucket ID which, based on the buckets already existing in the DB, - * is the most specific location in the tree in which it should reside. This may - * or may not be a bucket that already exists. - * - * Example: if there is a single bucket (1, 1) in the tree, a query for (1, 1) or - * (1, 3) will return (1, 1) as that is the most specific leaf in that subtree. - * A query for (1, 0) will return (1, 0) even though this doesn't currently exist, - * as there is no existing bucket that can contain the queried bucket. It is up to - * the caller to create this bucket according to its needs. - * - * Usually this function will be called with an ID whose used-bits is at max (58), in - * order to find a leaf bucket to route an incoming document operation to. - * - * TODO rename this function, it's very much _not_ obvious what an "appropriate" bucket is..! - * TODO this should be possible to do concurrently - */ BucketId BTreeBucketDatabase::getAppropriateBucket(uint16_t minBits, const BucketId& bid) { - // The bucket tree is ordered in such a way that it represents a - // natural in-order traversal of all buckets, with inner nodes being - // visited before leaf nodes. This means that a lower bound seek will - // never return a parent of a seeked bucket. The iterator will be pointing - // to a bucket that is either the actual bucket given as the argument to - // lowerBound() or the next in-order bucket (or end() if none exists). - auto iter = _tree.lowerBound(bid.toKey()); - if (iter.valid()) { - // Find the first level in the tree where the paths through the bucket tree - // diverge for the target bucket and the current bucket. - minBits = getMinDiffBits(minBits, bucket_from_valid_iterator(iter), bid); - } - // TODO is it better to copy original iterator and do begin() on the copy? - auto first_iter = _tree.begin(); - // Original iterator might be in a different subtree than that of our - // target bucket. If possible, rewind one node to discover any parent or - // leftmost sibling of our node. If there's no such node, we'll still - // discover the greatest equal bit prefix. - if (iter != first_iter) { - --iter; - minBits = getMinDiffBits(minBits, bucket_from_valid_iterator(iter), bid); - } - return BucketId(minBits, bid.getRawId()); + return _impl->getAppropriateBucket(minBits, bid); } /* @@ -527,24 +177,7 @@ BucketId BTreeBucketDatabase::getAppropriateBucket(uint16_t minBits, const Bucke */ // TODO rename/clarify to indicate this is child _subtrees_, not explicit child _buckets_! uint32_t BTreeBucketDatabase::childCount(const BucketId& bucket) const { - assert(bucket.getUsedBits() < BucketId::maxNumBits); - BucketId lhs_bucket(bucket.getUsedBits() + 1, bucket.getId()); - BucketId rhs_bucket(bucket.getUsedBits() + 1, (1ULL << bucket.getUsedBits()) | bucket.getId()); - - auto iter = _tree.lowerBound(lhs_bucket.toKey()); - if (!iter.valid()) { - return 0; - } - if (lhs_bucket.contains(bucket_from_valid_iterator(iter))) { - iter.seek(rhs_bucket.toKey()); - if (!iter.valid()) { - return 1; // lhs subtree only - } - return (rhs_bucket.contains(bucket_from_valid_iterator(iter)) ? 2 : 1); - } else if (rhs_bucket.contains(bucket_from_valid_iterator(iter))) { - return 1; // rhs subtree only - } - return 0; + return _impl->child_subtree_count(bucket); } void BTreeBucketDatabase::print(std::ostream& out, bool verbose, @@ -556,15 +189,22 @@ void BTreeBucketDatabase::print(std::ostream& out, bool verbose, } vespalib::MemoryUsage BTreeBucketDatabase::memory_usage() const noexcept { - auto mem_usage = _tree.getMemoryUsage(); - mem_usage.merge(_store.getMemoryUsage()); - return mem_usage; + return _impl->memory_usage(); } +class BTreeBucketDatabase::ReadGuardImpl final : public bucketdb::ReadGuard<Entry> { + ImplType::ReadSnapshot _snapshot; +public: + explicit ReadGuardImpl(const BTreeBucketDatabase& db); + ~ReadGuardImpl() override; + + void find_parents_and_self(const document::BucketId& bucket, + std::vector<Entry>& entries) const override; + [[nodiscard]] uint64_t generation() const noexcept override; +}; + BTreeBucketDatabase::ReadGuardImpl::ReadGuardImpl(const BTreeBucketDatabase& db) - : _db(&db), - _guard(_db->_generation_handler.takeGuard()), - _frozen_view(_db->_tree.getFrozenView()) + : _snapshot(*db._impl) {} BTreeBucketDatabase::ReadGuardImpl::~ReadGuardImpl() = default; @@ -572,52 +212,17 @@ BTreeBucketDatabase::ReadGuardImpl::~ReadGuardImpl() = default; void BTreeBucketDatabase::ReadGuardImpl::find_parents_and_self(const document::BucketId& bucket, std::vector<Entry>& entries) const { - _db->find_parents_and_self_internal(_frozen_view, bucket, entries); + _snapshot.find_parents_and_self<ByValue>(bucket, [&entries]([[maybe_unused]] uint64_t key, Entry entry){ + entries.emplace_back(std::move(entry)); + }); } uint64_t BTreeBucketDatabase::ReadGuardImpl::generation() const noexcept { - return _guard.getGeneration(); + return _snapshot.generation(); } -// TODO replace existing distributor DB code with generic impl. -// This is to ensure the generic implementation compiles with an ArrayStore backing in -// the meantime. -struct BTreeBucketDatabase2 { - struct ReplicaValueTraits { - using ValueType = Entry; - using ConstValueRef = ConstEntryRef; - using DataStoreType = vespalib::datastore::ArrayStore<BucketCopy>; - - static ValueType make_invalid_value() { - return Entry::createInvalid(); - } - static uint64_t wrap_and_store_value(DataStoreType& store, const Entry& entry) noexcept { - auto replicas_ref = store.add(entry.getBucketInfo().getRawNodes()); - return value_from(entry.getBucketInfo().getLastGarbageCollectionTime(), replicas_ref); - } - static void remove_by_wrapped_value(DataStoreType& store, uint64_t value) noexcept { - store.remove(entry_ref_from_value(value)); - } - static ValueType unwrap_from_key_value(const DataStoreType& store, uint64_t key, uint64_t value) { - const auto replicas_ref = store.get(entry_ref_from_value(value)); - const auto bucket = BucketId(BucketId::keyToBucketId(key)); - return entry_from_replica_array_ref(bucket, gc_timestamp_from_value(value), replicas_ref); - } - static ConstValueRef unwrap_const_ref_from_key_value(const DataStoreType& store, uint64_t key, uint64_t value) { - const auto replicas_ref = store.get(entry_ref_from_value(value)); - const auto bucket = BucketId(BucketId::keyToBucketId(key)); - return const_entry_ref_from_replica_array_ref(bucket, gc_timestamp_from_value(value), replicas_ref); - } - }; - - using BTreeImpl = bucketdb::GenericBTreeBucketDatabase<ReplicaValueTraits>; - BTreeImpl _impl; - - BTreeBucketDatabase2() - : _impl(make_default_array_store_config<ReplicaValueTraits::DataStoreType>()) - {} -}; - -template class bucketdb::GenericBTreeBucketDatabase<BTreeBucketDatabase2::ReplicaValueTraits>; +std::unique_ptr<bucketdb::ReadGuard<Entry>> BTreeBucketDatabase::acquire_read_guard() const { + return std::make_unique<ReadGuardImpl>(*this); +} } diff --git a/storage/src/vespa/storage/bucketdb/btree_bucket_database.h b/storage/src/vespa/storage/bucketdb/btree_bucket_database.h index 0dfa2b07b8a..122c6eeb0fb 100644 --- a/storage/src/vespa/storage/bucketdb/btree_bucket_database.h +++ b/storage/src/vespa/storage/bucketdb/btree_bucket_database.h @@ -3,13 +3,14 @@ #pragma once #include "bucketdatabase.h" -#include <vespa/vespalib/btree/btree.h> -#include <vespa/vespalib/btree/minmaxaggregated.h> -#include <vespa/vespalib/btree/minmaxaggrcalc.h> -#include <vespa/vespalib/datastore/array_store.h> +#include <memory> namespace storage { +namespace bucketdb { +template <typename DataStoreTraitsT> class GenericBTreeBucketDatabase; +} + /* * Bucket database implementation built around lock-free single-writer/multiple-readers B+tree. * @@ -25,27 +26,9 @@ namespace storage { */ // TODO create and use a new DB interface with better bulk loading, snapshot and iteration support class BTreeBucketDatabase : public BucketDatabase { - - struct KeyUsedBitsMinMaxAggrCalc : vespalib::btree::MinMaxAggrCalc { - constexpr static bool aggregate_over_values() { return false; } - constexpr static int32_t getVal(uint64_t key) noexcept { - static_assert(document::BucketId::CountBits == 6u); - return static_cast<int32_t>(key & 0b11'1111U); // 6 LSB of key contains used-bits - } - }; - - // Mapping from u64: bucket key -> <MSB u32: gc timestamp, LSB u32: ArrayStore ref> - using BTree = vespalib::btree::BTree<uint64_t, uint64_t, - vespalib::btree::MinMaxAggregated, - std::less<>, - vespalib::btree::BTreeDefaultTraits, - KeyUsedBitsMinMaxAggrCalc>; - using ReplicaStore = vespalib::datastore::ArrayStore<BucketCopy>; - using GenerationHandler = vespalib::GenerationHandler; - - BTree _tree; - ReplicaStore _store; - GenerationHandler _generation_handler; + struct ReplicaValueTraits; + using ImplType = bucketdb::GenericBTreeBucketDatabase<ReplicaValueTraits>; + std::unique_ptr<ImplType> _impl; public: BTreeBucketDatabase(); ~BTreeBucketDatabase() override; @@ -72,38 +55,10 @@ public: const std::string& indent) const override; private: - Entry entry_from_value(uint64_t bucket_key, uint64_t value) const; - Entry entry_from_iterator(const BTree::ConstIterator& iter) const; - ConstEntryRef const_entry_ref_from_iterator(const BTree::ConstIterator& iter) const; - document::BucketId bucket_from_valid_iterator(const BTree::ConstIterator& iter) const; - void commit_tree_changes(); - BTree::ConstIterator find_parents_internal(const BTree::FrozenView& frozen_view, - const document::BucketId& bucket, - std::vector<Entry>& entries) const; - void find_parents_and_self_internal(const BTree::FrozenView& frozen_view, - const document::BucketId& bucket, - std::vector<Entry>& entries) const; - - class ReadGuardImpl : public ReadGuard { - const BTreeBucketDatabase* _db; - GenerationHandler::Guard _guard; - BTree::FrozenView _frozen_view; - public: - explicit ReadGuardImpl(const BTreeBucketDatabase& db); - ~ReadGuardImpl() override; - - void find_parents_and_self(const document::BucketId& bucket, - std::vector<Entry>& entries) const override; - uint64_t generation() const noexcept override; - }; - + class ReadGuardImpl; friend class ReadGuardImpl; - friend struct BTreeBuilderMerger; - friend struct BTreeTrailingInserter; public: - std::unique_ptr<ReadGuard> acquire_read_guard() const override { - return std::make_unique<ReadGuardImpl>(*this); - } + std::unique_ptr<bucketdb::ReadGuard<Entry>> acquire_read_guard() const override; vespalib::MemoryUsage memory_usage() const noexcept override; }; diff --git a/storage/src/vespa/storage/bucketdb/btree_lockable_map.h b/storage/src/vespa/storage/bucketdb/btree_lockable_map.h index 136baefb615..3d58b85b063 100644 --- a/storage/src/vespa/storage/bucketdb/btree_lockable_map.h +++ b/storage/src/vespa/storage/bucketdb/btree_lockable_map.h @@ -24,7 +24,7 @@ template <typename DataStoreTraitsT> class GenericBTreeBucketDatabase; * Identical global and per-bucket locking semantics as LockableMap. */ template <typename T> -class BTreeLockableMap : public AbstractBucketMap<T> { +class BTreeLockableMap final : public AbstractBucketMap<T> { struct ValueTraits; public: using ParentType = AbstractBucketMap<T>; diff --git a/storage/src/vespa/storage/bucketdb/btree_lockable_map.hpp b/storage/src/vespa/storage/bucketdb/btree_lockable_map.hpp index 9c7228ae21d..18eae405dc6 100644 --- a/storage/src/vespa/storage/bucketdb/btree_lockable_map.hpp +++ b/storage/src/vespa/storage/bucketdb/btree_lockable_map.hpp @@ -439,7 +439,7 @@ BTreeLockableMap<T>::getContained(const BucketId& bucket, std::map<BucketId, WrappedEntry> results; std::vector<BucketId::Type> keys; - _impl->find_parents_and_self(bucket, [&keys](uint64_t key, [[maybe_unused]]const auto& value){ + _impl->template find_parents_and_self<ByConstRef>(bucket, [&keys](uint64_t key, [[maybe_unused]]const auto& value){ keys.emplace_back(key); }); @@ -454,7 +454,7 @@ template <typename T> void BTreeLockableMap<T>::getAllWithoutLocking(const BucketId& bucket, std::vector<BucketId::Type>& keys) { - _impl->find_parents_self_and_children(bucket, [&keys](uint64_t key, [[maybe_unused]]const auto& value){ + _impl->template find_parents_self_and_children<ByConstRef>(bucket, [&keys](uint64_t key, [[maybe_unused]]const auto& value){ keys.emplace_back(key); }); } @@ -480,7 +480,7 @@ template <typename T> bool BTreeLockableMap<T>::isConsistent(const BTreeLockableMap::WrappedEntry& entry) { std::lock_guard guard(_lock); uint64_t n_buckets = 0; - _impl->find_parents_self_and_children(entry.getBucketId(), + _impl->template find_parents_self_and_children<ByConstRef>(entry.getBucketId(), [&n_buckets]([[maybe_unused]] uint64_t key, [[maybe_unused]] const auto& value) { ++n_buckets; }); diff --git a/storage/src/vespa/storage/bucketdb/bucketdatabase.h b/storage/src/vespa/storage/bucketdb/bucketdatabase.h index 2dbcdd194ef..1ee165a739c 100644 --- a/storage/src/vespa/storage/bucketdb/bucketdatabase.h +++ b/storage/src/vespa/storage/bucketdb/bucketdatabase.h @@ -4,6 +4,8 @@ */ #pragma once +#include "db_merger.h" +#include "read_guard.h" #include <vespa/vespalib/util/printable.h> #include <vespa/storage/bucketdb/bucketinfo.h> #include <vespa/document/bucket/bucketid.h> @@ -84,104 +86,9 @@ public: EntryProcessor&, const document::BucketId& after = document::BucketId()) const = 0; - /** - * Database implementation-specific interface for appending entries - * during a merge() operation. - */ - struct TrailingInserter { - virtual ~TrailingInserter() = default; - /** - * Insert a new database entry at the end of the current bucket space. - * - * Precondition: the entry's bucket ID must sort after all entries that - * have already been iterated over or inserted via insert_at_end(). - */ - virtual void insert_at_end(const Entry&) = 0; - }; - - /** - * Database implementation-specific interface for accessing bucket - * entries and prepending entries during a merge() operation. - */ - struct Merger { - virtual ~Merger() = default; - - // TODO this should ideally be separated into read/write functions, but this - // will suffice for now to avoid too many changes. - - /** - * Bucket key/ID of the currently iterated entry. Unless the information stored - * in the DB Entry is needed, using one of these methods should be preferred to - * getting the bucket ID via current_entry(). The underlying DB is expected to - * have cheap access to the ID but _may_ have expensive access to the entry itself. - */ - virtual uint64_t bucket_key() const noexcept = 0; - virtual document::BucketId bucket_id() const noexcept = 0; - /** - * Returns a mutable representation of the currently iterated database - * entry. If changes are made to this object, Result::Update must be - * returned from merge(). Otherwise, mutation visibility is undefined. - */ - virtual Entry& current_entry() = 0; - /** - * Insert a new entry into the bucket database that is ordered before the - * currently iterated entry. - * - * Preconditions: - * - The entry's bucket ID must sort _before_ the currently iterated - * entry's bucket ID, in "reversed bits" bucket key order. - * - The entry's bucket ID must sort _after_ any entries previously - * inserted with insert_before_current(). - * - The entry's bucket ID must not be the same as a bucket that was - * already iterated over as part of the DB merge() call or inserted - * via a previous call to insert_before_current(). - * Such buckets must be handled by explicitly updating the provided - * entry for the iterated bucket and returning Result::Update. - */ - virtual void insert_before_current(const Entry&) = 0; - }; - - /** - * Interface to be implemented by callers that wish to receive callbacks - * during a bucket merge() operation. - */ - struct MergingProcessor { - // See merge() for semantics on enum values. - enum class Result { - Update, - KeepUnchanged, - Skip - }; - - virtual ~MergingProcessor() = default; - /** - * Invoked for each existing bucket in the database, in bucket key order. - * The provided Merge instance may be used to access the current entry - * and prepend entries to the DB. - * - * Return value semantics: - * - Result::Update: - * when merge() returns, the changes made to the current entry will - * become visible in the bucket database. - * - Result::KeepUnchanged: - * when merge() returns, the entry will remain in the same state as - * it was when merge() was originally called. - * - Result::Skip: - * when merge() returns, the entry will no longer be part of the DB. - * Any entries added via insert_before_current() _will_ be present. - * - */ - virtual Result merge(Merger&) = 0; - /** - * Invoked once after all existing buckets have been iterated over. - * The provided TrailingInserter instance may be used to append - * an arbitrary number of entries to the database. - * - * This is used to handle elements remaining at the end of a linear - * merge operation. - */ - virtual void insert_remaining_at_end(TrailingInserter&) {} - }; + using TrailingInserter = bucketdb::TrailingInserter<Entry>; + using Merger = bucketdb::Merger<Entry>; + using MergingProcessor = bucketdb::MergingProcessor<Entry>; /** * Iterate over the bucket database in bucket key order, allowing an arbitrary @@ -234,22 +141,10 @@ public: virtual uint32_t childCount(const document::BucketId&) const = 0; - struct ReadGuard { - ReadGuard() = default; - virtual ~ReadGuard() = default; - - ReadGuard(ReadGuard&&) = delete; - ReadGuard& operator=(ReadGuard&&) = delete; - ReadGuard(const ReadGuard&) = delete; - ReadGuard& operator=(const ReadGuard&) = delete; - - virtual void find_parents_and_self(const document::BucketId& bucket, - std::vector<Entry>& entries) const = 0; - virtual uint64_t generation() const noexcept = 0; - }; + using ReadGuard = bucketdb::ReadGuard<Entry>; virtual std::unique_ptr<ReadGuard> acquire_read_guard() const { - return std::unique_ptr<ReadGuard>(); + return std::unique_ptr<bucketdb::ReadGuard<Entry>>(); } [[nodiscard]] virtual vespalib::MemoryUsage memory_usage() const noexcept = 0; diff --git a/storage/src/vespa/storage/bucketdb/db_merger.h b/storage/src/vespa/storage/bucketdb/db_merger.h new file mode 100644 index 00000000000..4bda7ecff4d --- /dev/null +++ b/storage/src/vespa/storage/bucketdb/db_merger.h @@ -0,0 +1,111 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include <vespa/document/bucket/bucketid.h> + +namespace storage::bucketdb { + +/** + * Database implementation-specific interface for appending entries + * during a merge() operation. + */ +template <typename ValueT> +struct TrailingInserter { + virtual ~TrailingInserter() = default; + /** + * Insert a new database entry at the end of the current bucket space. + * + * Precondition: the bucket ID must sort after all entries that + * have already been iterated over or inserted via insert_at_end(). + */ + virtual void insert_at_end(const document::BucketId& bucket_id, const ValueT&) = 0; +}; + +/** + * Database implementation-specific interface for accessing bucket + * entries and prepending entries during a merge() operation. + */ +template <typename ValueT> +struct Merger { + virtual ~Merger() = default; + + // TODO this should ideally be separated into read/write functions, but this + // will suffice for now to avoid too many changes. + + /** + * Bucket key/ID of the currently iterated entry. Unless the information stored + * in the DB Entry is needed, using one of these methods should be preferred to + * getting the bucket ID via current_entry(). The underlying DB is expected to + * have cheap access to the ID but _may_ have expensive access to the entry itself. + */ + [[nodiscard]] virtual uint64_t bucket_key() const noexcept = 0; + [[nodiscard]] virtual document::BucketId bucket_id() const noexcept = 0; + /** + * Returns a mutable representation of the currently iterated database + * entry. If changes are made to this object, Result::Update must be + * returned from merge(). Otherwise, mutation visibility is undefined. + */ + [[nodiscard]] virtual ValueT& current_entry() = 0; + /** + * Insert a new entry into the bucket database that is ordered before the + * currently iterated entry. + * + * Preconditions: + * - The bucket ID must sort _before_ the currently iterated + * entry's bucket ID, in "reversed bits" bucket key order. + * - The bucket ID must sort _after_ any entries previously + * inserted with insert_before_current(). + * - The bucket ID must not be the same as a bucket that was + * already iterated over as part of the DB merge() call or inserted + * via a previous call to insert_before_current(). + * Such buckets must be handled by explicitly updating the provided + * entry for the iterated bucket and returning Result::Update. + */ + virtual void insert_before_current(const document::BucketId& bucket_id, const ValueT&) = 0; +}; + +/** + * Interface to be implemented by callers that wish to receive callbacks + * during a bucket merge() operation. + */ +template <typename ValueT> +struct MergingProcessor { + // See merge() for semantics on enum values. + enum class Result { + Update, + KeepUnchanged, + Skip + }; + + virtual ~MergingProcessor() = default; + /** + * Invoked for each existing bucket in the database, in bucket key order. + * The provided Merge instance may be used to access the current entry + * and prepend entries to the DB. + * + * Return value semantics: + * - Result::Update: + * when merge() returns, the changes made to the current entry will + * become visible in the bucket database. + * - Result::KeepUnchanged: + * when merge() returns, the entry will remain in the same state as + * it was when merge() was originally called. + * - Result::Skip: + * when merge() returns, the entry will no longer be part of the DB. + * Any entries added via insert_before_current() _will_ be present. + * + */ + virtual Result merge(Merger<ValueT>&) = 0; + /** + * Invoked once after all existing buckets have been iterated over. + * The provided TrailingInserter instance may be used to append + * an arbitrary number of entries to the database. + * + * This is used to handle elements remaining at the end of a linear + * merge operation. + */ + virtual void insert_remaining_at_end(TrailingInserter<ValueT>&) {} +}; + + +} diff --git a/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.cpp b/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.cpp index bcc471cc903..b4b8c6e54b9 100644 --- a/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.cpp +++ b/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.cpp @@ -5,6 +5,19 @@ namespace storage::bucketdb { using document::BucketId; +// TODO getMinDiffBits is hoisted from lockablemap.cpp, could probably be rewritten in terms of xor and MSB bit scan instr +/* + * 63 -------- ... -> 0 + * a: 1101111111 ... 0010 + * b: 1101110010 ... 0011 + * a ^ b: 0000001101 ... 0001 + * ^- diff bit = 57 + * + * 63 - vespalib::Optimized::msbIdx(a ^ b) ==> 6 + * + * what if a == b? special case? not a problem if we can prove this never happens. + */ + // TODO dedupe and unify common code uint8_t getMinDiffBits(uint16_t minBits, const BucketId& a, const BucketId& b) { diff --git a/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.h b/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.h index 15de7f3525b..8bc7a3379b3 100644 --- a/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.h +++ b/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.h @@ -1,6 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once +#include "db_merger.h" #include <vespa/document/bucket/bucketid.h> #include <vespa/vespalib/btree/btree.h> #include <vespa/vespalib/btree/minmaxaggregated.h> @@ -8,108 +9,6 @@ namespace storage::bucketdb { -/** - * Database implementation-specific interface for appending entries - * during a merge() operation. - */ -template <typename ValueT> -struct TrailingInserter { - virtual ~TrailingInserter() = default; - /** - * Insert a new database entry at the end of the current bucket space. - * - * Precondition: the bucket ID must sort after all entries that - * have already been iterated over or inserted via insert_at_end(). - */ - virtual void insert_at_end(const document::BucketId& bucket_id, const ValueT&) = 0; -}; - -/** - * Database implementation-specific interface for accessing bucket - * entries and prepending entries during a merge() operation. - */ -template <typename ValueT> -struct Merger { - virtual ~Merger() = default; - - // TODO this should ideally be separated into read/write functions, but this - // will suffice for now to avoid too many changes. - - /** - * Bucket key/ID of the currently iterated entry. Unless the information stored - * in the DB Entry is needed, using one of these methods should be preferred to - * getting the bucket ID via current_entry(). The underlying DB is expected to - * have cheap access to the ID but _may_ have expensive access to the entry itself. - */ - [[nodiscard]] virtual uint64_t bucket_key() const noexcept = 0; - [[nodiscard]] virtual document::BucketId bucket_id() const noexcept = 0; - /** - * Returns a mutable representation of the currently iterated database - * entry. If changes are made to this object, Result::Update must be - * returned from merge(). Otherwise, mutation visibility is undefined. - */ - [[nodiscard]] virtual ValueT& current_entry() = 0; - /** - * Insert a new entry into the bucket database that is ordered before the - * currently iterated entry. - * - * Preconditions: - * - The bucket ID must sort _before_ the currently iterated - * entry's bucket ID, in "reversed bits" bucket key order. - * - The bucket ID must sort _after_ any entries previously - * inserted with insert_before_current(). - * - The bucket ID must not be the same as a bucket that was - * already iterated over as part of the DB merge() call or inserted - * via a previous call to insert_before_current(). - * Such buckets must be handled by explicitly updating the provided - * entry for the iterated bucket and returning Result::Update. - */ - virtual void insert_before_current(const document::BucketId& bucket_id, const ValueT&) = 0; -}; - -/** - * Interface to be implemented by callers that wish to receive callbacks - * during a bucket merge() operation. - */ -template <typename ValueT> -struct MergingProcessor { - // See merge() for semantics on enum values. - enum class Result { - Update, - KeepUnchanged, - Skip - }; - - virtual ~MergingProcessor() = default; - /** - * Invoked for each existing bucket in the database, in bucket key order. - * The provided Merge instance may be used to access the current entry - * and prepend entries to the DB. - * - * Return value semantics: - * - Result::Update: - * when merge() returns, the changes made to the current entry will - * become visible in the bucket database. - * - Result::KeepUnchanged: - * when merge() returns, the entry will remain in the same state as - * it was when merge() was originally called. - * - Result::Skip: - * when merge() returns, the entry will no longer be part of the DB. - * Any entries added via insert_before_current() _will_ be present. - * - */ - virtual Result merge(Merger<ValueT>&) = 0; - /** - * Invoked once after all existing buckets have been iterated over. - * The provided TrailingInserter instance may be used to append - * an arbitrary number of entries to the database. - * - * This is used to handle elements remaining at the end of a linear - * merge operation. - */ - virtual void insert_remaining_at_end(TrailingInserter<ValueT>&) {} -}; - /* * Bucket database implementation built around lock-free single-writer/multiple-readers B+tree. * @@ -186,6 +85,7 @@ public: BTreeConstIterator find(uint64_t key) const noexcept; BTreeConstIterator lower_bound(uint64_t key) const noexcept; + BTreeConstIterator upper_bound(uint64_t key) const noexcept; BTreeConstIterator begin() const noexcept; void clear() noexcept; @@ -202,11 +102,11 @@ public: bool update(const document::BucketId& bucket, const ValueType& new_entry); bool update_by_raw_key(uint64_t bucket_key, const ValueType& new_entry); - template <typename Func> + template <typename IterValueExtractor, typename Func> void find_parents_and_self(const document::BucketId& bucket, Func func) const; - template <typename Func> + template <typename IterValueExtractor, typename Func> void find_parents_self_and_children(const document::BucketId& bucket, Func func) const; @@ -220,13 +120,31 @@ public: DataStoreType& store() noexcept { return _store; } void merge(MergingProcessor<ValueType>& proc); + + friend class ReadSnapshot; + // See ReadGuard class comments for semantics. + class ReadSnapshot { + const GenericBTreeBucketDatabase* _db; + vespalib::GenerationHandler::Guard _guard; + typename BTree::FrozenView _frozen_view; + public: + explicit ReadSnapshot(const GenericBTreeBucketDatabase& db); + ~ReadSnapshot(); + + ReadSnapshot(const ReadSnapshot&) = delete; + ReadSnapshot& operator=(const ReadSnapshot&) = delete; + + template <typename IterValueExtractor, typename Func> + void find_parents_and_self(const document::BucketId& bucket, Func func) const; + [[nodiscard]] uint64_t generation() const noexcept; + }; private: // Functor is called for each found element in key order, with raw u64 keys and values. - template <typename Func> + template <typename IterValueExtractor, typename Func> BTreeConstIterator find_parents_internal(const typename BTree::FrozenView& frozen_view, const document::BucketId& bucket, Func func) const; - template <typename Func> + template <typename IterValueExtractor, typename Func> void find_parents_and_self_internal(const typename BTree::FrozenView& frozen_view, const document::BucketId& bucket, Func func) const; diff --git a/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.hpp b/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.hpp index 4b1b507d95a..adaf402e4d1 100644 --- a/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.hpp +++ b/storage/src/vespa/storage/bucketdb/generic_btree_bucket_database.hpp @@ -2,6 +2,15 @@ #pragma once #include "generic_btree_bucket_database.h" +#include <vespa/vespalib/btree/btreebuilder.h> +#include <vespa/vespalib/btree/btreenodeallocator.hpp> +#include <vespa/vespalib/btree/btreenode.hpp> +#include <vespa/vespalib/btree/btreenodestore.hpp> +#include <vespa/vespalib/btree/btreeiterator.hpp> +#include <vespa/vespalib/btree/btreeroot.hpp> +#include <vespa/vespalib/btree/btreebuilder.hpp> +#include <vespa/vespalib/btree/btree.hpp> +#include <vespa/vespalib/btree/btreestore.hpp> namespace storage::bucketdb { @@ -80,6 +89,12 @@ GenericBTreeBucketDatabase<DataStoreTraitsT>::lower_bound(uint64_t key) const no template <typename DataStoreTraitsT> typename GenericBTreeBucketDatabase<DataStoreTraitsT>::BTreeConstIterator +GenericBTreeBucketDatabase<DataStoreTraitsT>::upper_bound(uint64_t key) const noexcept { + return _tree.upperBound(key); +} + +template <typename DataStoreTraitsT> +typename GenericBTreeBucketDatabase<DataStoreTraitsT>::BTreeConstIterator GenericBTreeBucketDatabase<DataStoreTraitsT>::find(uint64_t key) const noexcept { return _tree.find(key); } @@ -90,6 +105,20 @@ GenericBTreeBucketDatabase<DataStoreTraitsT>::begin() const noexcept { return _tree.begin(); } +struct ByValue { + template <typename DB, typename Iter> + static typename DB::ValueType apply(const DB& db, const Iter& iter) { + return db.entry_from_iterator(iter); + }; +}; + +struct ByConstRef { + template <typename DB, typename Iter> + static typename DB::ConstValueRef apply(const DB& db, const Iter& iter) { + return db.const_value_ref_from_valid_iterator(iter); + }; +}; + /* * Finding the complete set of parents of a given bucket is not obvious how to * do efficiently, as we only know that the parents are ordered before their @@ -159,7 +188,7 @@ GenericBTreeBucketDatabase<DataStoreTraitsT>::begin() const noexcept { * invocation of seek() on the iterator. */ template <typename DataStoreTraitsT> -template <typename Func> +template <typename IterValueExtractor, typename Func> typename GenericBTreeBucketDatabase<DataStoreTraitsT>::BTreeConstIterator GenericBTreeBucketDatabase<DataStoreTraitsT>::find_parents_internal( const typename BTree::FrozenView& frozen_view, @@ -186,7 +215,7 @@ GenericBTreeBucketDatabase<DataStoreTraitsT>::find_parents_internal( auto candidate = BucketId(BucketId::keyToBucketId(iter.getKey())); if (candidate.contains(bucket)) { assert(candidate.getUsedBits() >= bits); - func(iter.getKey(), const_value_ref_from_valid_iterator(iter)); + func(iter.getKey(), IterValueExtractor::apply(*this, iter)); } bits = next_parent_bit_seek_level(bits, candidate, bucket); const auto parent_key = BucketId(bits, bucket.getRawId()).toKey(); @@ -197,41 +226,41 @@ GenericBTreeBucketDatabase<DataStoreTraitsT>::find_parents_internal( } template <typename DataStoreTraitsT> -template <typename Func> +template <typename IterValueExtractor, typename Func> void GenericBTreeBucketDatabase<DataStoreTraitsT>::find_parents_and_self_internal( const typename BTree::FrozenView& frozen_view, const BucketId& bucket, Func func) const { - auto iter = find_parents_internal(frozen_view, bucket, func); + auto iter = find_parents_internal<IterValueExtractor>(frozen_view, bucket, func); if (iter.valid() && iter.getKey() == bucket.toKey()) { - func(iter.getKey(), entry_from_iterator(iter)); + func(iter.getKey(), IterValueExtractor::apply(*this, iter)); } } template <typename DataStoreTraitsT> -template <typename Func> +template <typename IterValueExtractor, typename Func> void GenericBTreeBucketDatabase<DataStoreTraitsT>::find_parents_and_self( const document::BucketId& bucket, Func func) const { auto view = _tree.getFrozenView(); - find_parents_and_self_internal(view, bucket, std::move(func)); + find_parents_and_self_internal<IterValueExtractor>(view, bucket, std::move(func)); } template <typename DataStoreTraitsT> -template <typename Func> +template <typename IterValueExtractor, typename Func> void GenericBTreeBucketDatabase<DataStoreTraitsT>::find_parents_self_and_children( const BucketId& bucket, Func func) const { auto view = _tree.getFrozenView(); - auto iter = find_parents_internal(view, bucket, func); + auto iter = find_parents_internal<IterValueExtractor>(view, bucket, func); // `iter` is already pointing at, or beyond, one of the bucket's subtrees. for (; iter.valid(); ++iter) { auto candidate = BucketId(BucketId::keyToBucketId(iter.getKey())); if (bucket.contains(candidate)) { - func(iter.getKey(), entry_from_iterator(iter)); + func(iter.getKey(), IterValueExtractor::apply(*this, iter)); } else { break; } @@ -490,5 +519,30 @@ void GenericBTreeBucketDatabase<DataStoreTraitsT>::merge(MergingProcessor<ValueT commit_tree_changes(); } +template <typename DataStoreTraitsT> +GenericBTreeBucketDatabase<DataStoreTraitsT>::ReadSnapshot::ReadSnapshot( + const GenericBTreeBucketDatabase<DataStoreTraitsT>& db) + : _db(&db), + _guard(_db->_generation_handler.takeGuard()), + _frozen_view(_db->_tree.getFrozenView()) +{ +} + +template <typename DataStoreTraitsT> +GenericBTreeBucketDatabase<DataStoreTraitsT>::ReadSnapshot::~ReadSnapshot() = default; + +template <typename DataStoreTraitsT> +template <typename IterValueExtractor, typename Func> +void GenericBTreeBucketDatabase<DataStoreTraitsT>::ReadSnapshot::find_parents_and_self( + const BucketId& bucket, + Func func) const +{ + _db->find_parents_and_self_internal<IterValueExtractor>(_frozen_view, bucket, std::move(func)); +} + +template <typename DataStoreTraitsT> +uint64_t GenericBTreeBucketDatabase<DataStoreTraitsT>::ReadSnapshot::generation() const noexcept { + return _guard.getGeneration(); +} } diff --git a/storage/src/vespa/storage/bucketdb/lockablemap.h b/storage/src/vespa/storage/bucketdb/lockablemap.h index 83ecac0a94f..66619a4f7e8 100644 --- a/storage/src/vespa/storage/bucketdb/lockablemap.h +++ b/storage/src/vespa/storage/bucketdb/lockablemap.h @@ -28,7 +28,7 @@ namespace storage { template <typename Map> -class LockableMap +class LockableMap final : public bucketdb::AbstractBucketMap<typename Map::mapped_type> { public: diff --git a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp index edb808da294..7556b80b29c 100644 --- a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp +++ b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp @@ -414,7 +414,8 @@ struct MapDbMerger final : BucketDatabase::Merger { BucketDatabase::Entry& current_entry() override { return _current_entry; } - void insert_before_current(const BucketDatabase::Entry& e) override { + void insert_before_current([[maybe_unused]] const document::BucketId& bucket_id, + const BucketDatabase::Entry& e) override { _to_insert.emplace_back(e); // TODO movable } }; @@ -423,7 +424,8 @@ struct MapDbTrailingInserter final : BucketDatabase::TrailingInserter { MapBucketDatabase& _db; explicit MapDbTrailingInserter(MapBucketDatabase& db) : _db(db) {} - void insert_at_end(const BucketDatabase::Entry& e) override { + void insert_at_end([[maybe_unused]] const document::BucketId& bucket_id, + const BucketDatabase::Entry& e) override { _db.update(e); } }; @@ -584,7 +586,7 @@ MapBucketDatabase::print(std::ostream& out, bool verbose, out << ')'; } -std::unique_ptr<BucketDatabase::ReadGuard> MapBucketDatabase::acquire_read_guard() const { +std::unique_ptr<bucketdb::ReadGuard<BucketDatabase::Entry>> MapBucketDatabase::acquire_read_guard() const { return std::make_unique<ReadGuardImpl>(*this); } diff --git a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h index e41b797a321..e8d5688068f 100644 --- a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h +++ b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h @@ -29,7 +29,7 @@ public: document::BucketId getAppropriateBucket(uint16_t minBits, const document::BucketId& bid) override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; - std::unique_ptr<ReadGuard> acquire_read_guard() const override; + std::unique_ptr<bucketdb::ReadGuard<Entry>> acquire_read_guard() const override; vespalib::MemoryUsage memory_usage() const noexcept override; private: struct E { @@ -66,7 +66,7 @@ private: uint32_t childCountImpl(int index, uint8_t bitCount, const document::BucketId& b) const; // NOT thread-safe for concurrent reads! - class ReadGuardImpl : public ReadGuard { + class ReadGuardImpl final : public bucketdb::ReadGuard<Entry> { const MapBucketDatabase* _db; public: explicit ReadGuardImpl(const MapBucketDatabase& db) : _db(&db) {} diff --git a/storage/src/vespa/storage/bucketdb/read_guard.h b/storage/src/vespa/storage/bucketdb/read_guard.h new file mode 100644 index 00000000000..cf37bafb0dd --- /dev/null +++ b/storage/src/vespa/storage/bucketdb/read_guard.h @@ -0,0 +1,46 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include <vespa/document/bucket/bucketid.h> +#include <vector> + +namespace storage::bucketdb { + +/* + * Read guard for accessing the bucket tree of an underlying bucket database + * in a thread-safe, read-only manner. + * + * Important: If the underlying database is _not_ backed by a B-tree, the + * read guard does _not_ provide a stable view of the bucket key set when + * iterating, as that is not possible without locking the entire DB. + * + * If the guard is created by a B-tree DB, the following properties hold: + * - The set of bucket keys that can be iterated over is stable for the lifetime + * of the read guard. + * - The bucket _values_ may change during the lifetime of the read guard, + * but the reader will always observe a fully consistent value as if it were + * written atomically. + * + * Do not hold read guards for longer than absolutely necessary, as they cause + * memory to be retained by the backing DB until released. + */ + +template <typename ValueT> +class ReadGuard { +public: + ReadGuard() = default; + virtual ~ReadGuard() = default; + + ReadGuard(ReadGuard&&) = delete; + ReadGuard& operator=(ReadGuard&&) = delete; + ReadGuard(const ReadGuard&) = delete; + ReadGuard& operator=(const ReadGuard&) = delete; + + virtual void find_parents_and_self(const document::BucketId& bucket, + std::vector<ValueT>& entries) const = 0; + // If the underlying guard represents a snapshot, returns its monotonically + // increasing generation. Otherwise returns 0. + [[nodiscard]] virtual uint64_t generation() const noexcept = 0; +}; + +} diff --git a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp index 35af71898e6..057b106f775 100644 --- a/storage/src/vespa/storage/distributor/bucketdbupdater.cpp +++ b/storage/src/vespa/storage/distributor/bucketdbupdater.cpp @@ -181,7 +181,7 @@ public: if (key_at_cursor >= key_to_insert) { break; } - m.insert_before_current(*_current); + m.insert_before_current(_current->getBucketId(), *_current); ++_current; } if ((_current != _last) && (key_at_cursor == key_to_insert)) { @@ -201,7 +201,7 @@ public: void insert_remaining_at_end(BucketDatabase::TrailingInserter& inserter) override { for (; _current != _last; ++_current) { - inserter.insert_at_end(*_current); + inserter.insert_at_end(_current->getBucketId(), *_current); } } }; diff --git a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp index 13f9c21eed0..6983e3594af 100644 --- a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp +++ b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp @@ -193,11 +193,12 @@ void PendingBucketSpaceDbTransition::insert_remaining_at_end(BucketDatabase::Tra void PendingBucketSpaceDbTransition::addToMerger(BucketDatabase::Merger& merger, const Range& range) { + const auto bucket_id = _entries[range.first].bucket_id(); LOG(spam, "Adding new bucket %s with %d copies", - _entries[range.first].bucket_id().toString().c_str(), + bucket_id.toString().c_str(), range.second - range.first); - BucketDatabase::Entry e(_entries[range.first].bucket_id(), BucketInfo()); + BucketDatabase::Entry e(bucket_id, BucketInfo()); insertInfo(e, range); if (e->getLastGarbageCollectionTime() == 0) { e->setLastGarbageCollectionTime( @@ -205,18 +206,19 @@ PendingBucketSpaceDbTransition::addToMerger(BucketDatabase::Merger& merger, cons .getSeconds().getTime()); } e.getBucketInfo().updateTrusted(); - merger.insert_before_current(e); + merger.insert_before_current(bucket_id, e); } void PendingBucketSpaceDbTransition::addToInserter(BucketDatabase::TrailingInserter& inserter, const Range& range) { // TODO dedupe + const auto bucket_id = _entries[range.first].bucket_id(); LOG(spam, "Adding new bucket %s with %d copies", - _entries[range.first].bucket_id().toString().c_str(), + bucket_id.toString().c_str(), range.second - range.first); - BucketDatabase::Entry e(_entries[range.first].bucket_id(), BucketInfo()); + BucketDatabase::Entry e(bucket_id, BucketInfo()); insertInfo(e, range); if (e->getLastGarbageCollectionTime() == 0) { e->setLastGarbageCollectionTime( @@ -224,7 +226,7 @@ PendingBucketSpaceDbTransition::addToInserter(BucketDatabase::TrailingInserter& .getSeconds().getTime()); } e.getBucketInfo().updateTrusted(); - inserter.insert_at_end(e); + inserter.insert_at_end(bucket_id, e); } void diff --git a/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java b/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java index 52708ea6d8e..c9acd625373 100644 --- a/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java +++ b/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java @@ -97,11 +97,8 @@ public class Distribution { parent.addSubGroup(group); } } - if (root == null) { - throw new IllegalStateException("Got config that did not " - + "specify even a root group. Need a root group at" - + "\nminimum:\n" + config.toString()); - } + if (root == null) + throw new IllegalStateException("Config does not specify a root group"); root.calculateDistributionHashValues(); Distribution.this.config.setRelease(new Config(root, config.redundancy(), config.distributor_auto_ownership_transfer_on_whole_group_down())); } catch (ParseException e) { diff --git a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/GenerateTestDescriptorMojo.java b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/GenerateTestDescriptorMojo.java index 8309b7a8124..259ae2602c4 100644 --- a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/GenerateTestDescriptorMojo.java +++ b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/GenerateTestDescriptorMojo.java @@ -33,6 +33,7 @@ public class GenerateTestDescriptorMojo extends AbstractMojo { TestDescriptor.CURRENT_VERSION, analyzer.systemTests(), analyzer.stagingTests(), + analyzer.stagingSetupTests(), analyzer.productionTests()); writeDescriptorFile(descriptor); } diff --git a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/TestAnnotationAnalyzer.java b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/TestAnnotationAnalyzer.java index c45ef21bc31..e8b29b2b0f7 100644 --- a/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/TestAnnotationAnalyzer.java +++ b/vespa-maven-plugin/src/main/java/ai/vespa/hosted/plugin/TestAnnotationAnalyzer.java @@ -3,6 +3,7 @@ package ai.vespa.hosted.plugin; import ai.vespa.hosted.cd.ProductionTest; +import ai.vespa.hosted.cd.StagingSetup; import ai.vespa.hosted.cd.StagingTest; import ai.vespa.hosted.cd.SystemTest; import org.objectweb.asm.AnnotationVisitor; @@ -28,10 +29,12 @@ class TestAnnotationAnalyzer { private final List<String> systemTests = new ArrayList<>(); private final List<String> stagingTests = new ArrayList<>(); + private final List<String> stagingSetupTests = new ArrayList<>(); private final List<String> productionTests = new ArrayList<>(); List<String> systemTests() { return systemTests; } List<String> stagingTests() { return stagingTests; } + List<String> stagingSetupTests() { return stagingSetupTests; } List<String> productionTests() { return productionTests; } void analyzeClass(Path classFile) { @@ -65,6 +68,8 @@ class TestAnnotationAnalyzer { productionTests.add(className); } else if (StagingTest.class.getName().equals(annotationClassName)) { stagingTests.add(className); + } else if (StagingSetup.class.getName().equals(annotationClassName)) { + stagingTests.add(className); } else if (SystemTest.class.getName().equals(annotationClassName)) { systemTests.add(className); } diff --git a/vespa-osgi-testrunner/pom.xml b/vespa-osgi-testrunner/pom.xml index 62ea578f14f..db0dba89b8a 100644 --- a/vespa-osgi-testrunner/pom.xml +++ b/vespa-osgi-testrunner/pom.xml @@ -22,7 +22,6 @@ <scope>provided</scope> </dependency> - <!-- Verify that we need all junit deps --> <dependency> <groupId>org.junit.jupiter</groupId> <artifactId>junit-jupiter-engine</artifactId> @@ -45,18 +44,7 @@ </exclusion> </exclusions> </dependency> - <dependency> - <groupId>org.junit.jupiter</groupId> - <artifactId>junit-jupiter</artifactId> - <version>5.6.2</version> - <exclusions> - <exclusion> - <groupId>org.junit.jupiter</groupId> - <artifactId>junit-jupiter-api</artifactId> - </exclusion> - </exclusions> - </dependency> - + <dependency> <groupId>com.yahoo.vespa</groupId> <artifactId>tenant-cd-api</artifactId> diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitHandler.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitHandler.java deleted file mode 100644 index cb7d5b8df6b..00000000000 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitHandler.java +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.testrunner; - -import ai.vespa.hosted.api.TestDescriptor; -import ai.vespa.hosted.cd.internal.TestRuntimeProvider; -import com.google.inject.Inject; -import com.yahoo.container.handler.metrics.JsonResponse; -import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.container.jdisc.LoggingRequestHandler; -import com.yahoo.container.logging.AccessLog; -import com.yahoo.restapi.ErrorResponse; -import com.yahoo.restapi.MessageResponse; -import org.osgi.framework.Bundle; - -import java.io.IOException; -import java.util.List; -import java.util.concurrent.Executor; -import java.util.function.Function; - -/** - * @author mortent - */ -public class JunitHandler extends LoggingRequestHandler { - - private final JunitRunner junitRunner; - private final TestRuntimeProvider testRuntimeProvider; - - @Inject - public JunitHandler(Executor executor, AccessLog accessLog, JunitRunner junitRunner, TestRuntimeProvider testRuntimeProvider) { - super(executor, accessLog); - this.junitRunner = junitRunner; - this.testRuntimeProvider = testRuntimeProvider; - } - - @Override - public HttpResponse handle(HttpRequest httpRequest) { - String mode = property("mode", "help", httpRequest, String::valueOf); - TestDescriptor.TestCategory category = property("category", TestDescriptor.TestCategory.systemtest, httpRequest, TestDescriptor.TestCategory::valueOf); - - try { - testRuntimeProvider.initialize(httpRequest.getData().readAllBytes()); - } catch (IOException e) { - return new ErrorResponse(500, "testruntime-initialization", "Exception reading test config"); - } - - if ("help".equalsIgnoreCase(mode)) { - return new MessageResponse("Accepted modes: \n help \n list \n execute"); - } - - if (!"list".equalsIgnoreCase(mode) && !"execute".equalsIgnoreCase(mode)) { - return new ErrorResponse(400, "client error", "Unknown mode \"" + mode + "\""); - } - - Bundle testBundle = junitRunner.findTestBundle("-tests"); - TestDescriptor testDescriptor = junitRunner.loadTestDescriptor(testBundle); - List<Class<?>> testClasses = junitRunner.loadClasses(testBundle, testDescriptor, category); - - String jsonResponse = junitRunner.executeTests(testClasses); - - return new JsonResponse(200, jsonResponse); - } - - private static <VAL> VAL property(String name, VAL defaultValue, HttpRequest request, Function<String, VAL> converter) { - final String propertyString = request.getProperty(name); - if (propertyString != null) { - return converter.apply(propertyString); - } - return defaultValue; - } -} diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java index 69134f86be0..3fc85365084 100644 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java @@ -2,15 +2,12 @@ package com.yahoo.vespa.testrunner; import ai.vespa.hosted.api.TestDescriptor; +import ai.vespa.hosted.cd.internal.TestRuntimeProvider; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; -import com.yahoo.exception.ExceptionUtils; import com.yahoo.io.IOUtils; import com.yahoo.jdisc.application.OsgiFramework; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Slime; -import com.yahoo.slime.SlimeUtils; -import com.yahoo.yolean.Exceptions; +import com.yahoo.vespa.testrunner.legacy.LegacyTestRunner; import org.junit.jupiter.engine.JupiterTestEngine; import org.junit.platform.engine.discovery.DiscoverySelectors; import org.junit.platform.launcher.Launcher; @@ -20,16 +17,19 @@ import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; import org.junit.platform.launcher.core.LauncherFactory; import org.junit.platform.launcher.listeners.LoggingListener; import org.junit.platform.launcher.listeners.SummaryGeneratingListener; -import org.junit.platform.launcher.listeners.TestExecutionSummary; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import java.io.IOException; import java.net.URL; import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -41,10 +41,12 @@ public class JunitRunner extends AbstractComponent { private static final Logger logger = Logger.getLogger(JunitRunner.class.getName()); private final BundleContext bundleContext; + private final TestRuntimeProvider testRuntimeProvider; + private Future<TestReport> execution; @Inject - public JunitRunner(OsgiFramework osgiFramework) { - // TODO mortent: Find a way to workaround this hack + public JunitRunner(OsgiFramework osgiFramework, TestRuntimeProvider testRuntimeProvider) { + this.testRuntimeProvider = testRuntimeProvider; var tmp = osgiFramework.bundleContext(); try { var field = tmp.getClass().getDeclaredField("wrapped"); @@ -55,27 +57,54 @@ public class JunitRunner extends AbstractComponent { } } - public Bundle findTestBundle(String bundleNameSuffix) { + public void executeTests(TestDescriptor.TestCategory category, byte[] testConfig) { + if (execution != null) { + throw new RuntimeException("Test execution already in progress"); + } + testRuntimeProvider.initialize(testConfig); + Optional<Bundle> testBundle = findTestBundle(); + if (testBundle.isEmpty()) { + throw new RuntimeException("No test bundle available"); + } + + Optional<TestDescriptor> testDescriptor = loadTestDescriptor(testBundle.get()); + if (testDescriptor.isEmpty()) { + throw new RuntimeException("Could not find test descriptor"); + } + List<Class<?>> testClasses = loadClasses(testBundle.get(), testDescriptor.get(), category); + + execution = CompletableFuture.supplyAsync(() -> launchJunit(testClasses)); + } + + public boolean isSupported() { + return findTestBundle().isPresent(); + } + + private Optional<Bundle> findTestBundle() { return Stream.of(bundleContext.getBundles()) - .filter(bundle -> bundle.getSymbolicName().endsWith(bundleNameSuffix)) - .findAny() - .orElseThrow(() -> new RuntimeException("No bundle on classpath with name ending on " + bundleNameSuffix)); + .filter(this::isTestBundle) + .findAny(); } - public TestDescriptor loadTestDescriptor(Bundle bundle) { + private boolean isTestBundle(Bundle bundle) { + var testBundleHeader = bundle.getHeaders().get("X-JDisc-Test-Bundle-Version"); + return testBundleHeader != null && !testBundleHeader.isBlank(); + } + + private Optional<TestDescriptor> loadTestDescriptor(Bundle bundle) { URL resource = bundle.getEntry(TestDescriptor.DEFAULT_FILENAME); TestDescriptor testDescriptor; try { var jsonDescriptor = IOUtils.readAll(resource.openStream(), Charset.defaultCharset()).trim(); testDescriptor = TestDescriptor.fromJsonString(jsonDescriptor); logger.info( "Test classes in bundle :" + testDescriptor.toString()); - return testDescriptor; + return Optional.of(testDescriptor); } catch (IOException e) { - throw new RuntimeException("Could not load " + TestDescriptor.DEFAULT_FILENAME + " [" + e.getMessage() + "]"); + return Optional.empty(); } } - public List<Class<?>> loadClasses(Bundle bundle, TestDescriptor testDescriptor, TestDescriptor.TestCategory testCategory) { + private List<Class<?>> loadClasses(Bundle bundle, TestDescriptor testDescriptor, TestDescriptor.TestCategory testCategory) { List<Class<?>> testClasses = testDescriptor.getConfiguredTests(testCategory).stream() .map(className -> loadClass(bundle, className)) .collect(Collectors.toList()); @@ -94,7 +123,7 @@ public class JunitRunner extends AbstractComponent { } } - public String executeTests(List<Class<?>> testClasses) { + private TestReport launchJunit(List<Class<?>> testClasses) { LauncherDiscoveryRequest discoveryRequest = LauncherDiscoveryRequestBuilder.request() .selectors( testClasses.stream().map(DiscoverySelectors::selectClass).collect(Collectors.toList()) @@ -116,36 +145,8 @@ public class JunitRunner extends AbstractComponent { // Execute request launcher.execute(discoveryRequest); - var report = summaryListener.getSummary(); - - return createJsonTestReport(report, logLines); - } - - private String createJsonTestReport(TestExecutionSummary report, List<String> logLines) { - var slime = new Slime(); - var root = slime.setObject(); - var summary = root.setObject("summary"); - summary.setLong("Total tests", report.getTestsFoundCount()); - summary.setLong("Test success", report.getTestsSucceededCount()); - summary.setLong("Test failed", report.getTestsFailedCount()); - summary.setLong("Test ignored", report.getTestsSkippedCount()); - summary.setLong("Test success", report.getTestsAbortedCount()); - summary.setLong("Test started", report.getTestsStartedCount()); - var failures = summary.setArray("failures"); - report.getFailures().forEach(failure -> serializeFailure(failure, failures.addObject())); - - var output = root.setArray("output"); - logLines.forEach(output::addString); - - return Exceptions.uncheck(() -> new String(SlimeUtils.toJsonBytes(slime), StandardCharsets.UTF_8)); - } - - private void serializeFailure(TestExecutionSummary.Failure failure, Cursor slime) { - var testIdentifier = failure.getTestIdentifier(); - slime.setString("testName", testIdentifier.getUniqueId()); - slime.setString("testError",failure.getException().getMessage()); - slime.setString("exception", ExceptionUtils.getStackTraceAsString(failure.getException())); + return new TestReport(report, logLines); } private void log(List<String> logs, String message, Throwable t) { @@ -162,4 +163,33 @@ public class JunitRunner extends AbstractComponent { public void deconstruct() { super.deconstruct(); } + + public LegacyTestRunner.Status getStatus() { + if (execution == null) return LegacyTestRunner.Status.NOT_STARTED; + if (!execution.isDone()) return LegacyTestRunner.Status.RUNNING; + try { + TestReport report = execution.get(); + if (report.isSuccess()) { + return LegacyTestRunner.Status.SUCCESS; + } else { + return LegacyTestRunner.Status.FAILURE; + } + } catch (InterruptedException|ExecutionException e) { + logger.log(Level.WARNING, "Error while getting test report", e); + return LegacyTestRunner.Status.ERROR; + } + } + + public String getReportAsJson() { + if (execution.isDone()) { + try { + return execution.get().toJson(); + } catch (Exception e) { + logger.log(Level.WARNING, "Error getting test report", e); + return ""; + } + } else { + return ""; + } + } } diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java new file mode 100644 index 00000000000..2e45ba96486 --- /dev/null +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java @@ -0,0 +1,55 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.testrunner; + +import com.yahoo.exception.ExceptionUtils; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Slime; +import com.yahoo.slime.SlimeUtils; +import com.yahoo.yolean.Exceptions; +import org.junit.platform.launcher.listeners.TestExecutionSummary; + +import java.nio.charset.StandardCharsets; +import java.util.List; + +/** + * @author mortent + */ +public class TestReport { + private final TestExecutionSummary junitReport; + private final List<String> logLines; + + public TestReport(TestExecutionSummary junitReport, List<String> logLines) { + this.junitReport = junitReport; + this.logLines = List.copyOf(logLines); + } + + private void serializeFailure(TestExecutionSummary.Failure failure, Cursor slime) { + var testIdentifier = failure.getTestIdentifier(); + slime.setString("testName", testIdentifier.getUniqueId()); + slime.setString("testError",failure.getException().getMessage()); + slime.setString("exception", ExceptionUtils.getStackTraceAsString(failure.getException())); + } + + public String toJson() { + var slime = new Slime(); + var root = slime.setObject(); + var summary = root.setObject("summary"); + summary.setLong("Total tests", junitReport.getTestsFoundCount()); + summary.setLong("Test success", junitReport.getTestsSucceededCount()); + summary.setLong("Test failed", junitReport.getTestsFailedCount()); + summary.setLong("Test ignored", junitReport.getTestsSkippedCount()); + summary.setLong("Test aborted", junitReport.getTestsAbortedCount()); + summary.setLong("Test started", junitReport.getTestsStartedCount()); + var failures = summary.setArray("failures"); + junitReport.getFailures().forEach(failure -> serializeFailure(failure, failures.addObject())); + + var output = root.setArray("output"); + logLines.forEach(output::addString); + + return Exceptions.uncheck(() -> new String(SlimeUtils.toJsonBytes(slime), StandardCharsets.UTF_8)); + } + + public boolean isSuccess() { + return (junitReport.getTestsFailedCount() + junitReport.getTestsAbortedCount()) == 0; + } +} diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java new file mode 100644 index 00000000000..cb337a0c176 --- /dev/null +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java @@ -0,0 +1,211 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.testrunner; + +import ai.vespa.hosted.api.TestDescriptor; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.inject.Inject; +import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.container.jdisc.LoggingRequestHandler; +import com.yahoo.container.logging.AccessLog; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.testrunner.legacy.LegacyTestRunner; +import com.yahoo.vespa.testrunner.legacy.TestProfile; +import com.yahoo.yolean.Exceptions; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.io.PrintStream; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.Executor; +import java.util.logging.Level; +import java.util.logging.LogRecord; + +import static com.yahoo.jdisc.Response.Status; + +/** + * @author valerijf + * @author jvenstad + * @author mortent + */ +public class TestRunnerHandler extends LoggingRequestHandler { + + private static final String CONTENT_TYPE_APPLICATION_JSON = "application/json"; + + private final JunitRunner junitRunner; + private final LegacyTestRunner testRunner; + private final boolean useOsgiMode; + + @Inject + public TestRunnerHandler(Executor executor, AccessLog accessLog, JunitRunner junitRunner, LegacyTestRunner testRunner) { + super(executor, accessLog); + this.junitRunner = junitRunner; + this.testRunner = testRunner; + this.useOsgiMode = junitRunner.isSupported(); + } + + @Override + public HttpResponse handle(HttpRequest request) { + try { + switch (request.getMethod()) { + case GET: return handleGET(request); + case POST: return handlePOST(request); + + default: return new Response(Status.METHOD_NOT_ALLOWED, "Method '" + request.getMethod() + "' is not supported"); + } + } catch (IllegalArgumentException e) { + return new Response(Status.BAD_REQUEST, Exceptions.toMessageString(e)); + } catch (Exception e) { + log.log(Level.WARNING, "Unexpected error handling '" + request.getUri() + "'", e); + return new Response(Status.INTERNAL_SERVER_ERROR, Exceptions.toMessageString(e)); + } + } + + private HttpResponse handleGET(HttpRequest request) { + String path = request.getUri().getPath(); + if (path.equals("/tester/v1/log")) { + if (useOsgiMode) { + // TODO (mortent): Handle case where log is returned multiple times + String report = junitRunner.getReportAsJson(); + List<LogRecord> logRecords = new ArrayList<>(); + if (!report.isBlank()) { + logRecords.add(new LogRecord(Level.INFO, report)); + } + return new SlimeJsonResponse(logToSlime(logRecords)); + } else { + return new SlimeJsonResponse(logToSlime(testRunner.getLog(request.hasProperty("after") + ? Long.parseLong(request.getProperty("after")) + : -1))); + } + } else if (path.equals("/tester/v1/status")) { + if (useOsgiMode) { + log.info("Responding with status " + junitRunner.getStatus()); + return new Response(junitRunner.getStatus().name()); + } else { + log.info("Responding with status " + testRunner.getStatus()); + return new Response(testRunner.getStatus().name()); + } + } + return new Response(Status.NOT_FOUND, "Not found: " + request.getUri().getPath()); + } + + private HttpResponse handlePOST(HttpRequest request) throws IOException { + final String path = request.getUri().getPath(); + if (path.startsWith("/tester/v1/run/")) { + String type = lastElement(path); + TestProfile testProfile = TestProfile.valueOf(type.toUpperCase() + "_TEST"); + byte[] config = request.getData().readAllBytes(); + if (useOsgiMode) { + junitRunner.executeTests(categoryFromProfile(testProfile), config); + log.info("Started tests of type " + type + " and status is " + junitRunner.getStatus()); + return new Response("Successfully started " + type + " tests"); + } else { + testRunner.test(testProfile, config); + log.info("Started tests of type " + type + " and status is " + testRunner.getStatus()); + return new Response("Successfully started " + type + " tests"); + } + } + return new Response(Status.NOT_FOUND, "Not found: " + request.getUri().getPath()); + } + + TestDescriptor.TestCategory categoryFromProfile(TestProfile testProfile) { + switch(testProfile) { + case SYSTEM_TEST: return TestDescriptor.TestCategory.systemtest; + case STAGING_SETUP_TEST: return TestDescriptor.TestCategory.stagingsetuptest; + case STAGING_TEST: return TestDescriptor.TestCategory.stagingtest; + case PRODUCTION_TEST: return TestDescriptor.TestCategory.productiontest; + default: throw new RuntimeException("Unknown test profile: " + testProfile.name()); + } + } + + private static String lastElement(String path) { + if (path.endsWith("/")) + path = path.substring(0, path.length() - 1); + int lastSlash = path.lastIndexOf("/"); + if (lastSlash < 0) return path; + return path.substring(lastSlash + 1); + } + + static Slime logToSlime(Collection<LogRecord> log) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + Cursor recordArray = root.setArray("logRecords"); + logArrayToSlime(recordArray, log); + return slime; + } + + static void logArrayToSlime(Cursor recordArray, Collection<LogRecord> log) { + log.forEach(record -> { + Cursor recordObject = recordArray.addObject(); + recordObject.setLong("id", record.getSequenceNumber()); + recordObject.setLong("at", record.getMillis()); + recordObject.setString("type", typeOf(record.getLevel())); + String message = record.getMessage(); + if (record.getThrown() != null) { + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + record.getThrown().printStackTrace(new PrintStream(buffer)); + message += "\n" + buffer; + } + recordObject.setString("message", message); + }); + } + + public static String typeOf(Level level) { + return level.getName().equals("html") ? "html" + : level.intValue() < Level.INFO.intValue() ? "debug" + : level.intValue() < Level.WARNING.intValue() ? "info" + : level.intValue() < Level.SEVERE.intValue() ? "warning" + : "error"; + } + + private static class SlimeJsonResponse extends HttpResponse { + private final Slime slime; + + private SlimeJsonResponse(Slime slime) { + super(200); + this.slime = slime; + } + + @Override + public void render(OutputStream outputStream) throws IOException { + new JsonFormat(true).encode(outputStream, slime); + } + + @Override + public String getContentType() { + return CONTENT_TYPE_APPLICATION_JSON; + } + } + + private static class Response extends HttpResponse { + private static final ObjectMapper objectMapper = new ObjectMapper(); + private final String message; + + private Response(String response) { + this(200, response); + } + + private Response(int statusCode, String message) { + super(statusCode); + this.message = message; + } + + @Override + public void render(OutputStream outputStream) throws IOException { + ObjectNode objectNode = objectMapper.createObjectNode(); + objectNode.put("message", message); + objectMapper.writeValue(outputStream, objectNode); + } + + @Override + public String getContentType() { + return CONTENT_TYPE_APPLICATION_JSON; + } + } +} diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/LegacyTestRunner.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/LegacyTestRunner.java new file mode 100644 index 00000000000..9f1a68218f0 --- /dev/null +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/LegacyTestRunner.java @@ -0,0 +1,22 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.testrunner.legacy; + +import java.util.Collection; +import java.util.logging.LogRecord; + +/** + * @author mortent + */ +public interface LegacyTestRunner { + + Collection<LogRecord> getLog(long after); + + Status getStatus(); + + void test(TestProfile testProfile, byte[] config); + + // TODO (mortent) : This seems to be duplicated in TesterCloud.Status and expects to have the same values + enum Status { + NOT_STARTED, RUNNING, FAILURE, ERROR, SUCCESS + } +} diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/TestProfile.java index d568b549f9b..59576209043 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/TestProfile.java @@ -1,11 +1,11 @@ -// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.testrunner; +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.testrunner.legacy; /** * @author valerijf * @author jvenstad */ -enum TestProfile { +public enum TestProfile { SYSTEM_TEST("system, com.yahoo.vespa.tenant.systemtest.base.SystemTest", true), STAGING_SETUP_TEST("staging-setup", false), @@ -20,12 +20,11 @@ enum TestProfile { this.failIfNoTests = failIfNoTests; } - String group() { + public String group() { return group; } - boolean failIfNoTests() { + public boolean failIfNoTests() { return failIfNoTests; } - } diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/package-info.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/package-info.java new file mode 100644 index 00000000000..49f6cef0c22 --- /dev/null +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/package-info.java @@ -0,0 +1,9 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +/** + * @author mortent + */ +@ExportPackage +package com.yahoo.vespa.testrunner.legacy; + +import com.yahoo.osgi.annotation.ExportPackage;
\ No newline at end of file diff --git a/vespa-testrunner-components/pom.xml b/vespa-testrunner-components/pom.xml index 31568d01fb5..e780da726a1 100644 --- a/vespa-testrunner-components/pom.xml +++ b/vespa-testrunner-components/pom.xml @@ -24,6 +24,24 @@ </dependency> <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>vespa-osgi-testrunner</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + <!-- junit must be excluded to keep maven-surefire-plugin to be confused --> + <exclusions> + <exclusion> + <groupId>org.junit.jupiter</groupId> + <artifactId>junit-jupiter-engine</artifactId> + </exclusion> + <exclusion> + <groupId>org.junit.platform</groupId> + <artifactId>junit-platform-launcher</artifactId> + </exclusion> + </exclusions> + </dependency> + + <dependency> <groupId>org.fusesource.jansi</groupId> <artifactId>jansi</artifactId> <version>1.11</version> diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/PomXmlGenerator.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/PomXmlGenerator.java index e6f402ba563..dd424de5471 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/PomXmlGenerator.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/PomXmlGenerator.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.testrunner; import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.vespa.testrunner.legacy.TestProfile; import java.nio.file.Path; import java.util.List; diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java index cdf320a6304..4308b0bba4c 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java @@ -3,6 +3,8 @@ package com.yahoo.vespa.hosted.testrunner; import com.google.inject.Inject; import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.vespa.testrunner.legacy.LegacyTestRunner; +import com.yahoo.vespa.testrunner.legacy.TestProfile; import org.fusesource.jansi.AnsiOutputStream; import org.fusesource.jansi.HtmlAnsiOutputStream; @@ -30,14 +32,13 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.logging.Level.INFO; import static java.util.logging.Level.SEVERE; /** * @author valerijf * @author jvenstad */ -public class TestRunner { +public class TestRunner implements LegacyTestRunner { private static final Logger logger = Logger.getLogger(TestRunner.class.getName()); private static final Level HTML = new Level("html", 1) { }; @@ -203,9 +204,4 @@ public class TestRunner { } } - - public enum Status { - NOT_STARTED, RUNNING, FAILURE, ERROR, SUCCESS - } - } diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java index e92dbcede5a..8f9966a898f 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java @@ -9,10 +9,10 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.container.logging.AccessLog; import com.yahoo.io.IOUtils; -import java.util.logging.Level; import com.yahoo.slime.Cursor; import com.yahoo.slime.JsonFormat; import com.yahoo.slime.Slime; +import com.yahoo.vespa.testrunner.legacy.TestProfile; import com.yahoo.yolean.Exceptions; import java.io.ByteArrayOutputStream; diff --git a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/PomXmlGeneratorTest.java b/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/PomXmlGeneratorTest.java index c7799bff116..823dca4a7a2 100644 --- a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/PomXmlGeneratorTest.java +++ b/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/PomXmlGeneratorTest.java @@ -1,6 +1,7 @@ // Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.testrunner; +import com.yahoo.vespa.testrunner.legacy.TestProfile; import org.junit.Test; import java.io.IOException; diff --git a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerTest.java b/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerTest.java index 22fd7fddf31..b2c7a77240b 100644 --- a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerTest.java +++ b/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerTest.java @@ -1,6 +1,7 @@ // Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.testrunner; +import com.yahoo.vespa.testrunner.legacy.TestProfile; import org.fusesource.jansi.Ansi; import org.junit.Before; import org.junit.Rule; @@ -16,7 +17,6 @@ import java.util.logging.LogRecord; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; /** * Unit tests relying on a UNIX shell >_< |