diff options
36 files changed, 212 insertions, 161 deletions
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000000..40e52f11c60 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,41 @@ +--- +name: Bug report +about: Create a report to help us improve +title: '' +labels: '' +assignees: '' + +--- + +**Describe the bug** +A clear and concise description of what the bug is. + +**To Reproduce** +Steps to reproduce the behavior: +1. Go to '...' +2. Click on '....' +3. Scroll down to '....' +4. See error + +**Expected behavior** +A clear and concise description of what you expected to happen. + +**Screenshots** +If applicable, add screenshots to help explain your problem. + +**Desktop (please complete the following information):** + - OS: [e.g. iOS] + - Browser [e.g. chrome, safari] + - Version [e.g. 22] + +**Smartphone (please complete the following information):** + - Device: [e.g. iPhone6] + - OS: [e.g. iOS8.1] + - Browser [e.g. stock browser, safari] + - Version [e.g. 22] + +**Vespa version** +Include Vespa version(s) used. + +**Additional context** +Add any other context about the problem here. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 00000000000..bbcbbe7d615 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,20 @@ +--- +name: Feature request +about: Suggest an idea for this project +title: '' +labels: '' +assignees: '' + +--- + +**Is your feature request related to a problem? Please describe.** +A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] + +**Describe the solution you'd like** +A clear and concise description of what you want to happen. + +**Describe alternatives you've considered** +A clear and concise description of any alternative solutions or features you've considered. + +**Additional context** +Add any other context or screenshots about the feature request here. diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java index e180016f286..b4e9a760d8e 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java @@ -25,7 +25,7 @@ import java.util.logging.Logger; public class SlobrokClient implements NodeLookup { - public static Logger log = Logger.getLogger(SlobrokClient.class.getName()); + public static final Logger log = Logger.getLogger(SlobrokClient.class.getName()); private final Timer timer; private String[] connectionSpecs; 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 abca424a838..42ef763fc62 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 @@ -74,20 +74,16 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}, comment = "Select sequencer type use while feeding") default String feedSequencerType() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default String responseSequencerType() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default int defaultNumResponseThreads() { return 2; } - @ModelFeatureFlag(owners = {"baldersheim"}) default int maxPendingMoveOps() { return 100; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean skipCommunicationManagerThread() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean skipMbusRequestThread() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean skipMbusReplyThread() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"tokle"}) default boolean useAccessControlTlsHandshakeClientAuth() { return true; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useAsyncMessageHandlingOnSchedule() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default double feedConcurrency() { throw new UnsupportedOperationException("TODO specify default value"); } - @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useBucketExecutorForLidSpaceCompact() { return true; } - @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useBucketExecutorForBucketMove() { return true; } - @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useBucketExecutorForPruneRemoved() { throw new UnsupportedOperationException("TODO specify default value"); } + @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useBucketExecutorForPruneRemoved() { return true; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useExternalRankExpressions() { return false; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean distributeExternalRankExpressions() { return false; } @ModelFeatureFlag(owners = {"geirst"}) default boolean enableFeedBlockInDistributor() { return true; } - @ModelFeatureFlag(owners = {"baldersheim", "geirst", "toregge"}) default double maxDeadBytesRatio() { return 0.05; } @ModelFeatureFlag(owners = {"hmusum"}, removeAfter = "7.406") default int clusterControllerMaxHeapSizeInMb() { return 128; } @ModelFeatureFlag(owners = {"hmusum"}) default int metricsProxyMaxHeapSizeInMb(ClusterSpec.Type type) { return 256; } @ModelFeatureFlag(owners = {"bjorncs", "tokle"}) default List<String> allowedAthenzProxyIdentities() { return List.of(); } diff --git a/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java b/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java index 061ad42e028..364dd1742ae 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java +++ b/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java @@ -65,8 +65,8 @@ public class ApplicationConfigProducerRoot extends AbstractConfigProducer<Abstra * Creates and initializes a new Vespa from the service config file * in the given application directory. * - * @param parent The parent, usually VespaModel - * @param name The name, used as configId + * @param parent the parent, usually VespaModel + * @param name the name, used as configId * @param documentModel DocumentModel to serve global document config from. */ public ApplicationConfigProducerRoot(AbstractConfigProducer parent, String name, DocumentModel documentModel, Version vespaVersion, ApplicationId applicationId) { diff --git a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java index 557df4bd106..100b5ec5a24 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java +++ b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java @@ -11,6 +11,16 @@ import com.yahoo.config.model.builder.xml.XmlHelper; import com.yahoo.config.model.graph.ModelGraphBuilder; import com.yahoo.config.model.graph.ModelNode; import com.yahoo.config.model.provision.HostsXmlProvisioner; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.TreeMap; import java.util.logging.Level; import com.yahoo.path.Path; import com.yahoo.text.XML; @@ -29,7 +39,6 @@ import java.io.IOException; import java.io.Reader; import java.io.Serializable; import java.io.StringReader; -import java.util.*; import java.util.logging.Logger; /** 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 5dd6ffe7247..304855e545d 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 @@ -51,7 +51,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private boolean useAccessControlTlsHandshakeClientAuth; private boolean useAsyncMessageHandlingOnSchedule = false; private double feedConcurrency = 0.5; - private boolean useBucketExecutorForPruneRemoved; private boolean enableFeedBlockInDistributor = true; private boolean useExternalRankExpression = false; private int clusterControllerMaxHeapSizeInMb = 128; @@ -92,7 +91,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public boolean useAccessControlTlsHandshakeClientAuth() { return useAccessControlTlsHandshakeClientAuth; } @Override public boolean useAsyncMessageHandlingOnSchedule() { return useAsyncMessageHandlingOnSchedule; } @Override public double feedConcurrency() { return feedConcurrency; } - @Override public boolean useBucketExecutorForPruneRemoved() { return useBucketExecutorForPruneRemoved; } @Override public boolean enableFeedBlockInDistributor() { return enableFeedBlockInDistributor; } @Override public int clusterControllerMaxHeapSizeInMb() { return clusterControllerMaxHeapSizeInMb; } @Override public int metricsProxyMaxHeapSizeInMb(ClusterSpec.Type type) { return metricsProxyMaxHeapSizeInMb; } @@ -201,11 +199,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea return this; } - public TestProperties useBucketExecutorForPruneRemoved(boolean enabled) { - useBucketExecutorForPruneRemoved = enabled; - return this; - } - public TestProperties enableFeedBlockInDistributor(boolean enabled) { enableFeedBlockInDistributor = enabled; return this; diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java index af40dc947c8..7e5b52f6af1 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java @@ -807,7 +807,6 @@ public class RankProfile implements Cloneable { // Function compiling second pass: compile all functions and insert previously compiled inline functions // TODO This merges all functions from inherited profiles too and erases inheritance information. Not good. functions = compileFunctions(this::getFunctions, queryProfiles, featureTypes, importedModels, inlineFunctions, expressionTransforms); - } private void checkNameCollisions(Map<String, RankingExpressionFunction> functions, Map<String, Value> constants) { @@ -843,8 +842,8 @@ public class RankProfile implements Cloneable { return compiledFunctions; } - private Map.Entry<String, RankingExpressionFunction> findUncompiledFunction(Map<String, RankingExpressionFunction> functions, - Set<String> compiledFunctionNames) { + private static Map.Entry<String, RankingExpressionFunction> findUncompiledFunction(Map<String, RankingExpressionFunction> functions, + Set<String> compiledFunctionNames) { for (Map.Entry<String, RankingExpressionFunction> entry : functions.entrySet()) { if ( ! compiledFunctionNames.contains(entry.getKey())) return entry; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java b/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java index 2bd9cb09aa6..ea52f9689ff 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java @@ -177,7 +177,7 @@ public class Content extends ConfigModel { s.setVespaMallocDebugStackTrace(cluster.getRootGroup().getVespaMallocDebugStackTrace().get()); } } - cluster.prepare(deployState); + cluster.prepare(); } private void setCpuSocketAffinity() { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java index 18580249ddc..51949e78838 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java @@ -65,7 +65,6 @@ public class ContentSearchCluster extends AbstractConfigProducer<SearchCluster> private Optional<ResourceLimits> resourceLimits = Optional.empty(); private final ProtonConfig.Indexing.Optimize.Enum feedSequencerType; private final double defaultFeedConcurrency; - private final boolean useBucketExecutorForPruneRemoved; /** Whether the nodes of this cluster also hosts a container cluster in a hosted system */ private final boolean combined; @@ -210,7 +209,6 @@ public class ContentSearchCluster extends AbstractConfigProducer<SearchCluster> this.combined = combined; feedSequencerType = convertFeedSequencerType(featureFlags.feedSequencerType()); defaultFeedConcurrency = featureFlags.feedConcurrency(); - useBucketExecutorForPruneRemoved = featureFlags.useBucketExecutorForPruneRemoved(); } public void setVisibilityDelay(double delay) { @@ -427,7 +425,6 @@ public class ContentSearchCluster extends AbstractConfigProducer<SearchCluster> } else { builder.indexing.optimize(feedSequencerType); } - builder.pruneremoveddocuments.usebucketexecutor(useBucketExecutorForPruneRemoved); } private boolean isGloballyDistributed(NewDocumentType docType) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java index b5cae857ce0..e0d311e6df6 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java @@ -2,12 +2,10 @@ package com.yahoo.vespa.model.content.cluster; import com.google.common.base.Preconditions; -import com.google.common.collect.Sets; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; -import com.yahoo.config.model.producer.AbstractConfigProducerRoot; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; @@ -47,7 +45,6 @@ import com.yahoo.vespa.model.content.IndexedHierarchicDistributionValidator; import com.yahoo.vespa.model.content.Redundancy; import com.yahoo.vespa.model.content.ReservedDocumentTypeNameValidator; import com.yahoo.vespa.model.content.StorageGroup; -import com.yahoo.vespa.model.content.StorageNode; import com.yahoo.vespa.model.content.engines.PersistenceEngine; import com.yahoo.vespa.model.content.engines.ProtonEngine; import com.yahoo.vespa.model.content.storagecluster.StorageCluster; @@ -64,7 +61,6 @@ import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.logging.Level; -import java.util.stream.Collectors; import static java.util.stream.Collectors.toList; @@ -74,7 +70,7 @@ import static java.util.stream.Collectors.toList; * @author mostly somebody unknown * @author bratseth */ -public class ContentCluster extends AbstractConfigProducer implements +public class ContentCluster extends AbstractConfigProducer<AbstractConfigProducer<?>> implements DistributionConfig.Producer, StorDistributionConfig.Producer, StorDistributormanagerConfig.Producer, @@ -98,14 +94,6 @@ public class ContentCluster extends AbstractConfigProducer implements private Integer maxNodesPerMerge; private final Zone zone; - /** - * If multitenant or a cluster controller was explicitly configured in this cluster: - * The cluster controller cluster of this particular content cluster. - * - * Otherwise: null - the cluster controller is shared by all content clusters and part of Admin. - */ - private ClusterControllerContainerCluster clusterControllers; - public enum DistributionMode { LEGACY, STRICT, LOOSE } private DistributionMode distributionMode; @@ -419,16 +407,10 @@ public class ContentCluster extends AbstractConfigProducer implements public ClusterSpec.Id id() { return ClusterSpec.Id.from(clusterId); } - public void prepare(DeployState deployState) { + public void prepare() { search.prepare(); - - if (clusterControllers != null) - clusterControllers.prepare(deployState); } - /** Returns cluster controllers if this is multitenant, null otherwise */ - public ClusterControllerContainerCluster getClusterControllers() { return clusterControllers; } - public DistributionMode getDistributionMode() { if (distributionMode != null) return distributionMode; return getPersistence().getDefaultDistributionMode(); diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index 2b72420614d..b1b386924d1 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -38,7 +38,6 @@ import com.yahoo.yolean.Exceptions; import org.junit.Test; import java.io.StringReader; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -502,7 +501,6 @@ public class ModelProvisioningTest { // Check content clusters ContentCluster cluster = model.getContentClusters().get("bar"); - assertNull("No own cluster controllers when hosted", cluster.getClusterControllers()); assertEquals(0, cluster.getRootGroup().getNodes().size()); assertEquals(9, cluster.getRootGroup().getSubgroups().size()); assertEquals("0", cluster.getRootGroup().getSubgroups().get(0).getIndex()); @@ -536,7 +534,6 @@ public class ModelProvisioningTest { assertThat(cluster.getRootGroup().getSubgroups().get(8).getNodes().get(2).getConfigId(), is("bar/storage/26")); cluster = model.getContentClusters().get("baz"); - assertNull("No own cluster controllers when hosted", cluster.getClusterControllers()); assertEquals(0, cluster.getRootGroup().getNodes().size()); assertEquals(27, cluster.getRootGroup().getSubgroups().size()); assertThat(cluster.getRootGroup().getSubgroups().get(0).getIndex(), is("0")); @@ -730,7 +727,6 @@ public class ModelProvisioningTest { // Check content cluster ContentCluster cluster = model.getContentClusters().get("bar"); - assertNull(cluster.getClusterControllers()); assertEquals(0, cluster.getRootGroup().getNodes().size()); assertEquals(8, cluster.getRootGroup().getSubgroups().size()); assertEquals(8, cluster.distributionBits()); @@ -865,8 +861,6 @@ public class ModelProvisioningTest { assertEquals(7, model.getRoot().hostSystem().getHosts().size()); // Check cluster controllers - assertNull(model.getContentClusters().get("foo").getClusterControllers()); - assertNull(model.getContentClusters().get("bar").getClusterControllers()); ClusterControllerContainerCluster clusterControllers = model.getAdmin().getClusterControllers(); assertEquals(3, clusterControllers.getContainers().size()); assertEquals("cluster-controllers", clusterControllers.getName()); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java index ad4603e5c6b..7bff4890b7e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java @@ -824,24 +824,6 @@ public class ContentBuilderTest extends DomBuilderTest { verifyThatFeatureFlagControlsVisibilityDelayDefault(0.6, 0.6); } - private void verifyThatFeatureFlagControlsUseBucketExecutorForPruneRemoved(boolean flag) { - DeployState.Builder deployStateBuilder = new DeployState.Builder().properties(new TestProperties().useBucketExecutorForPruneRemoved(flag)); - VespaModel model = new VespaModelCreatorWithMockPkg(new MockApplicationPackage.Builder() - .withServices(singleNodeContentXml()) - .withSearchDefinition(MockApplicationPackage.MUSIC_SEARCHDEFINITION) - .build()) - .create(deployStateBuilder); - ProtonConfig config = getProtonConfig(model.getContentClusters().values().iterator().next()); - assertEquals(flag, config.pruneremoveddocuments().usebucketexecutor()); - } - - - @Test - public void verifyUseBucketExecutorForPruneRemoved() { - verifyThatFeatureFlagControlsUseBucketExecutorForPruneRemoved(true); - verifyThatFeatureFlagControlsUseBucketExecutorForPruneRemoved(false); - } - @Test public void failWhenNoDocumentsElementSpecified() { expectedException.expect(IllegalArgumentException.class); 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 953c42243a6..13d02fc1fb8 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 @@ -26,7 +26,6 @@ import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.admin.clustercontroller.ClusterControllerContainer; import com.yahoo.vespa.model.admin.clustercontroller.ClusterControllerContainerCluster; import com.yahoo.vespa.model.container.ContainerCluster; -import com.yahoo.vespa.model.container.component.Component; import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.content.engines.ProtonEngine; import com.yahoo.vespa.model.content.utils.ContentClusterBuilder; @@ -46,7 +45,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import static java.util.stream.Collectors.toList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -533,6 +531,7 @@ public class ContentClusterTest extends ContentBaseTest { ContentCluster stagingNot16Bits = createWithZone(xml, new Zone(Environment.staging, RegionName.from("us-east-3"))); assertDistributionBitsInConfig(stagingNot16Bits, 8); } + @Test public void testGenerateSearchNodes() { @@ -1076,7 +1075,6 @@ public class ContentClusterTest extends ContentBaseTest { " </documents>" + " </content>" + " </services>"); - assertNull("No own cluster controller for content", oneContentModel.getContentClusters().get("storage").getClusterControllers()); assertNotNull("Shared cluster controller with content", oneContentModel.getAdmin().getClusterControllers()); String twoContentServices = "<?xml version='1.0' encoding='UTF-8' ?>" + @@ -1108,8 +1106,6 @@ public class ContentClusterTest extends ContentBaseTest { VespaModel twoContentModel = createEnd2EndOneNode(new TestProperties().setHostedVespa(true) .setMultitenant(true), twoContentServices); - assertNull("No own cluster controller for content", twoContentModel.getContentClusters().get("storage").getClusterControllers()); - assertNull("No own cluster controller for content", twoContentModel.getContentClusters().get("dev-null").getClusterControllers()); assertNotNull("Shared cluster controller with content", twoContentModel.getAdmin().getClusterControllers()); ClusterControllerContainerCluster clusterControllers = twoContentModel.getAdmin().getClusterControllers(); 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 1a635d5236a..d2fb5fd6f4b 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 @@ -168,7 +168,6 @@ public class ModelContextImpl implements ModelContext { private final boolean skipMbusReplyThread; private final boolean useAsyncMessageHandlingOnSchedule; private final double feedConcurrency; - private final boolean useBucketExecutorForPruneRemoved; private final boolean enableFeedBlockInDistributor; private final ToIntFunction<ClusterSpec.Type> metricsProxyMaxHeapSizeInMb; private final List<String> allowedAthenzProxyIdentities; @@ -192,7 +191,6 @@ public class ModelContextImpl implements ModelContext { this.skipMbusReplyThread = flagValue(source, appId, Flags.SKIP_MBUS_REPLY_THREAD); this.useAsyncMessageHandlingOnSchedule = flagValue(source, appId, Flags.USE_ASYNC_MESSAGE_HANDLING_ON_SCHEDULE); this.feedConcurrency = flagValue(source, appId, Flags.FEED_CONCURRENCY); - this.useBucketExecutorForPruneRemoved = flagValue(source, appId, Flags.USE_BUCKET_EXECUTOR_FOR_PRUNE_REMOVED); this.enableFeedBlockInDistributor = flagValue(source, appId, Flags.ENABLE_FEED_BLOCK_IN_DISTRIBUTOR); this.metricsProxyMaxHeapSizeInMb = type -> Flags.METRICS_PROXY_MAX_HEAP_SIZE_IN_MB.bindTo(source).with(CLUSTER_TYPE, type.name()).value(); this.allowedAthenzProxyIdentities = flagValue(source, appId, Flags.ALLOWED_ATHENZ_PROXY_IDENTITIES); @@ -216,7 +214,6 @@ public class ModelContextImpl implements ModelContext { @Override public boolean skipMbusReplyThread() { return skipMbusReplyThread; } @Override public boolean useAsyncMessageHandlingOnSchedule() { return useAsyncMessageHandlingOnSchedule; } @Override public double feedConcurrency() { return feedConcurrency; } - @Override public boolean useBucketExecutorForPruneRemoved() { return useBucketExecutorForPruneRemoved; } @Override public boolean enableFeedBlockInDistributor() { return enableFeedBlockInDistributor; } @Override public int metricsProxyMaxHeapSizeInMb(ClusterSpec.Type type) { return metricsProxyMaxHeapSizeInMb.applyAsInt(type); } @Override public List<String> allowedAthenzProxyIdentities() { return allowedAthenzProxyIdentities; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ClusterData.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ClusterData.java index d62ab781a50..b6163809f26 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ClusterData.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ClusterData.java @@ -56,9 +56,9 @@ public class ClusterData { scalingEvents == null ? List.of() : scalingEvents.stream().map(data -> data.toScalingEvent()).collect(Collectors.toList()), autoscalingStatus, - Duration.ofMillis(scalingDuration), - maxQueryGrowthRate, - currentQueryFractionOfMax); + scalingDuration == null ? Duration.ofMillis(0) : Duration.ofMillis(scalingDuration), + maxQueryGrowthRate == null ? -1 : maxQueryGrowthRate, + currentQueryFractionOfMax == null ? -1 : currentQueryFractionOfMax); } } 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 5fe1c70028b..c7270b6c426 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 @@ -182,7 +182,7 @@ public class InternalStepRunner implements StepRunner { private Optional<RunStatus> deployReal(RunId id, DualLogger logger) { Versions versions = controller.jobController().run(id).get().versions(); logger.log("Deploying platform version " + versions.targetPlatform() + - " and application version " + versions.targetApplication().id() + " ..."); + " and application version " + versions.targetApplication().id() + " ..."); return deployReal(id, false, logger); } @@ -239,7 +239,7 @@ public class InternalStepRunner implements StepRunner { case ACTIVATION_CONFLICT: case APPLICATION_LOCK_FAILURE: logger.log("Deployment failed with possibly transient error " + e.code() + - ", will retry: " + e.getMessage()); + ", will retry: " + e.getMessage()); return result; case LOAD_BALANCER_NOT_READY: case PARENT_HOST_NOT_READY: diff --git a/dist/vespa.spec b/dist/vespa.spec index de92077fdbd..34afc46c80c 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -724,9 +724,12 @@ fi %defattr(-,%{_vespa_user},%{_vespa_group},-) %endif %dir %{_prefix} +%dir %{_prefix}/conf +%dir %{_prefix}/conf/vespa-feed-client %dir %{_prefix}/lib %dir %{_prefix}/lib/jars %{_prefix}/bin/vespa-feed-client +%{_prefix}/conf/vespa-feed-client/logging.properties %{_prefix}/lib/jars/vespa-http-client-jar-with-dependencies.jar %{_prefix}/lib/jars/vespa-feed-client-cli.jar 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 aeec579183d..3f148f9174c 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -123,7 +123,7 @@ public class Flags { public static final UnboundBooleanFlag HIDE_SHARED_ROUTING_ENDPOINT = defineFeatureFlag( "hide-shared-routing-endpoint", false, - List.of("tokle"), "2020-12-02", "2021-06-01", + List.of("tokle", "bjormel"), "2020-12-02", "2021-09-01", "Whether the controller should hide shared routing layer endpoint", "Takes effect immediately", APPLICATION_ID @@ -143,13 +143,6 @@ public class Flags { "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); - public static final UnboundBooleanFlag USE_BUCKET_EXECUTOR_FOR_PRUNE_REMOVED = defineFeatureFlag( - "use-bucket-executor-for-prune-removed", true, - List.of("baldersheim"), "2021-05-04", "2021-06-01", - "Wheter to use content-level bucket executor or legacy frozen buckets for prune removed", - "Takes effect on next internal redeployment", - APPLICATION_ID); - public static final UnboundBooleanFlag GROUP_SUSPENSION = defineFeatureFlag( "group-suspension", true, List.of("hakon"), "2021-01-22", "2021-06-22", diff --git a/jrt/src/com/yahoo/jrt/slobrok/api/Mirror.java b/jrt/src/com/yahoo/jrt/slobrok/api/Mirror.java index 996459dc5db..058a1380480 100644 --- a/jrt/src/com/yahoo/jrt/slobrok/api/Mirror.java +++ b/jrt/src/com/yahoo/jrt/slobrok/api/Mirror.java @@ -31,16 +31,16 @@ import java.util.logging.Level; */ public class Mirror implements IMirror { - private static Logger log = Logger.getLogger(Mirror.class.getName()); + private static final Logger log = Logger.getLogger(Mirror.class.getName()); private final Supervisor orb; private final SlobrokList slobroks; - private String currSlobrok; - private final BackOffPolicy backOff; - private volatile int updates = 0; + private String currSlobrok; + private final BackOffPolicy backOff; + private volatile int updates = 0; private boolean requestDone = false; private boolean logOnSuccess = true; - private AtomicReference<Entry[]> specs = new AtomicReference<>(new Entry[0]); + private final AtomicReference<Entry[]> specs = new AtomicReference<>(new Entry[0]); private int specsGeneration = 0; private final TransportThread transportThread; private final Task updateTask; @@ -55,7 +55,7 @@ public class Mirror implements IMirror { * @param orb the Supervisor to use * @param slobroks slobrok connect spec list * @param bop custom backoff policy, mostly useful for testing - **/ + */ public Mirror(Supervisor orb, SlobrokList slobroks, BackOffPolicy bop) { this.orb = orb; this.slobroks = slobroks; diff --git a/jrt/src/com/yahoo/jrt/slobrok/api/SlobrokList.java b/jrt/src/com/yahoo/jrt/slobrok/api/SlobrokList.java index 10d8923d9f5..654ccd7e350 100644 --- a/jrt/src/com/yahoo/jrt/slobrok/api/SlobrokList.java +++ b/jrt/src/com/yahoo/jrt/slobrok/api/SlobrokList.java @@ -27,7 +27,6 @@ public class SlobrokList { } } - public String nextSlobrokSpec() { checkUpdate(); if (idx < slobroks.length) { @@ -90,4 +89,5 @@ public class SlobrokList { return Arrays.toString(slobroks); } } + } diff --git a/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/SlobrokConfigSubscriber.java b/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/SlobrokConfigSubscriber.java index 7f4c27a45f9..d576ec50af7 100755 --- a/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/SlobrokConfigSubscriber.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/network/rpc/SlobrokConfigSubscriber.java @@ -12,13 +12,13 @@ import com.yahoo.cloud.config.SlobroksConfig; */ public class SlobrokConfigSubscriber implements ConfigSubscriber.SingleSubscriber<SlobroksConfig>{ - private SlobrokList slobroks = new SlobrokList(); + private final SlobrokList slobroks = new SlobrokList(); private ConfigSubscriber subscriber; /** * Constructs a new config subscriber for a given config id. * - * @param configId The id of the config to subscribe to. + * @param configId the id of the config to subscribe to */ public SlobrokConfigSubscriber(String configId) { subscriber = new ConfigSubscriber(); @@ -55,4 +55,5 @@ public class SlobrokConfigSubscriber implements ConfigSubscriber.SingleSubscribe subscriber.close(); } } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java index e4b85b5317e..0fb537d8462 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java @@ -9,7 +9,10 @@ import com.yahoo.vespa.hosted.provision.applications.ScalingEvent; import java.time.Clock; import java.time.Duration; +import java.util.Optional; import java.util.OptionalDouble; +import java.util.logging.Level; +import java.util.logging.Logger; /** * A cluster with its associated metrics which allows prediction about its future behavior. @@ -19,6 +22,8 @@ import java.util.OptionalDouble; */ public class ClusterModel { + private static final Logger log = Logger.getLogger(ClusterModel.class.getName()); + private static final Duration CURRENT_LOAD_DURATION = Duration.ofMinutes(5); static final double idealQueryCpuLoad = 0.8; @@ -188,4 +193,24 @@ public class ClusterModel { return duration; } + /** + * Create a cluster model if possible and logs a warning and returns empty otherwise. + * This is useful in cases where it's possible to continue without the cluser model, + * as QuestDb is known to temporarily fail during reading of data. + */ + public static Optional<ClusterModel> create(Application application, + Cluster cluster, + ClusterSpec clusterSpec, + NodeList clusterNodes, + MetricsDb metricsDb, + Clock clock) { + try { + return Optional.of(new ClusterModel(application, cluster, clusterSpec, clusterNodes, metricsDb, clock)); + } + catch (Exception e) { + log.log(Level.WARNING, "Failed creating a cluster model for " + application + " " + cluster, e); + return Optional.empty(); + } + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java index 9665d8872de..10069fd1a18 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.maintenance; +import com.google.common.collect.Sets; import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -22,6 +23,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Predicate; import java.util.logging.Level; +import java.util.logging.Logger; import java.util.stream.Collectors; /** @@ -37,6 +39,8 @@ import java.util.stream.Collectors; */ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { + private static final Logger LOG = Logger.getLogger(LoadBalancerExpirer.class.getName()); + private static final Duration reservedExpiry = Duration.ofHours(1); private static final Duration inactiveExpiry = Duration.ofHours(1); @@ -72,6 +76,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { lb.changedAt().isBefore(expiry) && allocatedNodes(lb.id()).isEmpty(), lb -> { try { + log.log(Level.INFO, () -> "Removing expired inactive load balancer " + lb.id()); service.remove(lb.id().application(), lb.id().cluster()); db.removeLoadBalancer(lb.id()); } catch (Exception e){ @@ -80,13 +85,12 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { } }); if (!failed.isEmpty()) { - log.log(Level.WARNING, String.format("Failed to remove %d load balancers: %s, retrying in %s", - failed.size(), - failed.stream() - .map(LoadBalancerId::serializedForm) - .collect(Collectors.joining(", ")), - interval()), - lastException.get()); + log.log(Level.WARNING, lastException.get(), () -> String.format("Failed to remove %d load balancers: %s, retrying in %s", + failed.size(), + failed.stream() + .map(LoadBalancerId::serializedForm) + .collect(Collectors.joining(", ")), + interval())); } return lastException.get() == null; } @@ -101,7 +105,9 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { var reals = new LinkedHashSet<>(lb.instance().get().reals()); // Remove any real no longer allocated to this application reals.removeIf(real -> !allocatedNodes.contains(real.hostname().value())); + if (reals.equals(lb.instance().get().reals())) return; // Nothing to remove try { + LOG.log(Level.INFO, () -> "Removing reals from inactive load balancer " + lb.id() + ": " + Sets.difference(lb.instance().get().reals(), reals)); service.create(new LoadBalancerSpec(lb.id().application(), lb.id().cluster(), reals), true); db.writeLoadBalancer(lb.with(lb.instance().map(instance -> instance.withReals(reals)))); } catch (Exception e) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index 70fac32f7a6..3afe5824af5 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -248,7 +248,7 @@ public class Nodes { public List<Node> fail(List<Node> nodes, Agent agent, String reason) { NestedTransaction transaction = new NestedTransaction(); nodes = fail(nodes, agent, reason, transaction); - transaction.commit();; + transaction.commit(); return nodes; } @@ -642,16 +642,22 @@ public class Nodes { List<Node> result; boolean wantToDeprovision = op == DecommissionOperation.deprovision; boolean wantToRebuild = op == DecommissionOperation.rebuild; + Optional<Report> wantToEncryptReport = op == DecommissionOperation.encrypt + ? Optional.of(Report.basicReport(Report.WANT_TO_ENCRYPT_ID, Report.Type.UNSPECIFIED, instant, "")) + : Optional.empty(); try (NodeMutex lock = nodeMutex.get(); Mutex allocationLock = lockUnallocated()) { // This takes allocationLock to prevent any further allocation of nodes on this host host = lock.node(); - result = performOn(list(allocationLock).childrenOf(host), - (node, nodeLock) -> write(node.withWantToRetire(true, wantToDeprovision, wantToRebuild, agent, instant), - nodeLock)); + result = performOn(list(allocationLock).childrenOf(host), (node, nodeLock) -> { + Node newNode = node.withWantToRetire(true, wantToDeprovision, wantToRebuild, agent, instant); + if (wantToEncryptReport.isPresent()) { + newNode = newNode.with(newNode.reports().withReport(wantToEncryptReport.get())); + } + return write(newNode, nodeLock); + }); Node newHost = host.withWantToRetire(true, wantToDeprovision, wantToRebuild, agent, instant); - if (op == DecommissionOperation.encrypt) { - Report report = Report.basicReport(Report.WANT_TO_ENCRYPT_ID, Report.Type.UNSPECIFIED, instant, ""); - newHost = newHost.with(newHost.reports().withReport(report)); + if (wantToEncryptReport.isPresent()) { + newHost = newHost.with(newHost.reports().withReport(wantToEncryptReport.get())); } result.add(write(newHost, lock)); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index e665ace6334..c114aa58a05 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -182,7 +182,8 @@ public class LoadBalancerProvisioner { : loadBalancer.get().state(); newLoadBalancer = loadBalancer.get().with(instance).with(state, now); if (loadBalancer.get().state() != newLoadBalancer.state()) { - log.log(Level.FINE, () -> "Moving " + newLoadBalancer.id() + " to state " + newLoadBalancer.state()); + log.log(Level.INFO, () -> "Moving " + newLoadBalancer.id() + " from " + loadBalancer.get().state() + + " to " + newLoadBalancer.state()); } } @@ -209,13 +210,13 @@ public class LoadBalancerProvisioner { private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, Set<Real> reals, Optional<LoadBalancer> currentLoadBalancer) { if (hasReals(currentLoadBalancer, reals)) return currentLoadBalancer.get().instance(); - log.log(Level.FINE, () -> "Creating " + id + ", targeting: " + reals); + log.log(Level.INFO, () -> "Creating " + id + ", targeting: " + reals); try { return Optional.of(service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals), allowEmptyReals(currentLoadBalancer))); } catch (Exception e) { - log.log(Level.WARNING, "Could not (re)configure " + id + ", targeting: " + - reals + ". The operation will be retried on next deployment", e); + log.log(Level.WARNING, e, () -> "Could not (re)configure " + id + ", targeting: " + + reals + ". The operation will be retried on next deployment"); } return Optional.empty(); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java index 380003affb3..9c6efd9efe6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java @@ -15,6 +15,7 @@ import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; import java.net.URI; import java.util.List; +import java.util.Optional; /** * Serializes application information for nodes/v2/application responses @@ -61,8 +62,7 @@ public class ApplicationSerializer { NodeList nodes = applicationNodes.not().retired().cluster(cluster.id()); if (nodes.isEmpty()) return; ClusterResources currentResources = nodes.toResources(); - ClusterModel clusterModel = new ClusterModel(application, cluster, nodes.clusterSpec(), nodes, metricsDb, nodeRepository.clock()); - + Optional<ClusterModel> clusterModel = ClusterModel.create(application, cluster, nodes.clusterSpec(), nodes, metricsDb, nodeRepository.clock()); Cursor clusterObject = clustersObject.setObject(cluster.id().value()); clusterObject.setString("type", nodes.clusterSpec().type().name()); toSlime(cluster.minResources(), clusterObject.setObject("min")); @@ -71,12 +71,12 @@ public class ApplicationSerializer { if (cluster.shouldSuggestResources(currentResources)) cluster.suggestedResources().ifPresent(suggested -> toSlime(suggested.resources(), clusterObject.setObject("suggested"))); cluster.targetResources().ifPresent(target -> toSlime(target, clusterObject.setObject("target"))); - clusterUtilizationToSlime(clusterModel, clusterObject.setObject("utilization")); + clusterModel.ifPresent(model -> clusterUtilizationToSlime(model, clusterObject.setObject("utilization"))); scalingEventsToSlime(cluster.scalingEvents(), clusterObject.setArray("scalingEvents")); clusterObject.setString("autoscalingStatus", cluster.autoscalingStatus()); - clusterObject.setLong("scalingDuration", clusterModel.scalingDuration().toMillis()); - clusterObject.setDouble("maxQueryGrowthRate", clusterModel.maxQueryGrowthRate()); - clusterObject.setDouble("currentQueryFractionOfMax", clusterModel.queryFractionOfMax()); + clusterModel.ifPresent(model -> clusterObject.setLong("scalingDuration", model.scalingDuration().toMillis())); + clusterModel.ifPresent(model -> clusterObject.setDouble("maxQueryGrowthRate", model.maxQueryGrowthRate())); + clusterModel.ifPresent(model -> clusterObject.setDouble("currentQueryFractionOfMax", model.queryFractionOfMax())); } private static void toSlime(ClusterResources resources, Cursor clusterResourcesObject) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java index 4df7526c07f..010fc40ded7 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java @@ -70,7 +70,7 @@ public class HostEncrypterTest { assertEquals(owners.size(), hostsEncrypting.size()); for (int i = 0; i < hostsEncrypting.size(); i++) { Optional<ApplicationId> owner = owners.get(i); - List<Node> retiringChildren = allNodes.childrenOf(hostsEncrypting.get(i)).retiring().asList(); + List<Node> retiringChildren = allNodes.childrenOf(hostsEncrypting.get(i)).retiring().encrypting().asList(); assertEquals(owner.isPresent() ? 1 : 0, retiringChildren.size()); assertEquals("Encrypting host of " + owner.map(ApplicationId::toString) .orElse("no application"), diff --git a/storage/src/tests/distributor/mergeoperationtest.cpp b/storage/src/tests/distributor/mergeoperationtest.cpp index 7a90299bfb3..1026ea2855e 100644 --- a/storage/src/tests/distributor/mergeoperationtest.cpp +++ b/storage/src/tests/distributor/mergeoperationtest.cpp @@ -315,7 +315,7 @@ TEST_F(MergeOperationTest, allow_deleting_active_source_only_replica) { _sender.getLastCommand(true)); } -TEST_F(MergeOperationTest, MarkRedundantTrustedCopiesAsSourceOnly) { +TEST_F(MergeOperationTest, mark_redundant_trusted_copies_as_source_only) { // This test uses the same distribution as testGenerateNodeList(), i.e. // an ideal state sequence of [3, 5, 7, 6, 8, 0, 9, 2, 1, 4] @@ -415,6 +415,21 @@ TEST_F(MergeOperationTest, merge_operation_is_blocked_by_any_busy_target_node) { EXPECT_TRUE(op.isBlocked(*_pendingTracker, _operation_sequencer)); } + +TEST_F(MergeOperationTest, global_bucket_merges_are_not_blocked_by_busy_nodes) { + getClock().setAbsoluteTimeInSeconds(10); + document::BucketId bucket_id(16, 1); + addNodesToBucketDB(bucket_id, "0=10/1/1/t,1=20/1/1,2=10/1/1/t"); + enableDistributorClusterState("distributor:1 storage:3"); + document::Bucket global_bucket(document::FixedBucketSpaces::global_space(), bucket_id); + MergeOperation op(BucketAndNodes(global_bucket, toVector<uint16_t>(0, 1, 2))); + op.setIdealStateManager(&getIdealStateManager()); + + // Node 1 is included in operation node set but should not cause a block of global bucket merge + _pendingTracker->getNodeInfo().setBusy(0, std::chrono::seconds(10)); + EXPECT_FALSE(op.isBlocked(*_pendingTracker, _operation_sequencer)); +} + TEST_F(MergeOperationTest, merge_operation_is_blocked_by_locked_bucket) { getClock().setAbsoluteTimeInSeconds(10); addNodesToBucketDB(document::BucketId(16, 1), "0=10/1/1/t,1=20/1/1,2=10/1/1/t"); diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp index 142ff72bc79..1a48df0fd7c 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.cpp @@ -9,8 +9,8 @@ #include <vespa/log/log.h> LOG_SETUP(".distributor.operation"); -using namespace storage; -using namespace storage::distributor; +namespace storage::distributor { + using document::BucketSpace; const uint32_t IdealStateOperation::MAINTENANCE_MESSAGE_TYPES[] = @@ -85,7 +85,7 @@ IdealStateOperation::setIdealStateManager(IdealStateManager* manager) { void IdealStateOperation::done() { - if (_manager != NULL) { + if (_manager) { if (ok()) { _manager->getMetrics().operations[getType()]->ok.inc(1); } else { @@ -107,35 +107,6 @@ IdealStateOperation::setCommandMeta(api::MaintenanceCommand& cmd) const cmd.setReason(_detailedReason); } -std::string -IdealStateOperation::toXML(framework::Clock& clock) const -{ - std::ostringstream ost; - - ost << "<operation bucketid=\"" << getBucketId() - << "\" reason=\"" << _detailedReason << "\" operations=\""; - - ost << getName() << "["; - for (uint32_t j = 0; j < getNodes().size(); j++) { - if (j != 0) { - ost << ","; - } - ost << getNodes()[j]; - } - ost << "]"; - - if (getStartTime().isSet()) { - uint64_t timeSpent( - (clock.getTimeInMillis() - getStartTime()).getTime()); - ost << "\" runtime_secs=\"" << timeSpent << "\""; - } else { - ost << "\""; - } - - ost << "/>"; - return ost.str(); -} - namespace { class IdealStateOpChecker : public PendingMessageTracker::Checker @@ -277,3 +248,5 @@ IdealStateOperation::shouldBlockThisOperation(uint32_t messageType, return false; } + +} diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h index 7906150d0cb..0e45d7f3b3a 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/idealstateoperation.h @@ -208,8 +208,6 @@ public: */ void setCommandMeta(api::MaintenanceCommand& cmd) const; - std::string toXML(framework::Clock& clock) const; - std::string toString() const override; /** diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.cpp index 481506096eb..27e203a9060 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.cpp @@ -1,5 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "mergeoperation.h" +#include <vespa/document/bucket/fixed_bucket_spaces.h> #include <vespa/storage/distributor/idealstatemanager.h> #include <vespa/storage/distributor/distributor_bucket_space.h> #include <vespa/storage/distributor/pendingmessagetracker.h> @@ -325,13 +326,30 @@ bool MergeOperation::shouldBlockThisOperation(uint32_t messageType, uint8_t pri) bool MergeOperation::isBlocked(const PendingMessageTracker& pending_tracker, const OperationSequencer& op_seq) const { - const auto& node_info = pending_tracker.getNodeInfo(); - for (auto node : getNodes()) { - if (node_info.isBusy(node)) { - return true; + // To avoid starvation of high priority global bucket merges, we do not consider + // these for blocking due to a node being "busy" (usually caused by a full merge + // throttler queue). + // + // This is for two reasons: + // 1. When an ideal state op is blocked, it is still removed from the internal + // maintenance priority queue. This means a blocked high pri operation will + // not be retried until the next DB pass (at which point the node is likely + // to still be marked as busy when there's heavy merge traffic). + // 2. Global bucket merges have high priority and will most likely be allowed + // to enter the merge throttler queues, displacing lower priority merges. + if (!is_global_bucket_merge()) { + const auto& node_info = pending_tracker.getNodeInfo(); + for (auto node : getNodes()) { + if (node_info.isBusy(node)) { + return true; + } } } return IdealStateOperation::isBlocked(pending_tracker, op_seq); } +bool MergeOperation::is_global_bucket_merge() const noexcept { + return getBucket().getBucketSpace() == document::FixedBucketSpaces::global_space(); +} + } diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h index 5df9421e815..11b5494fd9b 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.h @@ -61,6 +61,7 @@ private: void deleteSourceOnlyNodes(const BucketDatabase::Entry& currentState, DistributorStripeMessageSender& sender); + bool is_global_bucket_merge() const noexcept; }; } diff --git a/vespa-feed-client-cli/CMakeLists.txt b/vespa-feed-client-cli/CMakeLists.txt index a918981dcd3..3967c135d1c 100644 --- a/vespa-feed-client-cli/CMakeLists.txt +++ b/vespa-feed-client-cli/CMakeLists.txt @@ -2,3 +2,4 @@ install_java_artifact(vespa-feed-client-cli) vespa_install_script(src/main/sh/vespa-feed-client.sh vespa-feed-client bin) +install(FILES src/main/resources/logging.properties DESTINATION conf/vespa-feed-client) diff --git a/vespa-feed-client-cli/src/main/resources/logging.properties b/vespa-feed-client-cli/src/main/resources/logging.properties new file mode 100644 index 00000000000..3f0e8b24e78 --- /dev/null +++ b/vespa-feed-client-cli/src/main/resources/logging.properties @@ -0,0 +1,2 @@ +# Disable verbose info logging from org.apache.hc.client5.http.impl.async.AsyncHttpRequestRetryExec +org.apache.hc.client5.http.impl.async.level = WARNING diff --git a/vespa-feed-client-cli/src/main/sh/vespa-feed-client.sh b/vespa-feed-client-cli/src/main/sh/vespa-feed-client.sh index 2a166dd40bb..ab43fca2f67 100755 --- a/vespa-feed-client-cli/src/main/sh/vespa-feed-client.sh +++ b/vespa-feed-client-cli/src/main/sh/vespa-feed-client.sh @@ -79,4 +79,6 @@ exec java \ -Djava.library.path=${VESPA_HOME}/libexec64/native:${VESPA_HOME}/lib64 \ -Djava.awt.headless=true \ -Xms128m -Xmx2048m $(getJavaOptionsIPV46) \ +--add-opens=java.base/sun.security.ssl=ALL-UNNAMED \ +-Djava.util.logging.config.file=${VESPA_HOME}/conf/vespa-feed-client/logging.properties \ -cp ${VESPA_HOME}/lib/jars/vespa-feed-client-cli.jar ai.vespa.feed.client.CliClient "$@" |