From f50caf7ee30872428b200f2da63a753c768a8dbf Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 30 Apr 2022 23:07:25 -0400 Subject: [PATCH 1/9] Speed up merging field-caps responses from single mapping --- .../search/fieldcaps/FieldCapabilitiesIT.java | 50 +++++++++++++++++++ .../TransportFieldCapabilitiesAction.java | 50 +++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java index a52940ae9a413..40147805851df 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java @@ -58,6 +58,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.IntStream; import static java.util.Collections.singletonList; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; @@ -68,6 +69,8 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; public class FieldCapabilitiesIT extends ESIntegTestCase { @@ -566,6 +569,53 @@ public void testRelocation() throws Exception { } } + public void testManyIndicesWithSameMapping() { + final String mapping = """ + { + "properties": { + "message_field": { "type": "text" }, + "value_field": { "type": "long" }, + "multi_field" : { "type" : "ip", "fields" : { "keyword" : { "type" : "keyword" } } }, + "timestamp": {"type": "date"} + } + } + """; + String[] indices = IntStream.range(1, between(1, 9)).mapToObj(n -> "test_many_index_" + n).toArray(String[]::new); + for (String index : indices) { + assertAcked(client().admin().indices().prepareCreate(index).setMapping(mapping).get()); + } + FieldCapabilitiesRequest request = new FieldCapabilitiesRequest(); + request.indices("test_many_index_*"); + request.fields("*"); + if (randomBoolean()) { + request.includeUnmapped(randomBoolean()); + } + boolean excludeMultiField = randomBoolean(); + if (excludeMultiField) { + request.filters("-multifield"); + } + FieldCapabilitiesResponse response = client().execute(FieldCapabilitiesAction.INSTANCE, request).actionGet(); + assertThat(response.getIndices(), equalTo(indices)); + assertThat(response.get().get("message_field"), hasKey("text")); + assertThat(response.get().get("message_field").get("text").indices(), nullValue()); + assertTrue(response.get().get("message_field").get("text").isSearchable()); + assertFalse(response.get().get("message_field").get("text").isAggregatable()); + + assertThat(response.get().get("value_field"), hasKey("long")); + assertThat(response.get().get("value_field").get("long").indices(), nullValue()); + assertTrue(response.get().get("value_field").get("long").isSearchable()); + assertTrue(response.get().get("value_field").get("long").isAggregatable()); + + assertThat(response.get().get("timestamp"), hasKey("date")); + + assertThat(response.get().get("multi_field"), hasKey("ip")); + if (excludeMultiField) { + assertThat(response.get().get("multi_field.keyword"), not(hasKey("keyword"))); + } else { + assertThat(response.get().get("multi_field.keyword"), hasKey("keyword")); + } + } + private void assertIndices(FieldCapabilitiesResponse response, String... indices) { assertNotNull(response.getIndices()); Arrays.sort(indices); diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java index e720079a356db..2cb8e4731c67a 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java @@ -238,6 +238,41 @@ private FieldCapabilitiesResponse merge( .sorted(Comparator.comparing(FieldCapabilitiesIndexResponse::getIndexName)) .toList(); final String[] indices = indexResponses.stream().map(FieldCapabilitiesIndexResponse::getIndexName).toArray(String[]::new); + + if (isFromSingleMapping(indexResponses)) { + FieldCapabilitiesIndexResponse firstResponse = indexResponses.get(0); + final Map fields = ResponseRewriter.rewriteOldResponses( + firstResponse.getOriginVersion(), + firstResponse.get(), + request.filters(), + request.types(), + metadataFieldPred + ); + final Map> responseMap = fields.entrySet() + .stream() + .collect(Collectors.toMap(Map.Entry::getKey, e -> { + final IndexFieldCapabilities cap = e.getValue(); + return Map.of( + cap.getType(), + new FieldCapabilities( + cap.getName(), + cap.getType(), + cap.isMetadatafield(), + cap.isSearchable(), + cap.isAggregatable(), + cap.isDimension(), + cap.getMetricType(), + null, + null, + null, + null, + null, + cap.meta().entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, v -> Set.of(v.getValue()))) + ) + ); + })); + return new FieldCapabilitiesResponse(indices, responseMap, failures); + } final Map> responseMapBuilder = new HashMap<>(); for (FieldCapabilitiesIndexResponse response : indexResponses) { innerMerge(responseMapBuilder, request, response); @@ -272,6 +307,21 @@ private static void addUnmappedFields(String[] indices, String field, Map indexResponses) { + String sharedMappingHash = null; + for (FieldCapabilitiesIndexResponse resp : indexResponses) { + if (resp.getIndexMappingHash() == null) { + return false; + } + if (sharedMappingHash == null) { + sharedMappingHash = resp.getIndexMappingHash(); + } else if (sharedMappingHash.equals(resp.getIndexMappingHash()) == false) { + return false; + } + } + return sharedMappingHash != null; + } + private void innerMerge( Map> responseMapBuilder, FieldCapabilitiesRequest request, From 0652ea20707c9c16b31e24d6a2549787a0db8f00 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sun, 1 May 2022 10:37:34 -0400 Subject: [PATCH 2/9] Update docs/changelog/86323.yaml --- docs/changelog/86323.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/86323.yaml diff --git a/docs/changelog/86323.yaml b/docs/changelog/86323.yaml new file mode 100644 index 0000000000000..39c52a3d5e066 --- /dev/null +++ b/docs/changelog/86323.yaml @@ -0,0 +1,5 @@ +pr: 86323 +summary: Speed up merging field-caps responses from single mapping +area: Search +type: enhancement +issues: [] From af2f29c73846bddd5f17716dcd4fa9142e8910d6 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Fri, 6 May 2022 20:45:20 -0400 Subject: [PATCH 3/9] General optimization --- .../search/fieldcaps/FieldCapabilitiesIT.java | 65 ++++++--- .../action/fieldcaps/FieldCapabilities.java | 76 ++++++---- .../TransportFieldCapabilitiesAction.java | 85 ++++-------- .../fieldcaps/FieldCapabilitiesTests.java | 131 ++++++++++-------- 4 files changed, 192 insertions(+), 165 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java index 40147805851df..bdbba4df1bc20 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java @@ -56,6 +56,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.IntStream; @@ -580,40 +581,58 @@ public void testManyIndicesWithSameMapping() { } } """; - String[] indices = IntStream.range(1, between(1, 9)).mapToObj(n -> "test_many_index_" + n).toArray(String[]::new); + String[] indices = IntStream.range(0, between(1, 9)).mapToObj(n -> "test_many_index_" + n).toArray(String[]::new); for (String index : indices) { assertAcked(client().admin().indices().prepareCreate(index).setMapping(mapping).get()); } FieldCapabilitiesRequest request = new FieldCapabilitiesRequest(); request.indices("test_many_index_*"); request.fields("*"); - if (randomBoolean()) { - request.includeUnmapped(randomBoolean()); - } boolean excludeMultiField = randomBoolean(); if (excludeMultiField) { request.filters("-multifield"); } - FieldCapabilitiesResponse response = client().execute(FieldCapabilitiesAction.INSTANCE, request).actionGet(); - assertThat(response.getIndices(), equalTo(indices)); - assertThat(response.get().get("message_field"), hasKey("text")); - assertThat(response.get().get("message_field").get("text").indices(), nullValue()); - assertTrue(response.get().get("message_field").get("text").isSearchable()); - assertFalse(response.get().get("message_field").get("text").isAggregatable()); - - assertThat(response.get().get("value_field"), hasKey("long")); - assertThat(response.get().get("value_field").get("long").indices(), nullValue()); - assertTrue(response.get().get("value_field").get("long").isSearchable()); - assertTrue(response.get().get("value_field").get("long").isAggregatable()); - - assertThat(response.get().get("timestamp"), hasKey("date")); - - assertThat(response.get().get("multi_field"), hasKey("ip")); - if (excludeMultiField) { - assertThat(response.get().get("multi_field.keyword"), not(hasKey("keyword"))); - } else { - assertThat(response.get().get("multi_field.keyword"), hasKey("keyword")); + Consumer verifyResponse = resp -> { + assertThat(resp.getIndices(), equalTo(indices)); + assertThat(resp.getField("message_field"), hasKey("text")); + assertThat(resp.getField("message_field").get("text").indices(), nullValue()); + assertTrue(resp.getField("message_field").get("text").isSearchable()); + assertFalse(resp.getField("message_field").get("text").isAggregatable()); + + assertThat(resp.getField("value_field"), hasKey("long")); + assertThat(resp.getField("value_field").get("long").indices(), nullValue()); + assertTrue(resp.getField("value_field").get("long").isSearchable()); + assertTrue(resp.getField("value_field").get("long").isAggregatable()); + + assertThat(resp.getField("timestamp"), hasKey("date")); + + assertThat(resp.getField("multi_field"), hasKey("ip")); + if (excludeMultiField) { + assertThat(resp.getField("multi_field.keyword"), not(hasKey("keyword"))); + } else { + assertThat(resp.getField("multi_field.keyword"), hasKey("keyword")); + } + }; + // Single mapping + verifyResponse.accept(client().execute(FieldCapabilitiesAction.INSTANCE, request).actionGet()); + + // add an extra field for some indices + String[] indicesWithExtraField = randomSubsetOf(between(1, indices.length), indices).stream().sorted().toArray(String[]::new); + ensureGreen(indices); + assertAcked(client().admin().indices().preparePutMapping(indicesWithExtraField).setSource("extra_field", "type=integer").get()); + for (String index : indicesWithExtraField) { + client().prepareIndex(index).setSource("extra_field", randomIntBetween(1, 1000)).get(); } + FieldCapabilitiesResponse resp = client().execute(FieldCapabilitiesAction.INSTANCE, request).actionGet(); + verifyResponse.accept(resp); + assertThat(resp.getField("extra_field"), hasKey("integer")); + assertThat( + "extra_indices " + Arrays.toString(indicesWithExtraField), + resp.getField("extra_field").get("integer").indices(), + nullValue() + ); + assertTrue(resp.getField("extra_field").get("integer").isSearchable()); + assertTrue(resp.getField("extra_field").get("integer").isAggregatable()); } private void assertIndices(FieldCapabilitiesResponse response, String... indices) { diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java index d60de9d0b9381..ccce8f88ce222 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java @@ -481,23 +481,37 @@ static class Builder { private int dimensionIndices = 0; private TimeSeriesParams.MetricType metricType; private boolean hasConflictMetricType; - private final List indiceList; + private final List indicesList; private final Map> meta; + private int totalIndices; Builder(String name, String type) { this.name = name; this.type = type; this.metricType = null; this.hasConflictMetricType = false; - this.indiceList = new ArrayList<>(); + this.indicesList = new ArrayList<>(); this.meta = new HashMap<>(); } + private boolean assertIndicesSorted(String[] indices) { + for (int i = 1; i < indices.length; i++) { + assert indices[i - 1].compareTo(indices[i]) < 0 : "indices [" + Arrays.toString(indices) + "] aren't sorted"; + } + if (indicesList.isEmpty() == false) { + final IndexCaps lastCaps = indicesList.get(indicesList.size() - 1); + final String lastIndex = lastCaps.indices[lastCaps.indices.length - 1]; + assert lastIndex.compareTo(indices[0]) < 0 + : "indices aren't sorted; previous [" + lastIndex + "], current [" + indices[0] + "]"; + } + return true; + } + /** * Collect the field capabilities for an index. */ void add( - String index, + String[] indices, boolean isMetadataField, boolean search, boolean agg, @@ -505,88 +519,96 @@ void add( TimeSeriesParams.MetricType metricType, Map meta ) { - assert indiceList.isEmpty() || indiceList.get(indiceList.size() - 1).name.compareTo(index) < 0 - : "indices aren't sorted; previous [" + indiceList.get(indiceList.size() - 1).name + "], current [" + index + "]"; + assert assertIndicesSorted(indices); + totalIndices += indices.length; if (search) { - searchableIndices++; + searchableIndices += indices.length; } if (agg) { - aggregatableIndices++; + aggregatableIndices += indices.length; } if (isDimension) { - dimensionIndices++; + dimensionIndices += indices.length; } this.isMetadataField |= isMetadataField; // If we have discrepancy in metric types or in some indices this field is not marked as a metric field - we will // treat is a non-metric field and report this discrepancy in metricConflictsIndices - if (indiceList.isEmpty()) { + if (indicesList.isEmpty()) { this.metricType = metricType; } else if (this.metricType != metricType) { hasConflictMetricType = true; this.metricType = null; } - IndexCaps indexCaps = new IndexCaps(index, search, agg, isDimension, metricType); - indiceList.add(indexCaps); + indicesList.add(new IndexCaps(indices, search, agg, isDimension, metricType)); for (Map.Entry entry : meta.entrySet()) { this.meta.computeIfAbsent(entry.getKey(), key -> new HashSet<>()).add(entry.getValue()); } } void getIndices(Collection indices) { - indiceList.forEach(cap -> indices.add(cap.name)); + for (IndexCaps indexCaps : indicesList) { + indices.addAll(Arrays.asList(indexCaps.indices)); + } } FieldCapabilities build(boolean withIndices) { final String[] indices; if (withIndices) { - indices = indiceList.stream().map(caps -> caps.name).toArray(String[]::new); + if (indicesList.size() == 1) { + indices = indicesList.get(0).indices; + } else { + indices = indicesList.stream().flatMap(caps -> Arrays.stream(caps.indices)).toArray(String[]::new); + } } else { indices = null; } // 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 boolean isSearchable = searchableIndices == totalIndices; final String[] nonSearchableIndices; if (isSearchable || searchableIndices == 0) { nonSearchableIndices = null; } else { - nonSearchableIndices = new String[indiceList.size() - searchableIndices]; + nonSearchableIndices = new String[totalIndices - searchableIndices]; int index = 0; - for (IndexCaps indexCaps : indiceList) { + for (IndexCaps indexCaps : indicesList) { if (indexCaps.isSearchable == false) { - nonSearchableIndices[index++] = indexCaps.name; + System.arraycopy(indexCaps.indices, 0, nonSearchableIndices, index, indexCaps.indices.length); + index += indexCaps.indices.length; } } } // 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 boolean isAggregatable = aggregatableIndices == totalIndices; final String[] nonAggregatableIndices; if (isAggregatable || aggregatableIndices == 0) { nonAggregatableIndices = null; } else { - nonAggregatableIndices = new String[indiceList.size() - aggregatableIndices]; + nonAggregatableIndices = new String[totalIndices - aggregatableIndices]; int index = 0; - for (IndexCaps indexCaps : indiceList) { + for (IndexCaps indexCaps : indicesList) { if (indexCaps.isAggregatable == false) { - nonAggregatableIndices[index++] = indexCaps.name; + System.arraycopy(indexCaps.indices, 0, nonAggregatableIndices, index, indexCaps.indices.length); + index += indexCaps.indices.length; } } } // 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 boolean isDimension = dimensionIndices == indicesList.size(); final String[] nonDimensionIndices; if (isDimension || dimensionIndices == 0) { nonDimensionIndices = null; } else { - nonDimensionIndices = new String[indiceList.size() - dimensionIndices]; + nonDimensionIndices = new String[totalIndices - dimensionIndices]; int index = 0; - for (IndexCaps indexCaps : indiceList) { + for (IndexCaps indexCaps : indicesList) { if (indexCaps.isDimension == false) { - nonDimensionIndices[index++] = indexCaps.name; + System.arraycopy(indexCaps.indices, 0, nonDimensionIndices, index, indexCaps.indices.length); + index += indexCaps.indices.length; } } } @@ -598,7 +620,7 @@ FieldCapabilities build(boolean withIndices) { // is present is probably the only sensible thing to do here metricConflictsIndices = Objects.requireNonNullElseGet( indices, - () -> indiceList.stream().map(caps -> caps.name).toArray(String[]::new) + () -> indicesList.stream().flatMap(caps -> Arrays.stream(caps.indices)).toArray(String[]::new) ); } else { metricConflictsIndices = null; @@ -627,7 +649,7 @@ FieldCapabilities build(boolean withIndices) { } private record IndexCaps( - String name, + String[] indices, boolean isSearchable, boolean isAggregatable, boolean isDimension, diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java index 3fed55cf14e70..3042a946b3151 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java @@ -8,6 +8,7 @@ package org.elasticsearch.action.fieldcaps; +import org.apache.lucene.util.ArrayUtil; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRunnable; @@ -35,6 +36,7 @@ import org.elasticsearch.transport.TransportService; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -234,44 +236,26 @@ private FieldCapabilitiesResponse merge( .sorted(Comparator.comparing(FieldCapabilitiesIndexResponse::getIndexName)) .toList(); final String[] indices = indexResponses.stream().map(FieldCapabilitiesIndexResponse::getIndexName).toArray(String[]::new); - - if (isFromSingleMapping(indexResponses)) { - FieldCapabilitiesIndexResponse firstResponse = indexResponses.get(0); - final Map fields = ResponseRewriter.rewriteOldResponses( - firstResponse.getOriginVersion(), - firstResponse.get(), - request.filters(), - request.types(), - metadataFieldPred - ); - final Map> responseMap = fields.entrySet() - .stream() - .collect(Collectors.toMap(Map.Entry::getKey, e -> { - final IndexFieldCapabilities cap = e.getValue(); - return Map.of( - cap.getType(), - new FieldCapabilities( - cap.getName(), - cap.getType(), - cap.isMetadatafield(), - cap.isSearchable(), - cap.isAggregatable(), - cap.isDimension(), - cap.getMetricType(), - null, - null, - null, - null, - null, - cap.meta().entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, v -> Set.of(v.getValue()))) - ) - ); - })); - return new FieldCapabilitiesResponse(indices, responseMap, failures); - } final Map> responseMapBuilder = new HashMap<>(); - for (FieldCapabilitiesIndexResponse response : indexResponses) { - innerMerge(responseMapBuilder, request, response); + int lastPendingIndex = 0; + for (int i = 1; i < indexResponses.size(); i++) { + if (indexResponses.get(lastPendingIndex).get() != indexResponses.get(i).get()) { + innerMerge( + ArrayUtil.copyOfSubArray(indices, lastPendingIndex, i), + responseMapBuilder, + request, + indexResponses.get(lastPendingIndex) + ); + lastPendingIndex = i; + } + } + if (lastPendingIndex < indexResponses.size()) { + innerMerge( + ArrayUtil.copyOfSubArray(indices, lastPendingIndex, indexResponses.size()), + responseMapBuilder, + request, + indexResponses.get(lastPendingIndex) + ); } final Map> responseMap = new HashMap<>(); for (Map.Entry> entry : responseMapBuilder.entrySet()) { @@ -294,31 +278,18 @@ private static void addUnmappedFields(String[] indices, String field, Map 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()); - } + final String[] unmappedIndices = Arrays.stream(indices) + .filter(index -> mappedIndices.contains(index) == false) + .toArray(String[]::new); + if (unmappedIndices.length > 0) { + unmapped.add(unmappedIndices, false, false, false, false, null, Collections.emptyMap()); } typeMap.put("unmapped", unmapped); } } - private boolean isFromSingleMapping(List indexResponses) { - String sharedMappingHash = null; - for (FieldCapabilitiesIndexResponse resp : indexResponses) { - if (resp.getIndexMappingHash() == null) { - return false; - } - if (sharedMappingHash == null) { - sharedMappingHash = resp.getIndexMappingHash(); - } else if (sharedMappingHash.equals(resp.getIndexMappingHash()) == false) { - return false; - } - } - return sharedMappingHash != null; - } - private void innerMerge( + String[] groupedIndices, Map> responseMapBuilder, FieldCapabilitiesRequest request, FieldCapabilitiesIndexResponse response @@ -338,7 +309,7 @@ private void innerMerge( key -> new FieldCapabilities.Builder(field, key) ); builder.add( - response.getIndexName(), + groupedIndices, fieldCap.isMetadatafield(), fieldCap.isSearchable(), fieldCap.isAggregatable(), diff --git a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java index e9ad435b35154..dab6b5d57800e 100644 --- a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java +++ b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java @@ -8,18 +8,18 @@ package org.elasticsearch.action.fieldcaps; +import org.apache.lucene.util.ArrayUtil; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.util.iterable.Iterables; -import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.index.mapper.TimeSeriesParams; import org.elasticsearch.test.AbstractSerializingTestCase; import org.elasticsearch.xcontent.XContentParser; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -49,9 +49,9 @@ protected Writeable.Reader instanceReader() { public void testBuilder() { FieldCapabilities.Builder builder = new FieldCapabilities.Builder("field", "type"); - builder.add("index1", false, true, false, false, null, Collections.emptyMap()); - builder.add("index2", false, true, false, false, null, Collections.emptyMap()); - builder.add("index3", false, true, false, false, null, Collections.emptyMap()); + builder.add(new String[] { "index1" }, false, true, false, false, null, Collections.emptyMap()); + builder.add(new String[] { "index2" }, false, true, false, false, null, Collections.emptyMap()); + builder.add(new String[] { "index3" }, false, true, false, false, null, Collections.emptyMap()); { FieldCapabilities cap1 = builder.build(false); @@ -79,9 +79,9 @@ public void testBuilder() { } builder = new FieldCapabilities.Builder("field", "type"); - builder.add("index1", false, false, true, true, null, Collections.emptyMap()); - builder.add("index2", false, true, false, false, TimeSeriesParams.MetricType.counter, Collections.emptyMap()); - builder.add("index3", false, false, false, false, null, Collections.emptyMap()); + builder.add(new String[] { "index1" }, false, false, true, true, null, Collections.emptyMap()); + builder.add(new String[] { "index2" }, false, true, false, false, TimeSeriesParams.MetricType.counter, Collections.emptyMap()); + builder.add(new String[] { "index3" }, false, false, false, false, null, Collections.emptyMap()); { FieldCapabilities cap1 = builder.build(false); assertThat(cap1.isSearchable(), equalTo(false)); @@ -108,9 +108,9 @@ public void testBuilder() { } builder = new FieldCapabilities.Builder("field", "type"); - builder.add("index1", false, true, true, true, TimeSeriesParams.MetricType.counter, Collections.emptyMap()); - builder.add("index2", false, true, true, true, TimeSeriesParams.MetricType.counter, Map.of("foo", "bar")); - builder.add("index3", false, true, true, true, TimeSeriesParams.MetricType.counter, Map.of("foo", "quux")); + builder.add(new String[] { "index1" }, false, true, true, true, TimeSeriesParams.MetricType.counter, Collections.emptyMap()); + builder.add(new String[] { "index2" }, false, true, true, true, TimeSeriesParams.MetricType.counter, Map.of("foo", "bar")); + builder.add(new String[] { "index3" }, false, true, true, true, TimeSeriesParams.MetricType.counter, Map.of("foo", "quux")); { FieldCapabilities cap1 = builder.build(false); assertThat(cap1.isSearchable(), equalTo(true)); @@ -137,9 +137,9 @@ public void testBuilder() { } builder = new FieldCapabilities.Builder("field", "type"); - builder.add("index1", false, true, true, true, TimeSeriesParams.MetricType.counter, Collections.emptyMap()); - builder.add("index2", false, true, true, true, TimeSeriesParams.MetricType.gauge, Map.of("foo", "bar")); - builder.add("index3", false, true, true, true, TimeSeriesParams.MetricType.counter, Map.of("foo", "quux")); + builder.add(new String[] { "index1" }, false, true, true, true, TimeSeriesParams.MetricType.counter, Collections.emptyMap()); + builder.add(new String[] { "index2" }, false, true, true, true, TimeSeriesParams.MetricType.gauge, Map.of("foo", "bar")); + builder.add(new String[] { "index3" }, false, true, true, true, TimeSeriesParams.MetricType.counter, Map.of("foo", "quux")); { FieldCapabilities cap1 = builder.build(false); assertThat(cap1.isSearchable(), equalTo(true)); @@ -167,66 +167,73 @@ public void testBuilder() { } public void testRandomBuilder() { - List indices = IntStream.range(0, randomIntBetween(1, 50)) + String[] indices = IntStream.range(0, randomIntBetween(1, 50)) .mapToObj(n -> String.format(Locale.ROOT, "index_%2d", n)) - .toList(); - Set searchableIndices = new HashSet<>(randomSubsetOf(indices)); - Set aggregatableIndices = new HashSet<>(randomSubsetOf(indices)); - Set dimensionIndices = new HashSet<>(randomSubsetOf(indices)); + .toArray(String[]::new); + + List nonSearchableIndices = new ArrayList<>(); + List nonAggregatableIndices = new ArrayList<>(); + List nonDimensionIndices = new ArrayList<>(); + FieldCapabilities.Builder builder = new FieldCapabilities.Builder("field", "type"); - for (String index : indices) { - builder.add( - index, - randomBoolean(), - searchableIndices.contains(index), - aggregatableIndices.contains(index), - dimensionIndices.contains(index), - null, - Map.of() - ); + for (int i = 0; i < indices.length;) { + int bulkSize = randomIntBetween(1, indices.length - i); + String[] groupIndices = ArrayUtil.copyOfSubArray(indices, i, i + bulkSize); + final boolean searchable = randomBoolean(); + if (searchable == false) { + nonSearchableIndices.addAll(Arrays.asList(groupIndices)); + } + final boolean aggregatable = randomBoolean(); + if (aggregatable == false) { + nonAggregatableIndices.addAll(Arrays.asList(groupIndices)); + } + final boolean hasDimension = randomBoolean(); + if (hasDimension == false) { + nonDimensionIndices.addAll(Arrays.asList(groupIndices)); + } + builder.add(groupIndices, false, searchable, aggregatable, hasDimension, null, Map.of()); + i += bulkSize; + } + boolean withIndices = randomBoolean(); + FieldCapabilities fieldCaps = builder.build(withIndices); + if (withIndices) { + assertThat(fieldCaps.indices(), equalTo(indices)); } - FieldCapabilities fieldCaps = builder.build(randomBoolean()); // search - if (searchableIndices.isEmpty()) { - assertFalse(fieldCaps.isSearchable()); - assertNull(fieldCaps.nonSearchableIndices()); - } else if (searchableIndices.size() == indices.size()) { + if (nonSearchableIndices.isEmpty()) { assertTrue(fieldCaps.isSearchable()); assertNull(fieldCaps.nonSearchableIndices()); } else { assertFalse(fieldCaps.isSearchable()); - assertThat( - Sets.newHashSet(fieldCaps.nonSearchableIndices()), - equalTo(Sets.difference(Sets.newHashSet(indices), searchableIndices)) - ); + if (nonSearchableIndices.size() == indices.length) { + assertThat(fieldCaps.nonSearchableIndices(), equalTo(null)); + } else { + assertThat(fieldCaps.nonSearchableIndices(), equalTo(nonSearchableIndices.toArray(String[]::new))); + } } // aggregate - if (aggregatableIndices.isEmpty()) { - assertFalse(fieldCaps.isAggregatable()); - assertNull(fieldCaps.nonAggregatableIndices()); - } else if (aggregatableIndices.size() == indices.size()) { + if (nonAggregatableIndices.isEmpty()) { assertTrue(fieldCaps.isAggregatable()); assertNull(fieldCaps.nonAggregatableIndices()); } else { assertFalse(fieldCaps.isAggregatable()); - assertThat( - Sets.newHashSet(fieldCaps.nonAggregatableIndices()), - equalTo(Sets.difference(Sets.newHashSet(indices), aggregatableIndices)) - ); + if (nonAggregatableIndices.size() == indices.length) { + assertThat(fieldCaps.nonAggregatableIndices(), equalTo(null)); + } else { + assertThat(fieldCaps.nonAggregatableIndices(), equalTo(nonAggregatableIndices.toArray(String[]::new))); + } } // dimension - if (dimensionIndices.isEmpty()) { - assertFalse(fieldCaps.isDimension()); - assertNull(fieldCaps.nonDimensionIndices()); - } else if (dimensionIndices.size() == indices.size()) { + if (nonDimensionIndices.isEmpty()) { assertTrue(fieldCaps.isDimension()); assertNull(fieldCaps.nonDimensionIndices()); } else { assertFalse(fieldCaps.isDimension()); - assertThat( - Sets.newHashSet(fieldCaps.nonDimensionIndices()), - equalTo(Sets.difference(Sets.newHashSet(indices), dimensionIndices)) - ); + if (nonDimensionIndices.size() == indices.length) { + assertThat(fieldCaps.nonDimensionIndices(), equalTo(null)); + } else { + assertThat(fieldCaps.nonDimensionIndices(), equalTo(nonDimensionIndices.toArray(String[]::new))); + } } } @@ -237,7 +244,7 @@ public void testBuilderSingleMetricType() { TimeSeriesParams.MetricType metric = randomBoolean() ? null : randomFrom(TimeSeriesParams.MetricType.values()); FieldCapabilities.Builder builder = new FieldCapabilities.Builder("field", "type"); for (String index : indices) { - builder.add(index, randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), metric, Map.of()); + builder.add(new String[] { index }, randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), metric, Map.of()); } FieldCapabilities fieldCaps = builder.build(randomBoolean()); assertThat(fieldCaps.getMetricType(), equalTo(metric)); @@ -256,7 +263,15 @@ public void testBuilderMixedMetricType() { } FieldCapabilities.Builder builder = new FieldCapabilities.Builder("field", "type"); for (String index : indices) { - builder.add(index, randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), metricTypes.get(index), Map.of()); + builder.add( + new String[] { index }, + randomBoolean(), + randomBoolean(), + randomBoolean(), + randomBoolean(), + metricTypes.get(index), + Map.of() + ); } FieldCapabilities fieldCaps = builder.build(randomBoolean()); if (metricTypes.isEmpty()) { @@ -276,7 +291,7 @@ public void testOutOfOrderIndices() { int numIndex = randomIntBetween(1, 5); for (int i = 1; i <= numIndex; i++) { builder.add( - "index-" + i, + new String[] { "index-" + i }, randomBoolean(), randomBoolean(), randomBoolean(), @@ -288,7 +303,7 @@ public void testOutOfOrderIndices() { final String outOfOrderIndex = randomBoolean() ? "abc" : "index-" + randomIntBetween(1, numIndex); AssertionError error = expectThrows(AssertionError.class, () -> { builder.add( - outOfOrderIndex, + new String[] { outOfOrderIndex }, randomBoolean(), randomBoolean(), randomBoolean(), From 99f94f53dbfb558685b6065fbdbe298c49c7076d Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 7 May 2022 14:54:54 -0400 Subject: [PATCH 4/9] change log --- docs/changelog/86323.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/86323.yaml b/docs/changelog/86323.yaml index 39c52a3d5e066..4abf25204c371 100644 --- a/docs/changelog/86323.yaml +++ b/docs/changelog/86323.yaml @@ -1,5 +1,5 @@ pr: 86323 -summary: Speed up merging field-caps responses from single mapping +summary: Bulk merge field-caps responses using mapping hash area: Search type: enhancement issues: [] From 0e6082e4cb096cdbc35854e098049766f9d15f40 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 7 May 2022 20:43:06 -0400 Subject: [PATCH 5/9] simplify --- .../elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java | 6 +----- .../action/fieldcaps/TransportFieldCapabilitiesAction.java | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java index bdbba4df1bc20..732605342a919 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java @@ -626,11 +626,7 @@ public void testManyIndicesWithSameMapping() { FieldCapabilitiesResponse resp = client().execute(FieldCapabilitiesAction.INSTANCE, request).actionGet(); verifyResponse.accept(resp); assertThat(resp.getField("extra_field"), hasKey("integer")); - assertThat( - "extra_indices " + Arrays.toString(indicesWithExtraField), - resp.getField("extra_field").get("integer").indices(), - nullValue() - ); + assertThat(resp.getField("extra_field").get("integer").indices(), nullValue()); assertTrue(resp.getField("extra_field").get("integer").isSearchable()); assertTrue(resp.getField("extra_field").get("integer").isAggregatable()); } diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java index 3042a946b3151..a2b318ef754c2 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java @@ -277,14 +277,14 @@ private static void addUnmappedFields(String[] indices, String field, Map mappedIndices = new HashSet<>(); typeMap.values().forEach(t -> t.getIndices(mappedIndices)); if (mappedIndices.size() != indices.length) { - final FieldCapabilities.Builder unmapped = new FieldCapabilities.Builder(field, "unmapped"); final String[] unmappedIndices = Arrays.stream(indices) .filter(index -> mappedIndices.contains(index) == false) .toArray(String[]::new); if (unmappedIndices.length > 0) { + final FieldCapabilities.Builder unmapped = new FieldCapabilities.Builder(field, "unmapped"); unmapped.add(unmappedIndices, false, false, false, false, null, Collections.emptyMap()); + typeMap.put("unmapped", unmapped); } - typeMap.put("unmapped", unmapped); } } From 87ec79abce980590a4e15c2c7fdcecf2380d065f Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 7 May 2022 22:27:46 -0400 Subject: [PATCH 6/9] Fix test --- .../elasticsearch/action/fieldcaps/FieldCapabilities.java | 2 +- .../action/fieldcaps/FieldCapabilitiesTests.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java index ccce8f88ce222..8ba0ed6ca38cb 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java @@ -598,7 +598,7 @@ FieldCapabilities build(boolean withIndices) { } // Collect all indices that have dimension == false if this field is marked as a dimension in at least one index - final boolean isDimension = dimensionIndices == indicesList.size(); + final boolean isDimension = dimensionIndices == totalIndices; final String[] nonDimensionIndices; if (isDimension || dimensionIndices == 0) { nonDimensionIndices = null; diff --git a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java index dab6b5d57800e..1e02ab8fe6b4f 100644 --- a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java +++ b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java @@ -187,11 +187,11 @@ public void testRandomBuilder() { if (aggregatable == false) { nonAggregatableIndices.addAll(Arrays.asList(groupIndices)); } - final boolean hasDimension = randomBoolean(); - if (hasDimension == false) { + final boolean isDimension = randomBoolean(); + if (isDimension == false) { nonDimensionIndices.addAll(Arrays.asList(groupIndices)); } - builder.add(groupIndices, false, searchable, aggregatable, hasDimension, null, Map.of()); + builder.add(groupIndices, false, searchable, aggregatable, isDimension, null, Map.of()); i += bulkSize; } boolean withIndices = randomBoolean(); From 5b836a3142d1a995d81b11788354a6270be61839 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Fri, 14 Oct 2022 10:13:49 -0700 Subject: [PATCH 7/9] Address feedback --- .../action/fieldcaps/FieldCapabilities.java | 55 ++++++------------- .../TransportFieldCapabilitiesAction.java | 21 +++---- .../fieldcaps/FieldCapabilitiesTests.java | 1 + 3 files changed, 26 insertions(+), 51 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java index 8ba0ed6ca38cb..b6c0bd99e8126 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java @@ -34,6 +34,7 @@ import java.util.Objects; import java.util.Set; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; import static org.elasticsearch.index.mapper.TimeSeriesParams.TIME_SERIES_DIMENSION_PARAM; @@ -551,17 +552,21 @@ void getIndices(Collection indices) { } } - FieldCapabilities build(boolean withIndices) { - final String[] indices; - if (withIndices) { - if (indicesList.size() == 1) { - indices = indicesList.get(0).indices; - } else { - indices = indicesList.stream().flatMap(caps -> Arrays.stream(caps.indices)).toArray(String[]::new); + private String[] filterIndices(int length, Predicate pred) { + int index = 0; + final String[] dst = new String[length]; + for (IndexCaps indexCaps : indicesList) { + if (pred.test(indexCaps)) { + System.arraycopy(indexCaps.indices, 0, dst, index, indexCaps.indices.length); + index += indexCaps.indices.length; } - } else { - indices = null; } + assert index == length : index + "!=" + length; + return dst; + } + + FieldCapabilities build(boolean withIndices) { + final String[] indices = withIndices ? filterIndices(totalIndices, ic -> true) : null; // Iff this field is searchable in some indices AND non-searchable in others // we record the list of non-searchable indices @@ -570,14 +575,7 @@ FieldCapabilities build(boolean withIndices) { if (isSearchable || searchableIndices == 0) { nonSearchableIndices = null; } else { - nonSearchableIndices = new String[totalIndices - searchableIndices]; - int index = 0; - for (IndexCaps indexCaps : indicesList) { - if (indexCaps.isSearchable == false) { - System.arraycopy(indexCaps.indices, 0, nonSearchableIndices, index, indexCaps.indices.length); - index += indexCaps.indices.length; - } - } + nonSearchableIndices = filterIndices(totalIndices - searchableIndices, ic -> ic.isSearchable == false); } // Iff this field is aggregatable in some indices AND non-aggregatable in others @@ -587,14 +585,7 @@ FieldCapabilities build(boolean withIndices) { if (isAggregatable || aggregatableIndices == 0) { nonAggregatableIndices = null; } else { - nonAggregatableIndices = new String[totalIndices - aggregatableIndices]; - int index = 0; - for (IndexCaps indexCaps : indicesList) { - if (indexCaps.isAggregatable == false) { - System.arraycopy(indexCaps.indices, 0, nonAggregatableIndices, index, indexCaps.indices.length); - index += indexCaps.indices.length; - } - } + nonAggregatableIndices = filterIndices(totalIndices - aggregatableIndices, ic -> ic.isAggregatable == false); } // Collect all indices that have dimension == false if this field is marked as a dimension in at least one index @@ -603,14 +594,7 @@ FieldCapabilities build(boolean withIndices) { if (isDimension || dimensionIndices == 0) { nonDimensionIndices = null; } else { - nonDimensionIndices = new String[totalIndices - dimensionIndices]; - int index = 0; - for (IndexCaps indexCaps : indicesList) { - if (indexCaps.isDimension == false) { - System.arraycopy(indexCaps.indices, 0, nonDimensionIndices, index, indexCaps.indices.length); - index += indexCaps.indices.length; - } - } + nonDimensionIndices = filterIndices(totalIndices - dimensionIndices, ic -> ic.isDimension == false); } final String[] metricConflictsIndices; @@ -618,10 +602,7 @@ FieldCapabilities build(boolean withIndices) { // Collect all indices that have this field. If it is marked differently in different indices, we cannot really // make a decisions which index is "right" and which index is "wrong" so collecting all indices where this field // is present is probably the only sensible thing to do here - metricConflictsIndices = Objects.requireNonNullElseGet( - indices, - () -> indicesList.stream().flatMap(caps -> Arrays.stream(caps.indices)).toArray(String[]::new) - ); + metricConflictsIndices = Objects.requireNonNullElseGet(indices, () -> filterIndices(totalIndices, ic -> true)); } else { metricConflictsIndices = null; } diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java index a2b318ef754c2..bbc07f91da814 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java @@ -231,32 +231,25 @@ private FieldCapabilitiesResponse merge( FieldCapabilitiesRequest request, List failures ) { - final List indexResponses = indexResponsesMap.values() + final FieldCapabilitiesIndexResponse[] indexResponses = indexResponsesMap.values() .stream() .sorted(Comparator.comparing(FieldCapabilitiesIndexResponse::getIndexName)) - .toList(); - final String[] indices = indexResponses.stream().map(FieldCapabilitiesIndexResponse::getIndexName).toArray(String[]::new); + .toArray(FieldCapabilitiesIndexResponse[]::new); + final String[] indices = Arrays.stream(indexResponses).map(FieldCapabilitiesIndexResponse::getIndexName).toArray(String[]::new); final Map> responseMapBuilder = new HashMap<>(); int lastPendingIndex = 0; - for (int i = 1; i < indexResponses.size(); i++) { - if (indexResponses.get(lastPendingIndex).get() != indexResponses.get(i).get()) { + for (int i = 1; i <= indexResponses.length; i++) { + // use object equality to avoid expensive string comparison of mapping hashes. + if (i == indexResponses.length || indexResponses[lastPendingIndex].get() != indexResponses[i].get()) { innerMerge( ArrayUtil.copyOfSubArray(indices, lastPendingIndex, i), responseMapBuilder, request, - indexResponses.get(lastPendingIndex) + indexResponses[lastPendingIndex] ); lastPendingIndex = i; } } - if (lastPendingIndex < indexResponses.size()) { - innerMerge( - ArrayUtil.copyOfSubArray(indices, lastPendingIndex, indexResponses.size()), - responseMapBuilder, - request, - indexResponses.get(lastPendingIndex) - ); - } final Map> responseMap = new HashMap<>(); for (Map.Entry> entry : responseMapBuilder.entrySet()) { final Map typeMapBuilder = entry.getValue(); diff --git a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java index d9690c748843d..9c354451c4090 100644 --- a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java +++ b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.stream.IntStream; From 058f1ab0f912ce3bbc6079d25df3bfdbc8da8d4c Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 15 Oct 2022 14:09:17 -0700 Subject: [PATCH 8/9] avoid copying sub array --- .../TransportFieldCapabilitiesAction.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java index bbc07f91da814..4e5e26407210e 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java @@ -241,12 +241,13 @@ private FieldCapabilitiesResponse merge( for (int i = 1; i <= indexResponses.length; i++) { // use object equality to avoid expensive string comparison of mapping hashes. if (i == indexResponses.length || indexResponses[lastPendingIndex].get() != indexResponses[i].get()) { - innerMerge( - ArrayUtil.copyOfSubArray(indices, lastPendingIndex, i), - responseMapBuilder, - request, - indexResponses[lastPendingIndex] - ); + final String[] subIndices; + if (lastPendingIndex == 0 && i == indexResponses.length) { + subIndices = indices; + } else { + subIndices = ArrayUtil.copyOfSubArray(indices, lastPendingIndex, i); + } + innerMerge(subIndices, responseMapBuilder, request, indexResponses[lastPendingIndex]); lastPendingIndex = i; } } @@ -282,7 +283,7 @@ private static void addUnmappedFields(String[] indices, String field, Map> responseMapBuilder, FieldCapabilitiesRequest request, FieldCapabilitiesIndexResponse response @@ -302,7 +303,7 @@ private void innerMerge( key -> new FieldCapabilities.Builder(field, key) ); builder.add( - groupedIndices, + indices, fieldCap.isMetadatafield(), fieldCap.isSearchable(), fieldCap.isAggregatable(), From 51eb9999dacf75f3ee60f1628231cfbf0bd7c46d Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Mon, 31 Oct 2022 17:13:00 -0700 Subject: [PATCH 9/9] avoid using object equality --- .../TransportFieldCapabilitiesAction.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java index 4e5e26407210e..f2ffee2507e4f 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java @@ -112,12 +112,12 @@ protected void doExecute(Task task, FieldCapabilitiesRequest request, final Acti final Map indexResponses = Collections.synchronizedMap(new HashMap<>()); // This map is used to share the index response for indices which have the same index mapping hash to reduce the memory usage. - final Map> indexMappingHashToResponses = Collections.synchronizedMap(new HashMap<>()); + final Map indexMappingHashToResponses = Collections.synchronizedMap(new HashMap<>()); final Consumer handleIndexResponse = resp -> { if (resp.canMatch() && resp.getIndexMappingHash() != null) { - Map curr = indexMappingHashToResponses.putIfAbsent(resp.getIndexMappingHash(), resp.get()); + FieldCapabilitiesIndexResponse curr = indexMappingHashToResponses.putIfAbsent(resp.getIndexMappingHash(), resp); if (curr != null) { - resp = new FieldCapabilitiesIndexResponse(resp.getIndexName(), resp.getIndexMappingHash(), curr, true); + resp = new FieldCapabilitiesIndexResponse(resp.getIndexName(), curr.getIndexMappingHash(), curr.get(), true); } } indexResponses.putIfAbsent(resp.getIndexName(), resp); @@ -226,6 +226,12 @@ private static FieldCapabilitiesRequest prepareRemoteRequest( return remoteRequest; } + private static boolean hasSameMappingHash(FieldCapabilitiesIndexResponse r1, FieldCapabilitiesIndexResponse r2) { + return r1.getIndexMappingHash() != null + && r2.getIndexMappingHash() != null + && r1.getIndexMappingHash().equals(r2.getIndexMappingHash()); + } + private FieldCapabilitiesResponse merge( Map indexResponsesMap, FieldCapabilitiesRequest request, @@ -239,8 +245,7 @@ private FieldCapabilitiesResponse merge( final Map> responseMapBuilder = new HashMap<>(); int lastPendingIndex = 0; for (int i = 1; i <= indexResponses.length; i++) { - // use object equality to avoid expensive string comparison of mapping hashes. - if (i == indexResponses.length || indexResponses[lastPendingIndex].get() != indexResponses[i].get()) { + if (i == indexResponses.length || hasSameMappingHash(indexResponses[lastPendingIndex], indexResponses[i]) == false) { final String[] subIndices; if (lastPendingIndex == 0 && i == indexResponses.length) { subIndices = indices;