Merge pull request #23821 from vespa-engine/balder/control-phrase-optimization-with-feature-flags

Control phrase execution with featureflag

7 files changed, 87 insertions, 34 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 f439e88f260..a2fca743bbb 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
@@ -77,6 +77,7 @@ public interface ModelContext {
         @ModelFeatureFlag(owners = {"baldersheim"}, comment = "Select sequencer type use while feeding") default String feedSequencerType() { return "THROUGHPUT"; }
         @ModelFeatureFlag(owners = {"baldersheim"}) default String responseSequencerType() { throw new UnsupportedOperationException("TODO specify default value"); }
         @ModelFeatureFlag(owners = {"baldersheim"}) default String queryDispatchPolicy() { return "adaptive"; }
+        @ModelFeatureFlag(owners = {"baldersheim"}) default String phraseOptimization() { return ""; }
         @ModelFeatureFlag(owners = {"baldersheim"}) default int defaultNumResponseThreads() { return 2; }
         @ModelFeatureFlag(owners = {"baldersheim"}, removeAfter="7.last") default boolean skipCommunicationManagerThread() { return true; }
         @ModelFeatureFlag(owners = {"baldersheim"}, removeAfter="7.last") default boolean skipMbusRequestThread() { return true; }
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 c2218cef684..57f01fd8f55 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
@@ -42,6 +42,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     private double defaultTermwiseLimit = 1.0;
     private String jvmGCOptions = null;
     private String queryDispatchPolicy = "adaptive";
+    private String phraseOptimization = "";
     private String sequencerType = "THROUGHPUT";
     private boolean firstTimeDeployment = false;
     private String responseSequencerType = "ADAPTIVE";
@@ -151,6 +152,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     @Override public int rpcEventsBeforeWakeup() { return rpc_events_before_wakeup; }
     @Override public String queryDispatchPolicy() { return queryDispatchPolicy; }
     @Override public boolean useTwoPhaseDocumentGc() { return useTwoPhaseDocumentGc; }
+    @Override public String phraseOptimization() { return phraseOptimization; }
 
 
     public TestProperties sharedStringRepoNoReclaim(boolean sharedStringRepoNoReclaim) {
@@ -200,6 +202,10 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
         queryDispatchPolicy = policy;
         return this;
     }
+    public TestProperties setPhraseOptimization(String value) {
+        phraseOptimization = value;
+        return this;
+    }
     public TestProperties setFeedSequencerType(String type) {
         sequencerType = type;
         return this;
diff --git a/config-model/src/main/java/com/yahoo/schema/RankProfile.java b/config-model/src/main/java/com/yahoo/schema/RankProfile.java
index 56786c733ec..acbfed30c4c 100644
--- a/config-model/src/main/java/com/yahoo/schema/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/schema/RankProfile.java
@@ -5,6 +5,7 @@ import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlModels;
 import com.google.common.collect.ImmutableMap;
 import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.application.api.DeployLogger;
+import com.yahoo.config.model.api.ModelContext;
 import com.yahoo.path.Path;
 import com.yahoo.search.query.profile.QueryProfileRegistry;
 import com.yahoo.search.query.profile.types.FieldDescription;
@@ -27,7 +28,6 @@ import com.yahoo.tensor.TensorType;
 
 import java.io.IOException;
 import java.io.Reader;
-import java.io.Serializable;
 import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -118,7 +118,7 @@ public class RankProfile implements Cloneable {
 
     private Map<Reference, Constant> constants = new LinkedHashMap<>();
 
-    private Map<String, OnnxModel> onnxModels = new LinkedHashMap<>();
+    private final Map<String, OnnxModel> onnxModels = new LinkedHashMap<>();
 
     private Set<String> filterFields = new HashSet<>();
 
@@ -142,11 +142,9 @@ public class RankProfile implements Cloneable {
      *                            and looking up rank profiles.
      */
     public RankProfile(String name, Schema schema, RankProfileRegistry rankProfileRegistry) {
-        this.name = Objects.requireNonNull(name, "name cannot be null");
-        this.schema = Objects.requireNonNull(schema, "schema cannot be null");
-        this.rankProfileRegistry = rankProfileRegistry;
-        this.applicationPackage = schema.applicationPackage();
-        this.deployLogger = schema.getDeployLogger();
+        this(name, Objects.requireNonNull(schema, "schema cannot be null"),
+                schema.applicationPackage(), schema.getDeployLogger(),
+                schema.getDeployProperties(), rankProfileRegistry);
     }
 
     /**
@@ -154,13 +152,19 @@ public class RankProfile implements Cloneable {
      *
      * @param name  the name of the new profile
      */
-    public RankProfile(String name, ApplicationPackage applicationPackage, DeployLogger deployLogger,
-                       RankProfileRegistry rankProfileRegistry) {
+    public RankProfile(String name, Schema schema, ApplicationPackage applicationPackage, DeployLogger deployLogger,
+                       ModelContext.Properties deployProperties, RankProfileRegistry rankProfileRegistry) {
         this.name = Objects.requireNonNull(name, "name cannot be null");
-        this.schema = null;
+        this.schema = schema;
         this.rankProfileRegistry = rankProfileRegistry;
         this.applicationPackage = applicationPackage;
         this.deployLogger = deployLogger;
+        if (deployProperties.featureFlags().phraseOptimization().contains("split")) {
+            addRankProperty(new RankProperty("vespa.matching.split_unpacking_iterators", "true"));
+        }
+        if (deployProperties.featureFlags().phraseOptimization().contains("delay")) {
+            addRankProperty(new RankProperty("vespa.matching.delay_unpacking_iterators", "true"));
+        }
     }
 
     public String name() { return name; }
@@ -317,7 +321,7 @@ public class RankProfile implements Cloneable {
                                              .filter(p -> nonEmptyValueFilter.test(p))
                                              .collect(Collectors.toSet());
         if (uniqueProperties.isEmpty()) return Optional.empty();
-        if (uniqueProperties.size() == 1) return Optional.of(uniqueProperties.stream().findAny().get());
+        if (uniqueProperties.size() == 1) return uniqueProperties.stream().findAny();
         throw new IllegalArgumentException("Only one of the profiles inherited by " + this + " can contain " +
                                            propertyDescription + ", but it is present in multiple");
     }
@@ -641,7 +645,7 @@ public class RankProfile implements Cloneable {
 
     /** Returns a read only map view of the rank properties to use in this profile. This is never null. */
     public Map<String, List<RankProperty>> getRankPropertyMap() {
-        if (rankProperties.size() == 0 && inherited().isEmpty()) return Map.of();
+        if (rankProperties.isEmpty() && inherited().isEmpty()) return Map.of();
         if (inherited().isEmpty()) return Collections.unmodifiableMap(rankProperties);
 
         var inheritedProperties = uniquelyInherited(p -> p.getRankPropertyMap(), m -> ! m.isEmpty(), "rank-properties")
@@ -1148,7 +1152,7 @@ public class RankProfile implements Cloneable {
      * A rank setting. The identity of a rank setting is its field name and type (not value).
      * A rank setting is immutable.
      */
-    public static class RankSetting implements Serializable {
+    public static class RankSetting {
 
         private final String fieldName;
 
@@ -1222,13 +1226,10 @@ public class RankProfile implements Cloneable {
 
         @Override
         public boolean equals(Object object) {
-            if (!(object instanceof RankSetting)) {
+            if (!(object instanceof RankSetting other)) {
                 return false;
             }
-            RankSetting other = (RankSetting)object;
-            return
-                    fieldName.equals(other.fieldName) &&
-                    type.equals(other.type);
+            return fieldName.equals(other.fieldName) && type.equals(other.type);
         }
 
         @Override
@@ -1239,7 +1240,7 @@ public class RankProfile implements Cloneable {
     }
 
     /** A rank property. Rank properties are Value Objects */
-    public static class RankProperty implements Serializable {
+    public static class RankProperty {
 
         private final String name;
         private final String value;
@@ -1260,8 +1261,7 @@ public class RankProfile implements Cloneable {
 
         @Override
         public boolean equals(Object object) {
-            if (! (object instanceof RankProperty)) return false;
-            RankProperty other=(RankProperty)object;
+            if (! (object instanceof RankProperty other)) return false;
             return (other.name.equals(this.name) && other.value.equals(this.value));
         }
 
@@ -1482,8 +1482,7 @@ public class RankProfile implements Cloneable {
         @Override
         public boolean equals(Object o) {
             if (o == this) return true;
-            if ( ! (o instanceof Constant)) return false;
-            Constant other = (Constant)o;
+            if ( ! (o instanceof Constant other)) return false;
             if ( ! other.name().equals(this.name())) return false;
             if ( ! other.type().equals(this.type())) return false;
             if ( ! other.value().equals(this.value())) return false;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java
index 7f5a78fa917..9bb03d3f07c 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java
@@ -17,10 +17,7 @@ import com.yahoo.config.model.ApplicationConfigProducerRoot;
 import com.yahoo.config.model.ConfigModelRegistry;
 import com.yahoo.config.model.ConfigModelRepo;
 import com.yahoo.config.model.NullConfigModelRegistry;
-import com.yahoo.config.model.api.ApplicationClusterInfo;
-import com.yahoo.config.model.api.HostInfo;
-import com.yahoo.config.model.api.Model;
-import com.yahoo.config.model.api.Provisioned;
+import com.yahoo.config.model.api.*;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.config.model.producer.AbstractConfigProducerRoot;
@@ -61,7 +58,6 @@ import com.yahoo.vespa.model.utils.internal.ReflectionUtil;
 import org.xml.sax.SAXException;
 
 import java.io.IOException;
-import java.io.Serializable;
 import java.lang.reflect.Constructor;
 import java.time.Instant;
 import java.util.ArrayList;
@@ -96,9 +92,7 @@ import static java.util.stream.Collectors.toUnmodifiableMap;
  *
  * @author gjoranv
  */
-public final class VespaModel extends AbstractConfigProducerRoot implements Serializable, Model {
-
-    private static final long serialVersionUID = 1L;
+public final class VespaModel extends AbstractConfigProducerRoot implements Model {
 
     public static final Logger log = Logger.getLogger(VespaModel.class.getName());
 
@@ -264,11 +258,13 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri
         DeployLogger deployLogger = deployState.getDeployLogger();
         RankProfileRegistry rankProfileRegistry = deployState.rankProfileRegistry();
         QueryProfiles queryProfiles = deployState.getQueryProfiles();
+        ModelContext.Properties deployProperties = deployState.getProperties();
         List <Future<ConvertedModel>> futureModels = new ArrayList<>();
         if ( ! importedModels.isEmpty()) { // models/ directory is available
             for (ImportedMlModel model : importedModels) {
                 // Due to automatic naming not guaranteeing unique names, there must be a 1-1 between OnnxModels and global RankProfiles.
-                RankProfile profile = new RankProfile(model.name(), applicationPackage, deployLogger, rankProfileRegistry);
+                RankProfile profile = new RankProfile(model.name(), null, applicationPackage,
+                                                      deployLogger, deployProperties, rankProfileRegistry);
                 addOnnxModelInfoFromSource(model, profile);
                 rankProfileRegistry.add(profile);
                 futureModels.add(deployState.getExecutor().submit(() -> {
@@ -285,7 +281,8 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri
                 String modelName = generatedModelDir.getPath().last();
                 if (modelName.contains(".")) continue; // Name space: Not a global profile
                 // Due to automatic naming not guaranteeing unique names, there must be a 1-1 between OnnxModels and global RankProfiles.
-                RankProfile profile = new RankProfile(modelName, applicationPackage, deployLogger, rankProfileRegistry);
+                RankProfile profile = new RankProfile(modelName, null, applicationPackage,
+                                                      deployLogger, deployProperties, rankProfileRegistry);
                 addOnnxModelInfoFromStore(modelName, profile);
                 rankProfileRegistry.add(profile);
                 futureModels.add(deployState.getExecutor().submit(() -> {
diff --git a/config-model/src/test/java/com/yahoo/schema/RankPropertiesTestCase.java b/config-model/src/test/java/com/yahoo/schema/RankPropertiesTestCase.java
index 06fa63707c0..b51d62defaf 100644
--- a/config-model/src/test/java/com/yahoo/schema/RankPropertiesTestCase.java
+++ b/config-model/src/test/java/com/yahoo/schema/RankPropertiesTestCase.java
@@ -8,6 +8,7 @@ import com.yahoo.schema.derived.AttributeFields;
 import com.yahoo.schema.derived.RawRankProfile;
 import com.yahoo.schema.parser.ParseException;
 import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlModels;
+import com.yahoo.search.query.ranking.RankProperties;
 import org.junit.jupiter.api.Test;
 
 import java.util.List;
@@ -79,6 +80,45 @@ public class RankPropertiesTestCase extends AbstractSchemaTestCase {
     }
 
     @Test
+    public void testDefaultRankProperties() throws ParseException {
+        RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
+        ApplicationBuilder builder = new ApplicationBuilder(rankProfileRegistry, new QueryProfileRegistry(), new TestProperties().setPhraseOptimization("split delay"));
+        builder.addSchema(joinLines(
+                "search test {",
+                "    document test {",
+                "        field a type string { ",
+                "            indexing: index ",
+                "        }",
+                "    }",
+                "    rank-profile a {",
+                "        first-phase {",
+                "            expression: a",
+                "        }",
+                "    }",
+                "    rank-profile b {",
+                "        first-phase {",
+                "            expression: a",
+                "        }",
+                "        rank-properties {",
+                "            query(a): 2000 ",
+                "        }",
+                "    }",
+                "}"));
+        builder.build(true);
+        Schema schema = builder.getSchema();
+        List<RankProfile.RankProperty> props = rankProfileRegistry.get(schema, "a").getRankProperties();
+        assertEquals(2, props.size());
+        assertEquals(new RankProfile.RankProperty("vespa.matching.split_unpacking_iterators","true"), props.get(0));
+        assertEquals(new RankProfile.RankProperty("vespa.matching.delay_unpacking_iterators","true"), props.get(1));
+
+        props = rankProfileRegistry.get(schema, "b").getRankProperties();
+        assertEquals(3, props.size());
+        assertEquals(new RankProfile.RankProperty("vespa.matching.split_unpacking_iterators","true"), props.get(0));
+        assertEquals(new RankProfile.RankProperty("vespa.matching.delay_unpacking_iterators","true"), props.get(1));
+        assertEquals(new RankProfile.RankProperty("query(a)","2000"), props.get(2));
+    }
+
+    @Test
     void testRankProfileMutate() throws ParseException {
         RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
         ApplicationBuilder builder = new ApplicationBuilder(rankProfileRegistry);
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 69551c62840..48450716131 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
@@ -167,6 +167,7 @@ public class ModelContextImpl implements ModelContext {
     public static class FeatureFlags implements ModelContext.FeatureFlags {
 
         private final String queryDispatchPolicy;
+        private final String phraseOptimization;
         private final double defaultTermwiseLimit;
         private final boolean useThreePhaseUpdates;
         private final String feedSequencer;
@@ -280,9 +281,11 @@ public class ModelContextImpl implements ModelContext {
             this.rpc_num_targets = flagValue(source, appId, version, Flags.RPC_NUM_TARGETS);
             this.rpc_events_before_wakeup = flagValue(source, appId, version, Flags.RPC_EVENTS_BEFORE_WAKEUP);
             this.queryDispatchPolicy = flagValue(source, appId, version, Flags.QUERY_DISPATCH_POLICY);
+            this.phraseOptimization = flagValue(source, appId, version, Flags.PHRASE_OPTIMIZATION);
         }
 
-        @Override public String queryDispatchPolicy() { return queryDispatchPolicy;}
+        @Override public String queryDispatchPolicy() { return queryDispatchPolicy; }
+        @Override public String phraseOptimization() { return phraseOptimization; }
         @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; }
         @Override public boolean useThreePhaseUpdates() { return useThreePhaseUpdates; }
         @Override public String feedSequencerType() { return feedSequencer; }
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 5527a565cf4..d5083353483 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -70,6 +70,13 @@ public class Flags {
             "Takes effect at redeployment (requires restart)",
             ZONE_ID, APPLICATION_ID);
 
+    public static final UnboundStringFlag PHRASE_OPTIMIZATION = defineStringFlag(
+            "phrase-optimization", "",
+            List.of("baldersheim"), "2022-08-28", "2023-01-01",
+            "Select phase optimization, valid values are 'delay', 'spilt' or both",
+            "Takes effect at redeployment",
+            ZONE_ID, APPLICATION_ID);
+
     public static final UnboundStringFlag FEED_SEQUENCER_TYPE = defineStringFlag(
             "feed-sequencer-type", "THROUGHPUT",
             List.of("baldersheim"), "2020-12-02", "2023-01-01",