Skip to content

Commit

Permalink
Re-add ResolvedExpression wrapper (elastic#118174)
Browse files Browse the repository at this point in the history
This PR reapplies elastic#114592 along with an update to remove the performance regression 
introduced with the original change.
  • Loading branch information
jbaiera authored Dec 10, 2024
1 parent 78620c6 commit 34b7e60
Show file tree
Hide file tree
Showing 13 changed files with 305 additions and 164 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.cluster.block.ClusterBlockException;
import org.elasticsearch.cluster.block.ClusterBlockLevel;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.routing.GroupShardsIterator;
import org.elasticsearch.cluster.routing.ShardIterator;
Expand Down Expand Up @@ -84,7 +85,7 @@ protected void masterOperation(
String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(clusterState, request);
Map<String, Set<String>> routingMap = indexNameExpressionResolver.resolveSearchRouting(state, request.routing(), request.indices());
Map<String, AliasFilter> indicesAndFilters = new HashMap<>();
Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
Set<ResolvedExpression> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
for (String index : concreteIndices) {
final AliasFilter aliasFilter = indicesService.buildAliasFilter(clusterState, index, indicesAndAliases);
final String[] aliases = indexNameExpressionResolver.indexAliases(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
Expand Down Expand Up @@ -565,8 +566,8 @@ static void resolveIndices(
if (names.length == 1 && (Metadata.ALL.equals(names[0]) || Regex.isMatchAllPattern(names[0]))) {
names = new String[] { "**" };
}
Set<String> resolvedIndexAbstractions = resolver.resolveExpressions(clusterState, indicesOptions, true, names);
for (String s : resolvedIndexAbstractions) {
Set<ResolvedExpression> resolvedIndexAbstractions = resolver.resolveExpressions(clusterState, indicesOptions, true, names);
for (ResolvedExpression s : resolvedIndexAbstractions) {
enrichIndexAbstraction(clusterState, s, indices, aliases, dataStreams);
}
indices.sort(Comparator.comparing(ResolvedIndexAbstraction::getName));
Expand Down Expand Up @@ -597,12 +598,12 @@ private static void mergeResults(

private static void enrichIndexAbstraction(
ClusterState clusterState,
String indexAbstraction,
ResolvedExpression indexAbstraction,
List<ResolvedIndex> indices,
List<ResolvedAlias> aliases,
List<ResolvedDataStream> dataStreams
) {
IndexAbstraction ia = clusterState.metadata().getIndicesLookup().get(indexAbstraction);
IndexAbstraction ia = clusterState.metadata().getIndicesLookup().get(indexAbstraction.resource());
if (ia != null) {
switch (ia.getType()) {
case CONCRETE_INDEX -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.elasticsearch.cluster.block.ClusterBlockException;
import org.elasticsearch.cluster.block.ClusterBlockLevel;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
import org.elasticsearch.cluster.routing.GroupShardsIterator;
import org.elasticsearch.cluster.routing.ShardIterator;
import org.elasticsearch.cluster.routing.ShardRouting;
Expand Down Expand Up @@ -133,7 +134,7 @@ protected void doExecute(Task task, ValidateQueryRequest request, ActionListener
@Override
protected ShardValidateQueryRequest newShardRequest(int numShards, ShardRouting shard, ValidateQueryRequest request) {
final ClusterState clusterState = clusterService.state();
final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
final Set<ResolvedExpression> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
final AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, shard.getIndexName(), indicesAndAliases);
return new ShardValidateQueryRequest(shard.shardId(), aliasFilter, request);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.elasticsearch.action.support.single.shard.TransportSingleShardAction;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
import org.elasticsearch.cluster.routing.ShardIterator;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.io.stream.Writeable;
Expand Down Expand Up @@ -109,7 +110,7 @@ protected boolean resolveIndex(ExplainRequest request) {

@Override
protected void resolveRequest(ClusterState state, InternalRequest request) {
final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(state, request.request().index());
final Set<ResolvedExpression> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(state, request.request().index());
final AliasFilter aliasFilter = searchService.buildAliasFilter(state, request.concreteIndex(), indicesAndAliases);
request.request().filteringAlias(aliasFilter);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.cluster.routing.GroupShardsIterator;
Expand Down Expand Up @@ -111,6 +112,7 @@
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.LongSupplier;
import java.util.stream.Collectors;

import static org.elasticsearch.action.search.SearchType.DFS_QUERY_THEN_FETCH;
import static org.elasticsearch.action.search.SearchType.QUERY_THEN_FETCH;
Expand Down Expand Up @@ -207,14 +209,17 @@ public TransportSearchAction(

private Map<String, OriginalIndices> buildPerIndexOriginalIndices(
ClusterState clusterState,
Set<String> indicesAndAliases,
Set<ResolvedExpression> indicesAndAliases,
String[] indices,
IndicesOptions indicesOptions
) {
Map<String, OriginalIndices> res = Maps.newMapWithExpectedSize(indices.length);
var blocks = clusterState.blocks();
// optimization: mostly we do not have any blocks so there's no point in the expensive per-index checking
boolean hasBlocks = blocks.global().isEmpty() == false || blocks.indices().isEmpty() == false;
// Get a distinct set of index abstraction names present from the resolved expressions to help with the reverse resolution from
// concrete index to the expression that produced it.
Set<String> indicesAndAliasesResources = indicesAndAliases.stream().map(ResolvedExpression::resource).collect(Collectors.toSet());
for (String index : indices) {
if (hasBlocks) {
blocks.indexBlockedRaiseException(ClusterBlockLevel.READ, index);
Expand All @@ -231,8 +236,8 @@ private Map<String, OriginalIndices> buildPerIndexOriginalIndices(
String[] finalIndices = Strings.EMPTY_ARRAY;
if (aliases == null
|| aliases.length == 0
|| indicesAndAliases.contains(index)
|| hasDataStreamRef(clusterState, indicesAndAliases, index)) {
|| indicesAndAliasesResources.contains(index)
|| hasDataStreamRef(clusterState, indicesAndAliasesResources, index)) {
finalIndices = new String[] { index };
}
if (aliases != null) {
Expand All @@ -251,7 +256,11 @@ private static boolean hasDataStreamRef(ClusterState clusterState, Set<String> i
return indicesAndAliases.contains(ret.getParentDataStream().getName());
}

Map<String, AliasFilter> buildIndexAliasFilters(ClusterState clusterState, Set<String> indicesAndAliases, Index[] concreteIndices) {
Map<String, AliasFilter> buildIndexAliasFilters(
ClusterState clusterState,
Set<ResolvedExpression> indicesAndAliases,
Index[] concreteIndices
) {
final Map<String, AliasFilter> aliasFilterMap = new HashMap<>();
for (Index index : concreteIndices) {
clusterState.blocks().indexBlockedRaiseException(ClusterBlockLevel.READ, index.getName());
Expand Down Expand Up @@ -1236,7 +1245,10 @@ private void executeSearch(
} else {
final Index[] indices = resolvedIndices.getConcreteLocalIndices();
concreteLocalIndices = Arrays.stream(indices).map(Index::getName).toArray(String[]::new);
final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, searchRequest.indices());
final Set<ResolvedExpression> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(
clusterState,
searchRequest.indices()
);
aliasFilter = buildIndexAliasFilters(clusterState, indicesAndAliases, indices);
aliasFilter.putAll(remoteAliasMap);
localShardIterators = getLocalShardsIterator(
Expand Down Expand Up @@ -1835,7 +1847,7 @@ List<SearchShardIterator> getLocalShardsIterator(
ClusterState clusterState,
SearchRequest searchRequest,
String clusterAlias,
Set<String> indicesAndAliases,
Set<ResolvedExpression> indicesAndAliases,
String[] concreteIndices
) {
var routingMap = indexNameExpressionResolver.resolveSearchRouting(clusterState, searchRequest.routing(), searchRequest.indices());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.action.support.HandledTransportAction;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
import org.elasticsearch.cluster.routing.GroupShardsIterator;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.index.Index;
Expand Down Expand Up @@ -127,7 +128,10 @@ public void searchShards(Task task, SearchShardsRequest searchShardsRequest, Act
searchService.getRewriteContext(timeProvider::absoluteStartMillis, resolvedIndices, null),
listener.delegateFailureAndWrap((delegate, searchRequest) -> {
Index[] concreteIndices = resolvedIndices.getConcreteLocalIndices();
final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, searchRequest.indices());
final Set<ResolvedExpression> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(
clusterState,
searchRequest.indices()
);
final Map<String, AliasFilter> aliasFilters = transportSearchAction.buildIndexAliasFilters(
clusterState,
indicesAndAliases,
Expand Down
Loading

0 comments on commit 34b7e60

Please sign in to comment.