Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop using feature flag RESOURCE_LIMIT_MEMORY_FOR_SMALL_NODES #33256

Merged
merged 1 commit into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ interface FeatureFlags {
@ModelFeatureFlag(owners = {"vekterli"}) default int maxActivationInhibitedOutOfSyncGroups() { return 0; }
@ModelFeatureFlag(owners = {"hmusum"}) default double resourceLimitDisk() { return 0.75; }
@ModelFeatureFlag(owners = {"hmusum"}) default double resourceLimitMemory() { return 0.8; }
@ModelFeatureFlag(owners = {"hmusum"}) default double resourceLimitMemorySmallNodes() { return 0.75; }
@ModelFeatureFlag(owners = {"hmusum"}, removeAfter = "8.474") default double resourceLimitMemorySmallNodes() { return 0.75; }
@ModelFeatureFlag(owners = {"geirst", "vekterli"}) default double minNodeRatioPerGroup() { return 0.0; }
@ModelFeatureFlag(owners = {"arnej"}) default boolean forwardIssuesAsErrors() { return true; }
@ModelFeatureFlag(owners = {"arnej"}) default boolean useV8GeoPositions() { return false; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
private List<X509Certificate> operatorCertificates = List.of();
private double resourceLimitDisk = 0.75;
private double resourceLimitMemory = 0.8;
private double resourceLimitMemorySmallNodes = 0.75;
private double minNodeRatioPerGroup = 0.0;
private boolean containerDumpHeapOnShutdownTimeout = false;
private double containerShutdownTimeout = 50.0;
Expand Down Expand Up @@ -115,7 +114,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
@Override public List<X509Certificate> operatorCertificates() { return operatorCertificates; }
@Override public double resourceLimitDisk() { return resourceLimitDisk; }
@Override public double resourceLimitMemory() { return resourceLimitMemory; }
@Override public double resourceLimitMemorySmallNodes() { return resourceLimitMemorySmallNodes; }
@Override public double minNodeRatioPerGroup() { return minNodeRatioPerGroup; }
@Override public double containerShutdownTimeout() { return containerShutdownTimeout; }
@Override public boolean containerDumpHeapOnShutdownTimeout() { return containerDumpHeapOnShutdownTimeout; }
Expand Down Expand Up @@ -290,11 +288,6 @@ public TestProperties setResourceLimitMemory(double value) {
return this;
}

public TestProperties setResourceLimitMemorySmallNodes(double value) {
this.resourceLimitMemorySmallNodes = value;
return this;
}

public TestProperties setMinNodeRatioPerGroup(double value) {
this.minNodeRatioPerGroup = value;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ private static ClusterResourceLimits calculateAndSetResourceLimits(ContentCluste
.mapToDouble(s -> s.getHostResource().realResources().memoryGiB())
.min()
.orElse(16);
// Use limit for small nodes when memory < ~8 Gib
// Use smaller limit for small nodes (when memory < ~8 Gib)
if (memoryGib < 8.1)
resourceLimitMemory = deployState.featureFlags().resourceLimitMemorySmallNodes();
resourceLimitMemory = 0.75;
}

var resourceLimits = new ClusterResourceLimits.Builder(isHosted,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1642,9 +1642,7 @@ void testResourceLimitsForSmallNodes() throws Exception {
var properties = new TestProperties()
.setHostedVespa(true)
.setMultitenant(true)
.setResourceLimitDisk(0.66)
.setResourceLimitMemory(0.67)
.setResourceLimitMemorySmallNodes(0.68);
.setResourceLimitDisk(0.66);
var deployState = new DeployState.Builder()
.properties(properties)
.modelHostProvisioner(provisioner)
Expand All @@ -1658,13 +1656,14 @@ void testResourceLimitsForSmallNodes() throws Exception {
contentCluster.getClusterControllerConfig().getConfig(builder);
var config = builder.build();
assertEquals(0.66, config.cluster_feed_block_limit().get("disk"), 0.001);
assertEquals(0.68, config.cluster_feed_block_limit().get("memory"), 0.001);
// Memory resource limit for small nodes is 0.75
assertEquals(0.75, config.cluster_feed_block_limit().get("memory"), 0.001);

// Resource limits for content cluster should be changed based on new values above
var protonConfigBuilder = new ProtonConfig.Builder();
model.getConfig(protonConfigBuilder, "foo/search/cluster.foo");
assertEquals(0.864, protonConfigBuilder.build().writefilter().disklimit(), 0.001);
assertEquals(0.84, protonConfigBuilder.build().writefilter().memorylimit(), 0.001);
assertEquals(0.875, protonConfigBuilder.build().writefilter().memorylimit(), 0.001);

// Resource limits for content nodes should be changed based on new values above
// Tested since values can be set for both clusters and nodes in general
Expand All @@ -1673,7 +1672,7 @@ void testResourceLimitsForSmallNodes() throws Exception {
var config2 = protonConfigBuilder2.build();
assertEquals(0, config2.distributionkey());
assertEquals(0.864, config2.writefilter().disklimit(), 0.001);
assertEquals(0.84, config2.writefilter().memorylimit(), 0.001);
assertEquals(0.875, config2.writefilter().memorylimit(), 0.001);
}

private String servicesWithGroups(int groupCount, double minGroupUpRatio) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ public static class FeatureFlags implements ModelContext.FeatureFlags {
private final int maxActivationInhibitedOutOfSyncGroups;
private final double resourceLimitDisk;
private final double resourceLimitMemory;
private final double resourceLimitMemorySmallNodes;
private final double minNodeRatioPerGroup;
private final boolean containerDumpHeapOnShutdownTimeout;
private final boolean loadCodeAsHugePages;
Expand Down Expand Up @@ -225,7 +224,6 @@ public FeatureFlags(FlagSource source, ApplicationId appId, Version version) {
this.maxActivationInhibitedOutOfSyncGroups = Flags.MAX_ACTIVATION_INHIBITED_OUT_OF_SYNC_GROUPS.bindTo(source).with(appId).with(version).value();
this.resourceLimitDisk = PermanentFlags.RESOURCE_LIMIT_DISK.bindTo(source).with(appId).with(version).value();
this.resourceLimitMemory = PermanentFlags.RESOURCE_LIMIT_MEMORY.bindTo(source).with(appId).with(version).value();
this.resourceLimitMemorySmallNodes = Flags.RESOURCE_LIMIT_MEMORY_FOR_SMALL_NODES.bindTo(source).with(appId).with(version).value();
this.minNodeRatioPerGroup = Flags.MIN_NODE_RATIO_PER_GROUP.bindTo(source).with(appId).with(version).value();
this.containerDumpHeapOnShutdownTimeout = Flags.CONTAINER_DUMP_HEAP_ON_SHUTDOWN_TIMEOUT.bindTo(source).with(appId).with(version).value();
this.loadCodeAsHugePages = Flags.LOAD_CODE_AS_HUGEPAGES.bindTo(source).with(appId).with(version).value();
Expand Down Expand Up @@ -278,7 +276,6 @@ public FeatureFlags(FlagSource source, ApplicationId appId, Version version) {
@Override public int maxActivationInhibitedOutOfSyncGroups() { return maxActivationInhibitedOutOfSyncGroups; }
@Override public double resourceLimitDisk() { return resourceLimitDisk; }
@Override public double resourceLimitMemory() { return resourceLimitMemory; }
@Override public double resourceLimitMemorySmallNodes() { return resourceLimitMemorySmallNodes; }
@Override public double minNodeRatioPerGroup() { return minNodeRatioPerGroup; }
@Override public double containerShutdownTimeout() { return containerShutdownTimeout; }
@Override public boolean containerDumpHeapOnShutdownTimeout() { return containerDumpHeapOnShutdownTimeout; }
Expand Down
7 changes: 0 additions & 7 deletions flags/src/main/java/com/yahoo/vespa/flags/Flags.java
Original file line number Diff line number Diff line change
Expand Up @@ -439,13 +439,6 @@ public class Flags {
"Use new RPC method for triggering download of file reference",
"Takes effect immediately");

public static final UnboundDoubleFlag RESOURCE_LIMIT_MEMORY_FOR_SMALL_NODES = defineDoubleFlag(
"resource-limit-memory-for-small-nodes", 0.75,
List.of("hmusum"), "2025-01-12", "2025-02-15",
"Resource limit (between 0.0 and 1.0) for memory usage on small content nodes (8 Gib memory or less), used by cluster controller for when to block feed",
"Takes effect on next deployment",
INSTANCE_ID);

public static final UnboundIntFlag DOCUMENT_V1_QUEUE_SIZE = defineIntFlag(
"document-v1-queue-size", -1,
List.of("bjorncs"), "2025-01-14", "2025-12-01",
Expand Down