Переглянути джерело

Don't create IndexCaps objects when recording unmapped fields (#90806)

Don't create IndexCaps when returning data for unmapped fields.
General tidyup of the TransportFieldCapabilitiesAction class.
This fixes issue #90796
Simon Cooper 3 роки тому
батько
коміт
cb96c95353

+ 5 - 0
docs/changelog/90806.yaml

@@ -0,0 +1,5 @@
+pr: 90806
+summary: Don't create IndexCaps objects when recording unmapped fields
+area: Mapping
+type: enhancement
+issues: [90796]

+ 28 - 42
server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java

@@ -25,7 +25,6 @@ import org.elasticsearch.xcontent.XContentParser;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -34,7 +33,9 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.elasticsearch.index.mapper.TimeSeriesParams.TIME_SERIES_DIMENSION_PARAM;
 import static org.elasticsearch.index.mapper.TimeSeriesParams.TIME_SERIES_METRIC_PARAM;
@@ -472,6 +473,10 @@ public class FieldCapabilities implements Writeable, ToXContentObject {
         return Strings.toString(this);
     }
 
+    static FieldCapabilities buildBasic(String field, String type, String[] indices) {
+        return new FieldCapabilities(field, type, false, false, false, false, null, indices, null, null, null, null, Map.of());
+    }
+
     static class Builder {
         private final String name;
         private final String type;
@@ -532,8 +537,22 @@ public class FieldCapabilities implements Writeable, ToXContentObject {
             }
         }
 
-        void getIndices(Collection<String> indices) {
-            indiceList.forEach(cap -> indices.add(cap.name));
+        Stream<String> getIndices() {
+            return indiceList.stream().map(c -> c.name);
+        }
+
+        private String[] getNonFeatureIndices(boolean featureInAll, int featureIndices, Predicate<IndexCaps> hasFeature) {
+            if (featureInAll || featureIndices == 0) {
+                return null;
+            }
+            String[] nonFeatureIndices = new String[indiceList.size() - featureIndices];
+            int index = 0;
+            for (IndexCaps indexCaps : indiceList) {
+                if (hasFeature.test(indexCaps) == false) {
+                    nonFeatureIndices[index++] = indexCaps.name;
+                }
+            }
+            return nonFeatureIndices;
         }
 
         FieldCapabilities build(boolean withIndices) {
@@ -546,50 +565,17 @@ public class FieldCapabilities implements Writeable, ToXContentObject {
 
             // Iff this field is searchable in some indices AND non-searchable in others
             // we record the list of non-searchable indices
-            final boolean isSearchable = searchableIndices == indiceList.size();
-            final String[] nonSearchableIndices;
-            if (isSearchable || searchableIndices == 0) {
-                nonSearchableIndices = null;
-            } else {
-                nonSearchableIndices = new String[indiceList.size() - searchableIndices];
-                int index = 0;
-                for (IndexCaps indexCaps : indiceList) {
-                    if (indexCaps.isSearchable == false) {
-                        nonSearchableIndices[index++] = indexCaps.name;
-                    }
-                }
-            }
+            boolean isSearchable = searchableIndices == indiceList.size();
+            String[] nonSearchableIndices = getNonFeatureIndices(isSearchable, searchableIndices, IndexCaps::isSearchable);
 
             // Iff this field is aggregatable in some indices AND non-aggregatable in others
             // we keep the list of non-aggregatable indices
-            final boolean isAggregatable = aggregatableIndices == indiceList.size();
-            final String[] nonAggregatableIndices;
-            if (isAggregatable || aggregatableIndices == 0) {
-                nonAggregatableIndices = null;
-            } else {
-                nonAggregatableIndices = new String[indiceList.size() - aggregatableIndices];
-                int index = 0;
-                for (IndexCaps indexCaps : indiceList) {
-                    if (indexCaps.isAggregatable == false) {
-                        nonAggregatableIndices[index++] = indexCaps.name;
-                    }
-                }
-            }
+            boolean isAggregatable = aggregatableIndices == indiceList.size();
+            String[] nonAggregatableIndices = getNonFeatureIndices(isAggregatable, aggregatableIndices, IndexCaps::isAggregatable);
 
             // Collect all indices that have dimension == false if this field is marked as a dimension in at least one index
-            final boolean isDimension = dimensionIndices == indiceList.size();
-            final String[] nonDimensionIndices;
-            if (isDimension || dimensionIndices == 0) {
-                nonDimensionIndices = null;
-            } else {
-                nonDimensionIndices = new String[indiceList.size() - dimensionIndices];
-                int index = 0;
-                for (IndexCaps indexCaps : indiceList) {
-                    if (indexCaps.isDimension == false) {
-                        nonDimensionIndices[index++] = indexCaps.name;
-                    }
-                }
-            }
+            boolean isDimension = dimensionIndices == indiceList.size();
+            String[] nonDimensionIndices = getNonFeatureIndices(isDimension, dimensionIndices, IndexCaps::isDimension);
 
             final String[] metricConflictsIndices;
             if (hasConflictMetricType) {

+ 47 - 29
server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java

@@ -23,6 +23,7 @@ import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.util.concurrent.CountDown;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.indices.IndicesService;
@@ -36,14 +37,17 @@ import org.elasticsearch.transport.TransportService;
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.function.Consumer;
+import java.util.function.Function;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.elasticsearch.action.search.TransportSearchHelper.checkCCSVersionCompatibility;
 
@@ -229,43 +233,57 @@ public class TransportFieldCapabilitiesAction extends HandledTransportAction<Fie
         FieldCapabilitiesRequest request,
         List<FieldCapabilitiesFailure> failures
     ) {
-        final List<FieldCapabilitiesIndexResponse> indexResponses = indexResponsesMap.values()
-            .stream()
-            .sorted(Comparator.comparing(FieldCapabilitiesIndexResponse::getIndexName))
-            .toList();
-        final String[] indices = indexResponses.stream().map(FieldCapabilitiesIndexResponse::getIndexName).toArray(String[]::new);
-        final Map<String, Map<String, FieldCapabilities.Builder>> responseMapBuilder = new HashMap<>();
-        for (FieldCapabilitiesIndexResponse response : indexResponses) {
+        Map<String, FieldCapabilitiesIndexResponse> responses = new TreeMap<>(indexResponsesMap);
+        Map<String, Map<String, FieldCapabilities.Builder>> responseMapBuilder = new HashMap<>();
+        for (FieldCapabilitiesIndexResponse response : responses.values()) {
             innerMerge(responseMapBuilder, request, response);
         }
-        final Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
+
+        Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
         for (Map.Entry<String, Map<String, FieldCapabilities.Builder>> entry : responseMapBuilder.entrySet()) {
-            final Map<String, FieldCapabilities.Builder> typeMapBuilder = entry.getValue();
+            Map<String, FieldCapabilities.Builder> typeMapBuilder = entry.getValue();
+
+            Optional<Function<Boolean, FieldCapabilities>> unmapped = Optional.empty();
             if (request.includeUnmapped()) {
-                addUnmappedFields(indices, entry.getKey(), typeMapBuilder);
-            }
-            boolean multiTypes = typeMapBuilder.size() > 1;
-            final Map<String, FieldCapabilities> typeMap = new HashMap<>();
-            for (Map.Entry<String, FieldCapabilities.Builder> fieldEntry : typeMapBuilder.entrySet()) {
-                typeMap.put(fieldEntry.getKey(), fieldEntry.getValue().build(multiTypes));
+                // do this directly, rather than using the builder, to save creating a whole lot of objects we don't need
+                unmapped = getUnmappedFields(
+                    responses.keySet(),
+                    entry.getKey(),
+                    typeMapBuilder.values().stream().flatMap(FieldCapabilities.Builder::getIndices).collect(Collectors.toSet())
+                );
             }
-            responseMap.put(entry.getKey(), Collections.unmodifiableMap(typeMap));
+
+            boolean multiTypes = typeMapBuilder.size() + unmapped.map(f -> 1).orElse(0) > 1;
+            responseMap.put(
+                entry.getKey(),
+                Collections.unmodifiableMap(
+                    Stream.concat(
+                        typeMapBuilder.entrySet()
+                            .stream()
+                            .map(e -> Map.<String, Function<Boolean, FieldCapabilities>>entry(e.getKey(), e.getValue()::build)),
+                        unmapped.stream().map(f -> Map.entry("unmapped", f))
+                    ).collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().apply(multiTypes)))
+                )
+            );
         }
-        return new FieldCapabilitiesResponse(indices, Collections.unmodifiableMap(responseMap), failures);
+        return new FieldCapabilitiesResponse(responses.keySet().toArray(String[]::new), Collections.unmodifiableMap(responseMap), failures);
     }
 
-    private static void addUnmappedFields(String[] indices, String field, Map<String, FieldCapabilities.Builder> typeMap) {
-        final Set<String> mappedIndices = new HashSet<>();
-        typeMap.values().forEach(t -> t.getIndices(mappedIndices));
-        if (mappedIndices.size() != indices.length) {
-            final FieldCapabilities.Builder unmapped = new FieldCapabilities.Builder(field, "unmapped");
-            for (String index : indices) {
-                if (mappedIndices.contains(index) == false) {
-                    unmapped.add(index, false, false, false, false, null, Collections.emptyMap());
-                }
-            }
-            typeMap.put("unmapped", unmapped);
+    private static Optional<Function<Boolean, FieldCapabilities>> getUnmappedFields(
+        Set<String> indices,
+        String field,
+        Set<String> mappedIndices
+    ) {
+        if (mappedIndices.size() != indices.size()) {
+            return Optional.of(
+                mt -> FieldCapabilities.buildBasic(
+                    field,
+                    "unmapped",
+                    mt ? Sets.difference(indices, mappedIndices).toArray(String[]::new) : null
+                )
+            );
         }
+        return Optional.empty();
     }
 
     private void innerMerge(