Merge branch 'main' into ml-remove-stream-any

Author: jonathan-buttner

docs/changelog/121324.yaml

@@ -0,0 +1,6 @@
+pr: 121324
+summary: Support duplicate suggestions in completion field
+area: Suggesters
+type: bug
+issues:
+ - 82432

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/30_context.yml

@@ -395,3 +395,75 @@ setup:
                 field: suggest_multi_contexts
                 contexts:
                   location: []
+
+---
+"Duplicate suggestions in different contexts":
+  - requires:
+      cluster_features: [ "search.completion_field.duplicate.support" ]
+      reason: "Support for duplicate suggestions in different contexts"
+
+  - do:
+      index:
+        refresh: true
+        index: test
+        id:    "1"
+        body:
+          suggest_context:
+            -
+              input: "foox"
+              weight: 2
+              contexts:
+                color: ["red", "yellow"]
+            -
+              input: "foox"
+              weight: 3
+              contexts:
+                color: ["blue", "green", "yellow"]
+  - do:
+      search:
+        body:
+          suggest:
+            result:
+              text: "foo"
+              completion:
+                field: suggest_context
+                contexts:
+                  color: "red"
+
+  - length: { suggest.result: 1  }
+  - length: { suggest.result.0.options: 1  }
+  - match:  { suggest.result.0.options.0.text: "foox" }
+  - match:  { suggest.result.0.options.0._score: 2 }
+
+  - do:
+      search:
+        body:
+          suggest:
+            result:
+              text: "foo"
+              completion:
+                field: suggest_context
+                contexts:
+                  color: "yellow"
+
+  - length: { suggest.result: 1  }
+  - length: { suggest.result.0.options: 1  }
+  - match:  { suggest.result.0.options.0.text: "foox" }
+  # the highest weight wins
+  - match:  { suggest.result.0.options.0._score: 3 }
+
+  - do:
+      search:
+        body:
+          suggest:
+            result:
+              text: "foo"
+              completion:
+                field: suggest_context
+                contexts:
+                  color: "blue"
+
+  - length: { suggest.result: 1 }
+  - length: { suggest.result.0.options: 1 }
+  - match: { suggest.result.0.options.0.text: "foox" }
+  - match: { suggest.result.0.options.0._score: 3 }

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/suggest/50_completion_with_multi_fields.yml

@@ -268,3 +268,80 @@
 
   - length: { suggest.result: 1  }
   - length: { suggest.result.0.options: 1  }
+
+---
+"Duplicate suggestions in different contexts in sub-fields":
+  - requires:
+      cluster_features: [ "search.completion_field.duplicate.support" ]
+      reason: "Support for duplicate suggestions in different contexts"
+
+  - do:
+      indices.create:
+        index: completion_with_context
+        body:
+          mappings:
+            "properties":
+              "suggest_1":
+                "type": "completion"
+                "contexts":
+                  -
+                    "name": "color"
+                    "type": "category"
+                "fields":
+                  "suggest_2":
+                    "type": "completion"
+                    "contexts":
+                      -
+                        "name": "color"
+                        "type": "category"
+
+
+  - do:
+      index:
+        refresh: true
+        index: completion_with_context
+        id:    "1"
+        body:
+          suggest_1:
+            -
+              input: "foox"
+              weight: 2
+              contexts:
+                color: ["red"]
+            -
+              input: "foox"
+              weight: 3
+              contexts:
+                color: ["blue", "green"]
+  - do:
+      search:
+        body:
+          suggest:
+            result:
+              text: "foo"
+              completion:
+                field: suggest_1.suggest_2
+                contexts:
+                  color: "red"
+
+  - length: { suggest.result: 1  }
+  - length: { suggest.result.0.options: 1  }
+  - match:  { suggest.result.0.options.0.text: "foox" }
+  - match:  { suggest.result.0.options.0._score: 2 }
+
+
+  - do:
+      search:
+        body:
+          suggest:
+            result:
+              text: "foo"
+              completion:
+                field: suggest_1.suggest_2
+                contexts:
+                  color: "blue"
+
+  - length: { suggest.result: 1 }
+  - length: { suggest.result.0.options: 1 }
+  - match: { suggest.result.0.options.0.text: "foox" }
+  - match: { suggest.result.0.options.0._score: 3 }

server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java

@@ -392,7 +392,7 @@ public void parse(DocumentParserContext context) throws IOException {
         // parse
         XContentParser parser = context.parser();
         Token token = parser.currentToken();
-        Map<String, CompletionInputMetadata> inputMap = Maps.newMapWithExpectedSize(1);
+        Map<String, CompletionInputMetadataContainer> inputMap = Maps.newMapWithExpectedSize(1);
 
         if (token == Token.VALUE_NULL) { // ignore null values
             return;
@@ -405,7 +405,7 @@ public void parse(DocumentParserContext context) throws IOException {
         }
 
         // index
-        for (Map.Entry<String, CompletionInputMetadata> completionInput : inputMap.entrySet()) {
+        for (Map.Entry<String, CompletionInputMetadataContainer> completionInput : inputMap.entrySet()) {
             String input = completionInput.getKey();
             if (input.trim().isEmpty()) {
                 context.addIgnoredField(mappedFieldType.name());
@@ -420,21 +420,33 @@ public void parse(DocumentParserContext context) throws IOException {
                 }
                 input = input.substring(0, len);
             }
-            CompletionInputMetadata metadata = completionInput.getValue();
+            CompletionInputMetadataContainer cmc = completionInput.getValue();
             if (fieldType().hasContextMappings()) {
-                fieldType().getContextMappings().addField(context.doc(), fieldType().name(), input, metadata.weight, metadata.contexts);
+                for (CompletionInputMetadata metadata : cmc.getValues()) {
+                    fieldType().getContextMappings().addField(context.doc(), fieldType().name(), input, metadata.weight, metadata.contexts);
+                }
             } else {
-                context.doc().add(new SuggestField(fieldType().name(), input, metadata.weight));
+                context.doc().add(new SuggestField(fieldType().name(), input, cmc.getWeight()));
             }
         }
-
         context.addToFieldNames(fieldType().name());
-        for (CompletionInputMetadata metadata : inputMap.values()) {
-            multiFields().parse(
-                this,
-                context,
-                () -> context.switchParser(new MultiFieldParser(metadata, fieldType().name(), context.parser().getTokenLocation()))
-            );
+        for (CompletionInputMetadataContainer cmc : inputMap.values()) {
+            if (fieldType().hasContextMappings()) {
+                for (CompletionInputMetadata metadata : cmc.getValues()) {
+                    multiFields().parse(
+                        this,
+                        context,
+                        () -> context.switchParser(new MultiFieldParser(metadata, fieldType().name(), context.parser().getTokenLocation()))
+                    );
+                }
+            } else {
+                CompletionInputMetadata metadata = cmc.getValue();
+                multiFields().parse(
+                    this,
+                    context,
+                    () -> context.switchParser(new MultiFieldParser(metadata, fieldType().name(), context.parser().getTokenLocation()))
+                );
+            }
         }
     }
 
@@ -447,11 +459,13 @@ private void parse(
         DocumentParserContext documentParserContext,
         Token token,
         XContentParser parser,
-        Map<String, CompletionInputMetadata> inputMap
+        Map<String, CompletionInputMetadataContainer> inputMap
     ) throws IOException {
         String currentFieldName = null;
         if (token == Token.VALUE_STRING) {
-            inputMap.put(parser.text(), new CompletionInputMetadata(parser.text(), Collections.<String, Set<String>>emptyMap(), 1));
+            CompletionInputMetadataContainer cmc = new CompletionInputMetadataContainer(fieldType().hasContextMappings());
+            cmc.add(new CompletionInputMetadata(parser.text(), Collections.emptyMap(), 1));
+            inputMap.put(parser.text(), cmc);
         } else if (token == Token.START_OBJECT) {
             Set<String> inputs = new HashSet<>();
             int weight = 1;
@@ -531,8 +545,14 @@ private void parse(
                 }
             }
             for (String input : inputs) {
-                if (inputMap.containsKey(input) == false || inputMap.get(input).weight < weight) {
-                    inputMap.put(input, new CompletionInputMetadata(input, contextsMap, weight));
+                CompletionInputMetadata cm = new CompletionInputMetadata(input, contextsMap, weight);
+                CompletionInputMetadataContainer cmc = inputMap.get(input);
+                if (cmc != null) {
+                    cmc.add(cm);
+                } else {
+                    cmc = new CompletionInputMetadataContainer(fieldType().hasContextMappings());
+                    cmc.add(cm);
+                    inputMap.put(input, cmc);
                 }
             }
         } else {
@@ -543,10 +563,46 @@ private void parse(
         }
     }
 
+    static class CompletionInputMetadataContainer {
+        private final boolean hasContexts;
+        private final List<CompletionInputMetadata> list;
+        private CompletionInputMetadata single;
+
+        CompletionInputMetadataContainer(boolean hasContexts) {
+            this.hasContexts = hasContexts;
+            this.list = hasContexts ? new ArrayList<>() : null;
+        }
+
+        void add(CompletionInputMetadata cm) {
+            if (hasContexts) {
+                list.add(cm);
+            } else {
+                if (single == null || single.weight < cm.weight) {
+                    single = cm;
+                }
+            }
+        }
+
+        List<CompletionInputMetadata> getValues() {
+            assert hasContexts;
+            return list;
+        }
+
+        CompletionInputMetadata getValue() {
+            assert hasContexts == false;
+            return single;
+        }
+
+        int getWeight() {
+            assert hasContexts == false;
+            return single.weight;
+        }
+    }
+
     static class CompletionInputMetadata {
-        public final String input;
-        public final Map<String, Set<String>> contexts;
-        public final int weight;
+        private final String input;
+        private final Map<String, Set<String>> contexts;
+        private final int weight;
 
         CompletionInputMetadata(String input, Map<String, Set<String>> contexts, int weight) {
             this.input = input;

server/src/main/java/org/elasticsearch/node/NodeConstruction.java

@@ -1067,7 +1067,7 @@ public Map<String, String> searchFields() {
         actionModule.getReservedClusterStateService().installStateHandler(new ReservedRepositoryAction(repositoriesService));
         actionModule.getReservedClusterStateService().installStateHandler(new ReservedPipelineAction());
 
-        FileSettingsHealthIndicatorService fileSettingsHealthIndicatorService = new FileSettingsHealthIndicatorService();
+        FileSettingsHealthIndicatorService fileSettingsHealthIndicatorService = new FileSettingsHealthIndicatorService(settings);
         FileSettingsService fileSettingsService = new FileSettingsService(
             clusterService,
             actionModule.getReservedClusterStateService(),

server/src/main/java/org/elasticsearch/reservedstate/service/FileSettingsService.java

@@ -21,6 +21,8 @@
 import org.elasticsearch.cluster.metadata.ReservedStateMetadata;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.file.MasterNodeFileWatchingService;
+import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.health.HealthIndicatorDetails;
 import org.elasticsearch.health.HealthIndicatorImpact;
@@ -212,7 +214,7 @@ protected void onProcessFileChangesException(Exception e) {
     }
 
     @Override
-    protected void processInitialFileMissing() throws ExecutionException, InterruptedException, IOException {
+    protected void processInitialFileMissing() throws ExecutionException, InterruptedException {
         PlainActionFuture<ActionResponse.Empty> completion = new PlainActionFuture<>();
         logger.info("setting file [{}] not found, initializing [{}] as empty", watchedFile(), NAMESPACE);
         stateService.initEmpty(NAMESPACE, completion);
@@ -236,11 +238,29 @@ public static class FileSettingsHealthIndicatorService implements HealthIndicato
             )
         );
 
+        /**
+         * We want a length limit so we don't blow past the indexing limit in the case of a long description string.
+         * This is an {@code OperatorDynamic} setting so that if the truncation hampers troubleshooting efforts,
+         * the operator could override it and retry the operation without necessarily restarting the cluster.
+         */
+        public static final String DESCRIPTION_LENGTH_LIMIT_KEY = "fileSettings.descriptionLengthLimit";
+        static final Setting<Integer> DESCRIPTION_LENGTH_LIMIT = Setting.intSetting(
+            DESCRIPTION_LENGTH_LIMIT_KEY,
+            100,
+            1, // Need room for the ellipsis
+            Setting.Property.OperatorDynamic
+        );
+
+        private final Settings settings;
         private boolean isActive = false;
         private long changeCount = 0;
         private long failureStreak = 0;
         private String mostRecentFailure = null;
 
+        public FileSettingsHealthIndicatorService(Settings settings) {
+            this.settings = settings;
+        }
+
         public synchronized void startOccurred() {
             isActive = true;
             failureStreak = 0;
@@ -262,7 +282,16 @@ public synchronized void successOccurred() {
 
         public synchronized void failureOccurred(String description) {
             ++failureStreak;
-            mostRecentFailure = description;
+            mostRecentFailure = limitLength(description);
+        }
+
+        private String limitLength(String description) {
+            int descriptionLengthLimit = DESCRIPTION_LENGTH_LIMIT.get(settings);
+            if (description.length() > descriptionLengthLimit) {
+                return description.substring(0, descriptionLengthLimit - 1) + "…";
+            } else {
+                return description;
+            }
         }
 
         @Override

server/src/main/java/org/elasticsearch/search/SearchFeatures.java

@@ -25,9 +25,12 @@ public Set<NodeFeature> getFeatures() {
     }
 
     public static final NodeFeature RETRIEVER_RESCORER_ENABLED = new NodeFeature("search.retriever.rescorer.enabled");
+    public static final NodeFeature COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS = new NodeFeature(
+        "search.completion_field.duplicate.support"
+    );
 
     @Override
     public Set<NodeFeature> getTestFeatures() {
-        return Set.of(RETRIEVER_RESCORER_ENABLED);
+        return Set.of(RETRIEVER_RESCORER_ENABLED, COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS);
     }
 }