Skip to content

Commit

Permalink
Single instance of the IndexNameExpressionResolver (#52596)
Browse files Browse the repository at this point in the history
This commit modifies the codebase so that our production code uses a
single instance of the IndexNameExpressionResolver class. This change
is being made in preparation for allowing name expression resolution
to be augmented by a plugin.

In order to remove some instances of IndexNameExpressionResolver, the
single instance is added as a parameter of Plugin#createComponents and
PersistentTaskPlugin#getPersistentTasksExecutor.
  • Loading branch information
jaymode authored Feb 20, 2020
1 parent 4c98d0c commit 0d1e67d
Show file tree
Hide file tree
Showing 70 changed files with 267 additions and 159 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.Version;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.logging.DeprecationLogger;
Expand Down Expand Up @@ -162,7 +163,8 @@ public class CommonAnalysisPlugin extends Plugin implements AnalysisPlugin, Scri
public Collection<Object> createComponents(Client client, ClusterService clusterService, ThreadPool threadPool,
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry) {
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver expressionResolver) {
this.scriptService.set(scriptService);
return Collections.emptyList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryTyp
};

CommonAnalysisPlugin plugin = new CommonAnalysisPlugin();
plugin.createComponents(null, null, null, null, scriptService, null, null, null, null);
plugin.createComponents(null, null, null, null, scriptService, null, null, null, null, null);
AnalysisModule module
= new AnalysisModule(TestEnvironment.newEnvironment(settings), Collections.singletonList(plugin));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryTyp
};

CommonAnalysisPlugin plugin = new CommonAnalysisPlugin();
plugin.createComponents(null, null, null, null, scriptService, null, null, null, null);
plugin.createComponents(null, null, null, null, scriptService, null, null, null, null, null);
AnalysisModule module
= new AnalysisModule(TestEnvironment.newEnvironment(settings), Collections.singletonList(plugin));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ public ScriptEngine getScriptEngine(Settings settings, Collection<ScriptContext<
public Collection<Object> createComponents(Client client, ClusterService clusterService, ThreadPool threadPool,
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry) {
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver expressionResolver) {
// this is a hack to bind the painless script engine in guice (all components are added to guice), so that
// the painless context api. this is a temporary measure until transport actions do no require guice
return Collections.singletonList(painlessScriptEngine.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
public Collection<Object> createComponents(Client client, ClusterService clusterService, ThreadPool threadPool,
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry) {
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver expressionResolver) {
return Collections.singletonList(new ReindexSslConfig(environment.settings(), environment, resourceWatcherService));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.action.support.ActionFilterChain;
import org.elasticsearch.action.support.WriteRequest.RefreshPolicy;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
Expand Down Expand Up @@ -151,7 +152,8 @@ public static class TestPlugin extends Plugin implements ActionPlugin {
public Collection<Object> createComponents(Client client, ClusterService clusterService, ThreadPool threadPool,
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry) {
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver expressionResolver) {
testFilter.set(new ReindexFromRemoteWithAuthTests.TestFilter(threadPool));
return Collections.emptyList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.logging.log4j.Logger;
import org.elasticsearch.Build;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.unit.TimeValue;
Expand Down Expand Up @@ -88,7 +89,8 @@ public Collection<Object> createComponents(
final NamedXContentRegistry xContentRegistry,
final Environment environment,
final NodeEnvironment nodeEnvironment,
final NamedWriteableRegistry namedWriteableRegistry) {
final NamedWriteableRegistry namedWriteableRegistry,
final IndexNameExpressionResolver expressionResolver) {
if (enabled) {
/*
* Since we have set the service type to notify, by default systemd will wait up to sixty seconds for the process to send the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,28 +61,28 @@ public class SystemdPluginTests extends ESTestCase {

public void testIsEnabled() {
final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, Boolean.TRUE.toString());
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null);
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null, null);
assertTrue(plugin.isEnabled());
assertNotNull(plugin.extender);
}

public void testIsNotPackageDistribution() {
final SystemdPlugin plugin = new SystemdPlugin(false, randomNonPackageBuildType, Boolean.TRUE.toString());
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null);
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null, null);
assertFalse(plugin.isEnabled());
assertNull(plugin.extender);
}

public void testIsImplicitlyNotEnabled() {
final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, null);
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null);
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null, null);
assertFalse(plugin.isEnabled());
assertNull(plugin.extender);
}

public void testIsExplicitlyNotEnabled() {
final SystemdPlugin plugin = new SystemdPlugin(false, randomPackageBuildType, Boolean.FALSE.toString());
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null);
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null, null);
assertFalse(plugin.isEnabled());
assertNull(plugin.extender);
}
Expand Down Expand Up @@ -181,7 +181,7 @@ int sd_notify(final int unset_environment, final String state) {
}

};
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null);
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null, null);
if (Boolean.TRUE.toString().equals(esSDNotify)) {
assertNotNull(plugin.extender);
} else {
Expand Down
8 changes: 6 additions & 2 deletions server/src/main/java/org/elasticsearch/index/IndexModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.Version;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.CheckedFunction;
import org.elasticsearch.common.TriFunction;
Expand Down Expand Up @@ -129,6 +130,7 @@ public final class IndexModule {
private final SetOnce<BiFunction<IndexSettings, IndicesQueryCache, QueryCache>> forceQueryCacheProvider = new SetOnce<>();
private final List<SearchOperationListener> searchOperationListeners = new ArrayList<>();
private final List<IndexingOperationListener> indexOperationListeners = new ArrayList<>();
private final IndexNameExpressionResolver expressionResolver;
private final AtomicBoolean frozen = new AtomicBoolean(false);
private final BooleanSupplier allowExpensiveQueries;

Expand All @@ -146,14 +148,16 @@ public IndexModule(
final AnalysisRegistry analysisRegistry,
final EngineFactory engineFactory,
final Map<String, IndexStorePlugin.DirectoryFactory> directoryFactories,
final BooleanSupplier allowExpensiveQueries) {
final BooleanSupplier allowExpensiveQueries,
final IndexNameExpressionResolver expressionResolver) {
this.indexSettings = indexSettings;
this.analysisRegistry = analysisRegistry;
this.engineFactory = Objects.requireNonNull(engineFactory);
this.searchOperationListeners.add(new SearchSlowLog(indexSettings));
this.indexOperationListeners.add(new IndexingSlowLog(indexSettings));
this.directoryFactories = Collections.unmodifiableMap(directoryFactories);
this.allowExpensiveQueries = allowExpensiveQueries;
this.expressionResolver = expressionResolver;
}

/**
Expand Down Expand Up @@ -427,7 +431,7 @@ public IndexService newIndexService(
new SimilarityService(indexSettings, scriptService, similarities), shardStoreDeleter, indexAnalyzers,
engineFactory, circuitBreakerService, bigArrays, threadPool, scriptService, clusterService, client, queryCache,
directoryFactory, eventListener, readerWrapperFactory, mapperRegistry, indicesFieldDataCache, searchOperationListeners,
indexOperationListeners, namedWriteableRegistry, idFieldDataEnabled, allowExpensiveQueries);
indexOperationListeners, namedWriteableRegistry, idFieldDataEnabled, allowExpensiveQueries, expressionResolver);
success = true;
return indexService;
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.Assertions;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.CheckedFunction;
Expand Down Expand Up @@ -142,6 +143,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
private final ClusterService clusterService;
private final Client client;
private final CircuitBreakerService circuitBreakerService;
private final IndexNameExpressionResolver expressionResolver;
private Supplier<Sort> indexSortSupplier;

public IndexService(
Expand All @@ -168,14 +170,16 @@ public IndexService(
List<IndexingOperationListener> indexingOperationListeners,
NamedWriteableRegistry namedWriteableRegistry,
BooleanSupplier idFieldDataEnabled,
BooleanSupplier allowExpensiveQueries) {
BooleanSupplier allowExpensiveQueries,
IndexNameExpressionResolver expressionResolver) {
super(indexSettings);
this.allowExpensiveQueries = allowExpensiveQueries;
this.indexSettings = indexSettings;
this.xContentRegistry = xContentRegistry;
this.similarityService = similarityService;
this.namedWriteableRegistry = namedWriteableRegistry;
this.circuitBreakerService = circuitBreakerService;
this.expressionResolver = expressionResolver;
if (needsMapperService(indexSettings, indexCreationContext)) {
assert indexAnalyzers != null;
this.mapperService = new MapperService(indexSettings, indexAnalyzers, xContentRegistry, similarityService, mapperRegistry,
Expand Down Expand Up @@ -567,7 +571,8 @@ public IndexSettings getIndexSettings() {
* {@link IndexReader}-specific optimizations, such as rewriting containing range queries.
*/
public QueryShardContext newQueryShardContext(int shardId, IndexSearcher searcher, LongSupplier nowInMillis, String clusterAlias) {
SearchIndexNameMatcher indexNameMatcher = new SearchIndexNameMatcher(index().getName(), clusterAlias, clusterService);
final SearchIndexNameMatcher indexNameMatcher =
new SearchIndexNameMatcher(index().getName(), clusterAlias, clusterService, expressionResolver);
return new QueryShardContext(
shardId, indexSettings, bigArrays, indexCache.bitsetFilterCache(), indexFieldData::getForField, mapperService(),
similarityService(), scriptService, xContentRegistry, namedWriteableRegistry, client, searcher, nowInMillis, clusterAlias,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@ public class SearchIndexNameMatcher implements Predicate<String> {
*/
public SearchIndexNameMatcher(String indexName,
String clusterAlias,
ClusterService clusterService) {
ClusterService clusterService,
IndexNameExpressionResolver expressionResolver) {
this.indexName = indexName;
this.clusterAlias = RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY.equals(clusterAlias) ? null : clusterAlias;
this.clusterService = clusterService;
this.expressionResolver = new IndexNameExpressionResolver();
this.expressionResolver = expressionResolver;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ private synchronized IndexService createIndexService(IndexService.IndexCreationC
indexCreationContext);

final IndexModule indexModule = new IndexModule(idxSettings, analysisRegistry, getEngineFactory(idxSettings),
directoryFactories, () -> allowExpensiveQueries);
directoryFactories, () -> allowExpensiveQueries, indexNameExpressionResolver);
for (IndexingOperationListener operationListener : indexingOperationListeners) {
indexModule.addIndexOperationListener(operationListener);
}
Expand Down Expand Up @@ -662,7 +662,7 @@ private EngineFactory getEngineFactory(final IndexSettings idxSettings) {
public synchronized MapperService createIndexMapperService(IndexMetaData indexMetaData) throws IOException {
final IndexSettings idxSettings = new IndexSettings(indexMetaData, this.settings, indexScopedSettings);
final IndexModule indexModule = new IndexModule(idxSettings, analysisRegistry, getEngineFactory(idxSettings),
directoryFactories, () -> allowExpensiveQueries);
directoryFactories, () -> allowExpensiveQueries, indexNameExpressionResolver);
pluginsService.onIndexModule(indexModule);
return indexModule.newIndexMapperService(xContentRegistry, mapperRegistry, scriptService);
}
Expand Down
5 changes: 3 additions & 2 deletions server/src/main/java/org/elasticsearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ protected Node(
Collection<Object> pluginComponents = pluginsService.filterPlugins(Plugin.class).stream()
.flatMap(p -> p.createComponents(client, clusterService, threadPool, resourceWatcherService,
scriptModule.getScriptService(), xContentRegistry, environment, nodeEnvironment,
namedWriteableRegistry).stream())
namedWriteableRegistry, clusterModule.getIndexNameExpressionResolver()).stream())
.collect(Collectors.toList());

ActionModule actionModule = new ActionModule(settings, clusterModule.getIndexNameExpressionResolver(),
Expand Down Expand Up @@ -533,7 +533,8 @@ protected Node(

final List<PersistentTasksExecutor<?>> tasksExecutors = pluginsService
.filterPlugins(PersistentTaskPlugin.class).stream()
.map(p -> p.getPersistentTasksExecutor(clusterService, threadPool, client, settingsModule))
.map(p -> p.getPersistentTasksExecutor(clusterService, threadPool, client, settingsModule,
clusterModule.getIndexNameExpressionResolver()))
.flatMap(List::stream)
.collect(toList());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.elasticsearch.plugins;

import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.settings.SettingsModule;
import org.elasticsearch.persistent.PersistentTasksExecutor;
Expand All @@ -38,7 +39,8 @@ public interface PersistentTaskPlugin {
default List<PersistentTasksExecutor<?>> getPersistentTasksExecutor(ClusterService clusterService,
ThreadPool threadPool,
Client client,
SettingsModule settingsModule) {
SettingsModule settingsModule,
IndexNameExpressionResolver expressionResolver) {
return Collections.emptyList();
}

Expand Down
Loading

0 comments on commit 0d1e67d

Please sign in to comment.