Skip to content

Commit

Permalink
Merge pull request #33084 from vespa-engine/hmusum/add-feature-flag-f…
Browse files Browse the repository at this point in the history
…or-distribution-bits

Add feature flag and use it for overriding distribution bits in dev z…
  • Loading branch information
hmusum authored Jan 6, 2025
2 parents 3a1c2e9 + 6788d2f commit 6968177
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 8 deletions.
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

0 comments on commit 6968177

Please sign in to comment.