Merge pull request #31283 from vespa-engine/vekterli/add-replica-selection-symmetry-feature-flag

Add feature flag for Put/Activate replica selection symmetry

7 files changed, 58 insertions, 9 deletions

diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json
index 888a233c62a..7f8400ec653 100644
--- a/config-model-api/abi-spec.json
+++ b/config-model-api/abi-spec.json
@@ -1328,7 +1328,8 @@
       "public int persistenceThreadMaxFeedOpBatchSize()",
       "public boolean logserverOtelCol()",
       "public com.yahoo.config.provision.SharedHosts sharedHosts()",
-      "public com.yahoo.config.provision.NodeResources$Architecture adminClusterArchitecture()"
+      "public com.yahoo.config.provision.NodeResources$Architecture adminClusterArchitecture()",
+      "public boolean symmetricPutAndActivateReplicaSelection()"
     ],
     "fields" : [ ]
   },
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 67735329287..0746079a626 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
@@ -119,6 +119,7 @@ public interface ModelContext {
         @ModelFeatureFlag(owners = {"olaa"}) default boolean logserverOtelCol() { return false; }
         @ModelFeatureFlag(owners = {"bratseth"}) default SharedHosts sharedHosts() { return SharedHosts.empty(); }
         @ModelFeatureFlag(owners = {"bratseth"}) default Architecture adminClusterArchitecture() { return Architecture.x86_64; }
+        @ModelFeatureFlag(owners = {"vekterli"}) default boolean symmetricPutAndActivateReplicaSelection() { return false; }
     }
 
     /** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */
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 3c45588a054..d67335522ed 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
@@ -84,6 +84,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     private int contentLayerMetadataFeatureLevel = 0;
     private int persistenceThreadMaxFeedOpBatchSize = 1;
     private boolean logserverOtelCol = false;
+    private boolean symmetricPutAndActivateReplicaSelection = false;
 
     @Override public ModelContext.FeatureFlags featureFlags() { return this; }
     @Override public boolean multitenant() { return multitenant; }
@@ -142,6 +143,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     @Override public int contentLayerMetadataFeatureLevel() { return contentLayerMetadataFeatureLevel; }
     @Override public int persistenceThreadMaxFeedOpBatchSize() { return persistenceThreadMaxFeedOpBatchSize; }
     @Override public boolean logserverOtelCol() { return logserverOtelCol; }
+    @Override public boolean symmetricPutAndActivateReplicaSelection() { return symmetricPutAndActivateReplicaSelection; }
 
     public TestProperties sharedStringRepoNoReclaim(boolean sharedStringRepoNoReclaim) {
         this.sharedStringRepoNoReclaim = sharedStringRepoNoReclaim;
@@ -382,6 +384,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
         return this;
     }
 
+    public TestProperties setSymmetricPutAndActivateReplicaSelection(boolean symmetricReplicaSelection) {
+        this.symmetricPutAndActivateReplicaSelection = symmetricReplicaSelection;
+        return this;
+    }
+
     public static class Spec implements ConfigServerSpec {
 
         private final String hostName;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java
index c182f0e0507..1e35a94db59 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java
@@ -35,6 +35,8 @@ public class DistributorCluster extends TreeConfigProducer<Distributor> implemen
     private final boolean hasIndexedDocumentType;
     private final int maxActivationInhibitedOutOfSyncGroups;
     private final int contentLayerMetadataFeatureLevel;
+    private final boolean symmetricPutAndActivateReplicaSelection;
+
     public static class Builder extends VespaDomBuilder.DomConfigProducerBuilderBase<DistributorCluster> {
 
         ContentCluster parent;
@@ -97,19 +99,22 @@ public class DistributorCluster extends TreeConfigProducer<Distributor> implemen
             var featureFlags = deployState.getProperties().featureFlags();
             int maxInhibitedGroups = featureFlags.maxActivationInhibitedOutOfSyncGroups();
             int contentLayerMetadataFeatureLevel = featureFlags.contentLayerMetadataFeatureLevel();
+            boolean symmetricPutAndActivateReplicaSelection = featureFlags.symmetricPutAndActivateReplicaSelection();
 
             return new DistributorCluster(parent,
                     new BucketSplitting.Builder().build(new ModelElement(producerSpec)), gc,
                     hasIndexedDocumentType,
                     maxInhibitedGroups,
-                    contentLayerMetadataFeatureLevel);
+                    contentLayerMetadataFeatureLevel,
+                    symmetricPutAndActivateReplicaSelection);
         }
     }
 
     private DistributorCluster(ContentCluster parent, BucketSplitting bucketSplitting,
                                GcOptions gc, boolean hasIndexedDocumentType,
                                int maxActivationInhibitedOutOfSyncGroups,
-                               int contentLayerMetadataFeatureLevel)
+                               int contentLayerMetadataFeatureLevel,
+                               boolean symmetricPutAndActivateReplicaSelection)
     {
         super(parent, "distributor");
         this.parent = parent;
@@ -118,6 +123,7 @@ public class DistributorCluster extends TreeConfigProducer<Distributor> implemen
         this.hasIndexedDocumentType = hasIndexedDocumentType;
         this.maxActivationInhibitedOutOfSyncGroups = maxActivationInhibitedOutOfSyncGroups;
         this.contentLayerMetadataFeatureLevel = contentLayerMetadataFeatureLevel;
+        this.symmetricPutAndActivateReplicaSelection = symmetricPutAndActivateReplicaSelection;
     }
 
     @Override
@@ -132,6 +138,7 @@ public class DistributorCluster extends TreeConfigProducer<Distributor> implemen
         if (contentLayerMetadataFeatureLevel > 0) {
             builder.enable_operation_cancellation(true);
         }
+        builder.symmetric_put_and_activate_replica_selection(symmetricPutAndActivateReplicaSelection);
         bucketSplitting.getConfig(builder);
     }
 
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 786caa4b317..a24abc21b76 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
@@ -48,6 +48,7 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.Set;
+import java.util.function.Consumer;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -1471,13 +1472,18 @@ public class ContentClusterTest extends ContentBaseTest {
         assertEquals(expectedGroupsAllowedDown, config.max_number_of_groups_allowed_to_be_down());
     }
 
-    private boolean resolveDistributorOperationCancellationConfig(Integer featureLevel) throws Exception {
+    private StorDistributormanagerConfig resolveDistributorConfig(Consumer<TestProperties> propertyMutator) throws Exception {
         var properties = new TestProperties();
-        if (featureLevel != null) {
-            properties.setContentLayerMetadataFeatureLevel(featureLevel);
-        }
-        var cfg = resolveStorDistributormanagerConfig(properties);
-        return cfg.enable_operation_cancellation();
+        propertyMutator.accept(properties);
+        return resolveStorDistributormanagerConfig(properties);
+    }
+
+    private boolean resolveDistributorOperationCancellationConfig(Integer featureLevel) throws Exception {
+        return resolveDistributorConfig((props) -> {
+            if (featureLevel != null) {
+                props.setContentLayerMetadataFeatureLevel(featureLevel);
+            }
+        }).enable_operation_cancellation();
     }
 
     @Test
@@ -1488,6 +1494,21 @@ public class ContentClusterTest extends ContentBaseTest {
         assertTrue(resolveDistributorOperationCancellationConfig(2));
     }
 
+    private boolean resolveDistributorSymmetricReplicaSelectionConfig(Boolean flagValue) throws Exception {
+        return resolveDistributorConfig((props) -> {
+            if (flagValue != null) {
+                props.setSymmetricPutAndActivateReplicaSelection(flagValue);
+            }
+        }).symmetric_put_and_activate_replica_selection();
+    }
+
+    @Test
+    void distributor_symmetric_replica_selection_config_controlled_by_properties() throws Exception {
+        assertFalse(resolveDistributorSymmetricReplicaSelectionConfig(null)); // defaults to false
+        assertFalse(resolveDistributorSymmetricReplicaSelectionConfig(false));
+        assertTrue(resolveDistributorSymmetricReplicaSelectionConfig(true));
+    }
+
     @Test
     void node_distribution_key_outside_legal_range_is_disallowed() {
         // Only [0, UINT16_MAX - 1] is a valid range. UINT16_MAX is a special content layer-internal
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 1c285270cb1..e0891854d07 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
@@ -213,6 +213,7 @@ public class ModelContextImpl implements ModelContext {
         private final boolean logserverOtelCol;
         private final SharedHosts sharedHosts;
         private final Architecture adminClusterArchitecture;
+        private final boolean symmetricPutAndActivateReplicaSelection;
 
         public FeatureFlags(FlagSource source, ApplicationId appId, Version version) {
             this.defaultTermwiseLimit = Flags.DEFAULT_TERM_WISE_LIMIT.bindTo(source).with(appId).with(version).value();
@@ -259,6 +260,7 @@ public class ModelContextImpl implements ModelContext {
             this.logserverOtelCol = Flags.LOGSERVER_OTELCOL_AGENT.bindTo(source).with(appId).with(version).value();
             this.sharedHosts = PermanentFlags.SHARED_HOST.bindTo(source).with( appId).with(version).value();
             this.adminClusterArchitecture = Architecture.valueOf(PermanentFlags.ADMIN_CLUSTER_NODE_ARCHITECTURE.bindTo(source).with(appId).with(version).value());
+            this.symmetricPutAndActivateReplicaSelection = Flags.SYMMETRIC_PUT_AND_ACTIVATE_REPLICA_SELECTION.bindTo(source).with(appId).with(version).value();
         }
 
         @Override public int heapSizePercentage() { return heapPercentage; }
@@ -313,6 +315,7 @@ public class ModelContextImpl implements ModelContext {
         @Override public boolean logserverOtelCol() { return logserverOtelCol; }
         @Override public SharedHosts sharedHosts() { return sharedHosts; }
         @Override public Architecture adminClusterArchitecture() { return adminClusterArchitecture; }
+        @Override public boolean symmetricPutAndActivateReplicaSelection() { return symmetricPutAndActivateReplicaSelection; }
 
         private String translateJvmOmitStackTraceInFastThrowToString(Predicate<ClusterSpec.Type> function,
                                                                      ClusterSpec.Type clusterType) {
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 a577bbe74df..acd61c9ccc5 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -463,6 +463,15 @@ public class Flags {
             "Whether EndpointDnsMaintainer should remove orphaned records instead of logging them",
             "Takes effect on next maintenance run");
 
+    public static final UnboundBooleanFlag SYMMETRIC_PUT_AND_ACTIVATE_REPLICA_SELECTION = defineFeatureFlag(
+            "symmetric-put-and-activate-replica-selection", false,
+            List.of("vekterli"), "2024-05-23", "2024-08-01",
+            "Iff true there will be an 1-1 symmetry between the replicas chosen as feed targets " +
+            "for Put operations and the replica selection logic for bucket activation. If false, " +
+            "legacy feed behavior is used.",
+            "Takes effect immediately",
+            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,