From 0973c5c6582ff96388581206d05198e4f0a20eb7 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 14 Jun 2022 23:51:23 +0200 Subject: Add control of wether shared string repo will reclaim unused enums, or if they will just grow. --- .../com/yahoo/config/model/api/ModelContext.java | 1 + .../yahoo/config/model/deploy/TestProperties.java | 14 ++++++++++ .../vespa/model/content/ContentSearchCluster.java | 2 +- .../com/yahoo/vespa/model/search/SearchNode.java | 25 ++++++++++-------- .../vespa/model/search/test/SearchNodeTest.java | 30 ++++++++++++++++------ .../config/server/deploy/ModelContextImpl.java | 3 +++ .../src/main/java/com/yahoo/vespa/flags/Flags.java | 7 +++++ 7 files changed, 63 insertions(+), 19 deletions(-) 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 10103e50e78..94b10ac4441 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 @@ -87,6 +87,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}) default int maxUnCommittedMemory() { return 130000; } @ModelFeatureFlag(owners = {"baldersheim"}) default int maxConcurrentMergesPerNode() { return 16; } @ModelFeatureFlag(owners = {"baldersheim"}) default int maxMergeQueueSize() { return 100; } + @ModelFeatureFlag(owners = {"baldersheim"}) default boolean sharedStringRepoReclaim() { return true; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean loadCodeAsHugePages() { return false; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean containerDumpHeapOnShutdownTimeout() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default double containerShutdownTimeout() { throw new UnsupportedOperationException("TODO specify default value"); } 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 aa8a0f6e1ec..1e135b5ac6c 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 @@ -76,6 +76,8 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private List environmentVariables = List.of(); private boolean avoidRenamingSummaryFeatures = true; private boolean enableBitVectors = false; + private boolean loadCodeAsHugePages = false; + private boolean sharedStringRepoReclaim = true; private Architecture adminClusterNodeResourcesArchitecture = Architecture.getDefault(); @Override public ModelContext.FeatureFlags featureFlags() { return this; } @@ -132,6 +134,18 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public boolean avoidRenamingSummaryFeatures() { return this.avoidRenamingSummaryFeatures; } @Override public boolean enableBitVectors() { return this.enableBitVectors; } @Override public Architecture adminClusterArchitecture() { return adminClusterNodeResourcesArchitecture; } + @Override public boolean sharedStringRepoReclaim() { return sharedStringRepoReclaim; } + @Override public boolean loadCodeAsHugePages() { return loadCodeAsHugePages; } + + public TestProperties sharedStringRepoReclaim(boolean sharedStringRepoReclaim) { + this.sharedStringRepoReclaim = sharedStringRepoReclaim; + return this; + } + + public TestProperties loadCodeAsHugePages(boolean loadCodeAsHugePages) { + this.loadCodeAsHugePages = loadCodeAsHugePages; + return this; + } public TestProperties maxUnCommittedMemory(int maxUnCommittedMemory) { this.maxUnCommittedMemory = maxUnCommittedMemory; 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 1ab5fd22648..379115ab3ca 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 @@ -274,7 +274,7 @@ public class ContentSearchCluster extends AbstractConfigProducer if (element == null) { searchNode = SearchNode.create(parent, "" + node.getDistributionKey(), node.getDistributionKey(), spec, clusterName, node, flushOnShutdown, tuning, resourceLimits, deployState.isHosted(), - deployState.featureFlags().loadCodeAsHugePages(), fractionOfMemoryReserved); + fractionOfMemoryReserved, deployState.featureFlags()); searchNode.setHostResource(node.getHostResource()); searchNode.initService(deployState); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java index 3461cab1201..0c8ff954375 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.model.search; import com.yahoo.cloud.config.filedistribution.FiledistributorrpcConfig; +import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.config.provision.NodeResources; @@ -60,10 +61,10 @@ public class SearchNode extends AbstractService implements private final boolean isHostedVespa; private final boolean flushOnShutdown; private final NodeSpec nodeSpec; - private int distributionKey; + private final int distributionKey; private final String clusterName; private TransactionLogServer tls; - private AbstractService serviceLayerService; + private final AbstractService serviceLayerService; private final Optional tuning; private final Optional resourceLimits; private final double fractionOfMemoryReserved; @@ -96,7 +97,7 @@ public class SearchNode extends AbstractService implements protected SearchNode doBuild(DeployState deployState, AbstractConfigProducer ancestor, Element producerSpec) { return SearchNode.create(ancestor, name, contentNode.getDistributionKey(), nodeSpec, clusterName, contentNode, flushOnShutdown, tuning, resourceLimits, deployState.isHosted(), - deployState.featureFlags().loadCodeAsHugePages(), fractionOfMemoryReserved); + fractionOfMemoryReserved, deployState.featureFlags()); } } @@ -104,22 +105,26 @@ public class SearchNode extends AbstractService implements public static SearchNode create(AbstractConfigProducer parent, String name, int distributionKey, NodeSpec nodeSpec, String clusterName, AbstractService serviceLayerService, boolean flushOnShutdown, Optional tuning, Optional resourceLimits, boolean isHostedVespa, - boolean loadCodeAsHugePages, double fractionOfMemoryReserved) { - return new SearchNode(parent, name, distributionKey, nodeSpec, clusterName, serviceLayerService, flushOnShutdown, - tuning, resourceLimits, isHostedVespa, loadCodeAsHugePages, fractionOfMemoryReserved); + double fractionOfMemoryReserved, ModelContext.FeatureFlags featureFlags) { + SearchNode node = new SearchNode(parent, name, distributionKey, nodeSpec, clusterName, serviceLayerService, flushOnShutdown, + tuning, resourceLimits, isHostedVespa, fractionOfMemoryReserved); + if (featureFlags.loadCodeAsHugePages()) { + node.addEnvironmentVariable("VESPA_LOAD_CODE_AS_HUGEPAGES", "true"); + } + if ( ! featureFlags.sharedStringRepoReclaim()) { + node.addEnvironmentVariable("VESPA_SHARED_STRING_REPO_NO_RECLAIM", "true"); + } + return node; } private SearchNode(AbstractConfigProducer parent, String name, int distributionKey, NodeSpec nodeSpec, String clusterName, AbstractService serviceLayerService, boolean flushOnShutdown, Optional tuning, Optional resourceLimits, boolean isHostedVespa, - boolean loadCodeAsHugePages, double fractionOfMemoryReserved) { + double fractionOfMemoryReserved) { super(parent, name); this.distributionKey = distributionKey; this.serviceLayerService = serviceLayerService; this.isHostedVespa = isHostedVespa; - if (loadCodeAsHugePages) { - addEnvironmentVariable("VESPA_LOAD_CODE_AS_HUGEPAGES", "true"); - } this.fractionOfMemoryReserved = fractionOfMemoryReserved; this.nodeSpec = nodeSpec; this.clusterName = clusterName; diff --git a/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java b/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java index cbd6ac4bea9..34dbd55f01a 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java @@ -48,14 +48,14 @@ public class SearchNodeTest { root.freezeModelTopology(); } - private static SearchNode createSearchNode(MockRoot root, String name, int distributionKey, - NodeSpec nodeSpec, boolean flushOnShutDown, boolean isHosted, boolean loadCodeAsHugePages) { + private static SearchNode createSearchNode(MockRoot root, String name, int distributionKey, NodeSpec nodeSpec, + boolean flushOnShutDown, boolean isHosted, ModelContext.FeatureFlags featureFlags) { return SearchNode.create(root, name, distributionKey, nodeSpec, "mycluster", null, flushOnShutDown, - Optional.empty(), Optional.empty(), isHosted, loadCodeAsHugePages, 0.0); + Optional.empty(), Optional.empty(), isHosted, 0.0, featureFlags); } private static SearchNode createSearchNode(MockRoot root) { - return createSearchNode(root, "mynode", 3, new NodeSpec(7, 5), true, true, false); + return createSearchNode(root, "mynode", 3, new NodeSpec(7, 5), true, true, new TestProperties()); } @Test @@ -68,7 +68,7 @@ public class SearchNodeTest { @Test public void requireThatBasedirIsCorrectForElasticMode() { MockRoot root = new MockRoot(""); - SearchNode node = createSearchNode(root, "mynode", 3, new NodeSpec(7, 5), false, root.getDeployState().isHosted(), false); + SearchNode node = createSearchNode(root, "mynode", 3, new NodeSpec(7, 5), false, root.getDeployState().isHosted(), new TestProperties()); prepare(root, node, true); assertBaseDir(Defaults.getDefaults().underVespaHome("var/db/vespa/search/cluster.mycluster/n3"), node); } @@ -76,7 +76,7 @@ public class SearchNodeTest { @Test public void requireThatPreShutdownCommandIsEmptyWhenNotActivated() { MockRoot root = new MockRoot(""); - SearchNode node = createSearchNode(root, "mynode", 3, new NodeSpec(7, 5), false, root.getDeployState().isHosted(), false); + SearchNode node = createSearchNode(root, "mynode", 3, new NodeSpec(7, 5), false, root.getDeployState().isHosted(), new TestProperties()); node.setHostResource(new HostResource(new Host(node, "mynbode"))); node.initService(root.getDeployState()); assertFalse(node.getPreShutdownCommand().isPresent()); @@ -85,7 +85,7 @@ public class SearchNodeTest { @Test public void requireThatPreShutdownCommandUsesPrepareRestartWhenActivated() { MockRoot root = new MockRoot(""); - SearchNode node = createSearchNode(root, "mynode2", 4, new NodeSpec(7, 5), true, root.getDeployState().isHosted(), false); + SearchNode node = createSearchNode(root, "mynode2", 4, new NodeSpec(7, 5), true, root.getDeployState().isHosted(), new TestProperties()); node.setHostResource(new HostResource(new Host(node, "mynbode2"))); node.initService(root.getDeployState()); assertTrue(node.getPreShutdownCommand().isPresent()); @@ -94,7 +94,7 @@ public class SearchNodeTest { private void verifyCodePlacement(boolean hugePages) { MockRoot root = new MockRoot(""); - SearchNode node = createSearchNode(root, "mynode2", 4, new NodeSpec(7, 5), true, false, hugePages); + SearchNode node = createSearchNode(root, "mynode2", 4, new NodeSpec(7, 5), true, false, new TestProperties().loadCodeAsHugePages(hugePages)); node.setHostResource(new HostResource(new Host(node, "mynbode2"))); node.initService(root.getDeployState()); assertEquals(hugePages, node.getStartupCommand().contains("VESPA_LOAD_CODE_AS_HUGEPAGES=")); @@ -106,6 +106,20 @@ public class SearchNodeTest { verifyCodePlacement(false); } + private void verifySharedStringRepoReclaim(boolean sharedStringRepoReclaim) { + MockRoot root = new MockRoot(""); + SearchNode node = createSearchNode(root, "mynode2", 4, new NodeSpec(7, 5), true, false, new TestProperties().sharedStringRepoReclaim(sharedStringRepoReclaim)); + node.setHostResource(new HostResource(new Host(node, "mynbode2"))); + node.initService(root.getDeployState()); + assertEquals(sharedStringRepoReclaim, ! node.getStartupCommand().contains("VESPA_SHARED_STRING_REPO_NO_RECLAIM=")); + } + + @Test + public void requireThatSharedRepoReclaimCanBeControlled() { + verifySharedStringRepoReclaim(true); + verifySharedStringRepoReclaim(false); + } + private MockRoot createRoot(ModelContext.Properties properties) { return new MockRoot("", new DeployState.Builder().properties(properties).build()); } 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 c49e79f8698..486779dcf9a 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 @@ -208,6 +208,7 @@ public class ModelContextImpl implements ModelContext { private final boolean enableBitVectors; private final Architecture adminClusterArchitecture; private final boolean enableProxyProtocolMixedMode; + private final boolean sharedStringRepoReclaim; public FeatureFlags(FlagSource source, ApplicationId appId, Version version) { this.defaultTermwiseLimit = flagValue(source, appId, version, Flags.DEFAULT_TERM_WISE_LIMIT); @@ -252,6 +253,7 @@ public class ModelContextImpl implements ModelContext { this.enableBitVectors = flagValue(source, appId, version, Flags.ENABLE_BIT_VECTORS); this.adminClusterArchitecture = Architecture.valueOf(flagValue(source, appId, version, PermanentFlags.ADMIN_CLUSTER_NODE_ARCHITECTURE)); this.enableProxyProtocolMixedMode = flagValue(source, appId, version, Flags.ENABLE_PROXY_PROTOCOL_MIXED_MODE); + this.sharedStringRepoReclaim = flagValue(source, appId, version, Flags.SHARED_STRING_REPO_RECLAIM); } @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; } @@ -298,6 +300,7 @@ public class ModelContextImpl implements ModelContext { @Override public boolean enableBitVectors() { return this.enableBitVectors; } @Override public Architecture adminClusterArchitecture() { return adminClusterArchitecture; } @Override public boolean enableProxyProtocolMixedMode() { return enableProxyProtocolMixedMode; } + @Override public boolean sharedStringRepoReclaim() { return sharedStringRepoReclaim; } private static V flagValue(FlagSource source, ApplicationId appId, Version vespaVersion, UnboundFlag flag) { return flag.bindTo(source) 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 21170e6979c..38653e65e33 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -123,6 +123,13 @@ public class Flags { "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); + public static final UnboundBooleanFlag SHARED_STRING_REPO_RECLAIM = defineFeatureFlag( + "shared-string-repo-reclaim", true, + List.of("baldersheim"), "2022-06-14", "2023-01-01", + "Controls whether we do track usage and reclaim unused enum values in shared string repo", + "Takes effect at redeployment", + ZONE_ID, APPLICATION_ID); + public static final UnboundBooleanFlag CONTAINER_DUMP_HEAP_ON_SHUTDOWN_TIMEOUT = defineFeatureFlag( "container-dump-heap-on-shutdown-timeout", false, List.of("baldersheim"), "2021-09-25", "2023-01-01", -- cgit v1.2.3