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

Add feature flag and use it for overriding distribution bits in dev z… #33084

Merged
merged 1 commit into from
Jan 6, 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
7 changes: 4 additions & 3 deletions config-model-api/abi-spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -1343,7 +1343,8 @@
"public boolean distributionConfigFromClusterController()",
"public boolean useLegacyWandQueryParsing()",
"public boolean forwardAllLogLevels()",
"public long zookeeperPreAllocSize()"
"public long zookeeperPreAllocSize()",
"public int distributionBitsInDev()"
],
"fields" : [ ]
},
Expand Down Expand Up @@ -1812,8 +1813,8 @@
"public final java.lang.String toString()",
"public final int hashCode()",
"public final boolean equals(java.lang.Object)",
"public java.lang.String name()",
"public java.lang.String id()"
"public java.lang.String id()",
"public java.lang.String name()"
],
"fields" : [ ]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ interface FeatureFlags {
@ModelFeatureFlag(owners = {"arnej"}) default boolean useLegacyWandQueryParsing() { return true; }
@ModelFeatureFlag(owners = {"hmusum"}) default boolean forwardAllLogLevels() { return true; }
@ModelFeatureFlag(owners = {"hmusum"}) default long zookeeperPreAllocSize() { return 65536L; }
@ModelFeatureFlag(owners = {"hmusum"}) default int distributionBitsInDev() { return 0; }
}

/** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
private boolean symmetricPutAndActivateReplicaSelection = false;
private boolean enforceStrictlyIncreasingClusterStateVersions = false;
private boolean distributionConfigFromClusterController = false;
private int distributionBitsInDev = 0;

@Override public ModelContext.FeatureFlags featureFlags() { return this; }
@Override public boolean multitenant() { return multitenant; }
Expand Down Expand Up @@ -150,6 +151,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
@Override public boolean symmetricPutAndActivateReplicaSelection() { return symmetricPutAndActivateReplicaSelection; }
@Override public boolean enforceStrictlyIncreasingClusterStateVersions() { return enforceStrictlyIncreasingClusterStateVersions; }
@Override public boolean distributionConfigFromClusterController() { return distributionConfigFromClusterController; }
@Override public int distributionBitsInDev() { return distributionBitsInDev; }

public TestProperties sharedStringRepoNoReclaim(boolean sharedStringRepoNoReclaim) {
this.sharedStringRepoNoReclaim = sharedStringRepoNoReclaim;
Expand Down Expand Up @@ -410,6 +412,11 @@ public TestProperties setContainerEndpoints(Set<ContainerEndpoint> containerEndp
return this;
}

public TestProperties setDistributionBitsInDev(int bits) {
this.distributionBitsInDev = bits;
return this;
}

public static class Spec implements ConfigServerSpec {

private final String hostName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.google.common.base.Preconditions;
import com.yahoo.config.application.api.DeployLogger;
import com.yahoo.config.model.ConfigModelContext;
import com.yahoo.config.model.api.ModelContext;
import com.yahoo.config.model.deploy.DeployState;
import com.yahoo.config.model.producer.AnyConfigProducer;
import com.yahoo.config.model.producer.TreeConfigProducer;
Expand Down Expand Up @@ -63,9 +64,9 @@
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
import java.util.logging.Level;

import static com.yahoo.vespa.model.content.DistributionBitCalculator.getDistributionBits;
import static java.util.logging.Level.INFO;
import static java.util.logging.Level.WARNING;

/**
Expand Down Expand Up @@ -98,6 +99,7 @@ public class ContentCluster extends TreeConfigProducer<AnyConfigProducer> implem
private Integer maxNodesPerMerge;
private final Zone zone;
private final Optional<Integer> distributionBitsInPreviousModel;
private final ModelContext.FeatureFlags featureFlags;

public enum DistributionMode { LEGACY, STRICT, LOOSE }
private DistributionMode distributionMode;
Expand Down Expand Up @@ -339,7 +341,7 @@ else if (admin.multitenant()) { // system tests: cluster controllers on logserve
if (hosts.size() > 1) {
var message = "When having content clusters and more than 1 config server " +
"it is recommended to configure cluster controllers explicitly.";
deployState.getDeployLogger().logApplicationPackage(Level.INFO, message);
deployState.getDeployLogger().logApplicationPackage(INFO, message);
}
admin.setClusterControllers(createClusterControllers(admin,
hosts,
Expand Down Expand Up @@ -438,6 +440,7 @@ private ContentCluster(TreeConfigProducer<?> parent, String clusterId,
this.documentSelection = routingSelection;
this.zone = deployState.zone();
this.distributionBitsInPreviousModel = distributionBitsInPreviousModel(deployState, clusterId);
this.featureFlags = deployState.featureFlags();
}

public ClusterSpec.Id id() { return ClusterSpec.Id.from(clusterId); }
Expand Down Expand Up @@ -561,8 +564,17 @@ public int distributionBits() {
// Avoid number of distribution bits being reduced
if (distributionBitsInPreviousModel.isPresent() && distributionBitsInPreviousModel.get() > distributionBits)
return distributionBitsInPreviousModel.get();
else

if (isHosted && zone.environment() == Environment.dev) {
var overriddenDistributionBits = featureFlags.distributionBitsInDev();
if (overriddenDistributionBits == 0) return distributionBits;

log.log(INFO, "Overriding distribution bits to be " + overriddenDistributionBits);
return overriddenDistributionBits;
}
else {
return distributionBits;
}
}

private boolean zoneEnvImplies16DistributionBits() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,14 @@ void testZoneDependentDistributionBits() throws Exception {

ContentCluster stagingNot16Bits = createWithZone(xml, new Zone(Environment.staging, RegionName.from("us-east-3")));
assertDistributionBitsInConfig(stagingNot16Bits, 8);

// Default should be 8 bits
ContentCluster dev8Bits = createWithZone(xml, new Zone(Environment.dev, RegionName.from("us-east-3")));
assertDistributionBitsInConfig(dev8Bits, 8);

// Overriding feature flag with 16 bits
ContentCluster dev16Bits = createWithZone(xml, new Zone(Environment.dev, RegionName.from("us-east-3")), 16);
assertDistributionBitsInConfig(dev16Bits, 16);
}

@Test
Expand Down Expand Up @@ -1194,10 +1202,19 @@ void test_routing_with_multiple_clusters() {
assertRoute(spec.getRoute(9), "storage/cluster.foo_c", "route:foo_c");
}


private ContentCluster createWithZone(String clusterXml, Zone zone) throws Exception {
return createWithZone(clusterXml, zone, 0);
}

private ContentCluster createWithZone(String clusterXml, Zone zone, int bitsInDev) throws Exception {
DeployState.Builder deployStateBuilder = new DeployState.Builder()
.zone(zone)
.properties(new TestProperties().setHostedVespa(true));
.zone(zone);
var properties = new TestProperties().setHostedVespa(true);
if (bitsInDev != 0)
properties.setDistributionBitsInDev(bitsInDev);
deployStateBuilder.properties(properties);

List<String> schemas = SchemaBuilder.createSchemas("test");
MockRoot root = ContentClusterUtils.createMockRoot(schemas, deployStateBuilder);
ContentCluster cluster = ContentClusterUtils.createCluster(clusterXml, root);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ public static class FeatureFlags implements ModelContext.FeatureFlags {
private final boolean useLegacyWandQueryParsing;
private final boolean forwardAllLogLevels;
private final long zookeeperPreAllocSize;
private final int distributionBitsInDev;

public FeatureFlags(FlagSource source, ApplicationId appId, Version version) {
this.defaultTermwiseLimit = Flags.DEFAULT_TERM_WISE_LIMIT.bindTo(source).with(appId).with(version).value();
Expand Down Expand Up @@ -268,6 +269,7 @@ public FeatureFlags(FlagSource source, ApplicationId appId, Version version) {
this.useLegacyWandQueryParsing = Flags.USE_LEGACY_WAND_QUERY_PARSING.bindTo(source).with(appId).with(version).value();
this.forwardAllLogLevels = PermanentFlags.FORWARD_ALL_LOG_LEVELS.bindTo(source).with(appId).with(version).value();
this.zookeeperPreAllocSize = Flags.ZOOKEEPER_PRE_ALLOC_SIZE_KIB.bindTo(source).value();
this.distributionBitsInDev = Flags.DISTRIBUTION_BITS_IN_DEV.bindTo(source).value();
}

@Override public int heapSizePercentage() { return heapPercentage; }
Expand Down Expand Up @@ -326,6 +328,7 @@ public FeatureFlags(FlagSource source, ApplicationId appId, Version version) {
@Override public boolean useLegacyWandQueryParsing() { return useLegacyWandQueryParsing; }
@Override public boolean forwardAllLogLevels() { return forwardAllLogLevels; }
@Override public long zookeeperPreAllocSize() { return zookeeperPreAllocSize; }
@Override public int distributionBitsInDev() { return distributionBitsInDev; }
}

public static class Properties implements ModelContext.Properties {
Expand Down
7 changes: 7 additions & 0 deletions flags/src/main/java/com/yahoo/vespa/flags/Flags.java
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,13 @@ public class Flags {
"Use new RPC method for triggering download of file reference",
"Takes effect immediately");

public static final UnboundIntFlag DISTRIBUTION_BITS_IN_DEV = defineIntFlag(
"distribution-bits-in-dev", 0,
List.of("hmusum", "vekterli"), "2025-01-06", "2025-02-01",
"If non-zero, override number of distribution bits to use in dev zone in hosted Vespa for an application.",
"Takes effect at redeployment",
INSTANCE_ID);

/** WARNING: public for testing: All flags should be defined in {@link Flags}. */
public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners,
String createdAt, String expiresAt, String description,
Expand Down
Loading