From a8f5285ba533d16aba8e729b53f41cd7dac75d85 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 23 Apr 2025 15:19:00 +0200 Subject: [PATCH 1/2] Some obvious speedups to parsing alias filters It's in the title, saving some allocations and weird indirection in obvious spots. --- .../metadata/IndexNameExpressionResolver.java | 7 +- .../elasticsearch/indices/IndicesService.java | 64 ++++++++----------- .../search/internal/ShardSearchRequest.java | 30 +++++---- 3 files changed, 50 insertions(+), 51 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 19189a81a0187..b31b2b510eab4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -374,11 +374,14 @@ protected static Collection resolveExpressionsToResources(Co return List.of(); } } else { - Predicate isMatchAll = (((Predicate) Metadata.ALL::equals)).or(Regex::isMatchAllPattern); if (expressions == null || expressions.length == 0 || expressions.length == 1 - && (SelectorResolver.selectorsValidatedAndMatchesPredicate(expressions[0], context, isMatchAll))) { + && (SelectorResolver.selectorsValidatedAndMatchesPredicate( + expressions[0], + context, + s -> Metadata.ALL.equals(s) || Regex.isMatchAllPattern(s) + ))) { IndexComponentSelector selector; if (expressions != null && expressions.length == 1) { selector = SelectorResolver.parseMatchAllToSelector(context, expressions[0]); diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index c3adcf1160e66..ef58b1e05e9a8 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1023,6 +1023,12 @@ public IndicesQueryCache getIndicesQueryCache() { return indicesQueryCache; } + private QueryBuilder parseFilter(BytesReference bytes) throws IOException { + try (XContentParser parser = XContentHelper.createParser(parserConfig, bytes)) { + return parseTopLevelQuery(parser); + } + } + static class OldShardsStats implements IndexEventListener { final SearchStats searchStats = new SearchStats(); @@ -1743,13 +1749,6 @@ interface IndexDeletionAllowedPredicate { public AliasFilter buildAliasFilter(ProjectState project, String index, Set resolvedExpressions) { /* Being static, parseAliasFilter doesn't have access to whatever guts it needs to parse a query. Instead of passing in a bunch * of dependencies we pass in a function that can perform the parsing. */ - CheckedFunction filterParser = bytes -> { - try ( - XContentParser parser = XContentHelper.createParserNotCompressed(parserConfig, bytes, XContentHelper.xContentType(bytes)) - ) { - return parseTopLevelQuery(parser); - } - }; final ProjectMetadata metadata = project.metadata(); String[] aliases = indexNameExpressionResolver.filteringAliases(metadata, index, resolvedExpressions); @@ -1759,43 +1758,36 @@ public AliasFilter buildAliasFilter(ProjectState project, String index, Set filters = Arrays.stream(aliases) - .map(name -> metadata.dataStreamAliases().get(name)) - .filter(dataStreamAlias -> dataStreamAlias.getFilter(dataStreamName) != null) - .map(dataStreamAlias -> { - try { - return filterParser.apply(dataStreamAlias.getFilter(dataStreamName).uncompressed()); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }) - .toList(); + List filters = Arrays.stream(aliases).map(key -> { + var f = dsAliases.get(key).getFilter(dataStreamName); + if (f == null) { + return null; + } + try { + return parseFilter(f.compressedReference()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }).filter(Objects::nonNull).toList(); if (filters.isEmpty()) { - return AliasFilter.of(null, aliases); + filter = null; + } else if (filters.size() == 1) { + filter = filters.getFirst(); } else { - if (filters.size() == 1) { - return AliasFilter.of(filters.get(0), aliases); - } else { - BoolQueryBuilder bool = new BoolQueryBuilder(); - for (QueryBuilder filter : filters) { - bool.should(filter); - } - return AliasFilter.of(bool, aliases); + BoolQueryBuilder bool = new BoolQueryBuilder(); + for (QueryBuilder f : filters) { + bool.should(f); } + filter = bool; } } else { - IndexMetadata indexMetadata = metadata.index(index); - return AliasFilter.of(ShardSearchRequest.parseAliasFilter(filterParser, indexMetadata, aliases), aliases); + filter = ShardSearchRequest.parseAliasFilter(this::parseFilter, metadata.index(index), aliases); } - } - - /** - * Returns a new {@link QueryRewriteContext} with the given {@code now} provider - */ - public QueryRewriteContext getRewriteContext(LongSupplier nowInMillis, ResolvedIndices resolvedIndices, PointInTimeBuilder pit) { - return getRewriteContext(nowInMillis, resolvedIndices, pit, false); + return AliasFilter.of(filter, aliases); } /** diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java index be504b77713c4..10d2fb0e23b3b 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java @@ -58,7 +58,6 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.function.Function; import static java.util.Collections.emptyMap; import static org.elasticsearch.search.internal.SearchContext.TRACK_TOTAL_HITS_DISABLED; @@ -630,23 +629,13 @@ public static QueryBuilder parseAliasFilter( } Index index = metadata.getIndex(); Map aliases = metadata.getAliases(); - Function parserFunction = (alias) -> { - if (alias.filter() == null) { - return null; - } - try { - return filterParser.apply(alias.filter().uncompressed()); - } catch (IOException ex) { - throw new AliasFilterParsingException(index, alias.getAlias(), "Invalid alias filter", ex); - } - }; if (aliasNames.length == 1) { AliasMetadata alias = aliases.get(aliasNames[0]); if (alias == null) { // This shouldn't happen unless alias disappeared after filteringAliases was called. throw new InvalidAliasNameException(index, aliasNames[0], "Unknown alias name was passed to alias Filter"); } - return parserFunction.apply(alias); + return parseAliasFilter(filterParser, alias, index); } else { // we need to bench here a bit, to see maybe it makes sense to use OrFilter BoolQueryBuilder combined = new BoolQueryBuilder(); @@ -656,7 +645,7 @@ public static QueryBuilder parseAliasFilter( // This shouldn't happen unless alias disappeared after filteringAliases was called. throw new InvalidAliasNameException(index, aliasNames[0], "Unknown alias name was passed to alias Filter"); } - QueryBuilder parsedFilter = parserFunction.apply(alias); + QueryBuilder parsedFilter = parseAliasFilter(filterParser, alias, index); if (parsedFilter != null) { combined.should(parsedFilter); } else { @@ -668,6 +657,21 @@ public static QueryBuilder parseAliasFilter( } } + private static QueryBuilder parseAliasFilter( + CheckedFunction filterParser, + AliasMetadata alias, + Index index + ) { + if (alias.filter() == null) { + return null; + } + try { + return filterParser.apply(alias.filter().compressedReference()); + } catch (IOException ex) { + throw new AliasFilterParsingException(index, alias.getAlias(), "Invalid alias filter", ex); + } + } + public final Map getRuntimeMappings() { return source == null ? emptyMap() : source.runtimeMappings(); } From 4239e1a6cec7aa06f6709796113391040ed56d14 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 23 Apr 2025 22:54:31 +0200 Subject: [PATCH 2/2] fix test --- .../elasticsearch/search/internal/ShardSearchRequestTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchRequestTests.java b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchRequestTests.java index 5ff6e04648c87..302ce085ea317 100644 --- a/server/src/test/java/org/elasticsearch/search/internal/ShardSearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/search/internal/ShardSearchRequestTests.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.compress.CompressorFactory; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; @@ -210,7 +211,7 @@ private IndexMetadata add(IndexMetadata indexMetadata, String alias, @Nullable C public QueryBuilder aliasFilter(IndexMetadata indexMetadata, String... aliasNames) { return ShardSearchRequest.parseAliasFilter(bytes -> { try ( - InputStream inputStream = bytes.streamInput(); + InputStream inputStream = CompressorFactory.COMPRESSOR.uncompress(bytes).streamInput(); XContentParser parser = XContentFactory.xContentType(inputStream) .xContent() .createParser(xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, inputStream)