Skip to content

Commit

Permalink
Add RepositoriesService to createComponents() args (elastic#54814)
Browse files Browse the repository at this point in the history
Today we pass the `RepositoriesService` to the searchable snapshots plugin
during the initialization of the `RepositoryModule`, forcing the plugin to be a
`RepositoryPlugin` even though it does not implement any repositories.

After discussion we decided it best for now to pass this in via
`Plugin#createComponents` instead, pending some future work in which plugins
can depend on services more dynamically.
  • Loading branch information
DaveCTurner committed Apr 16, 2020
1 parent 327d268 commit 7941f4a
Show file tree
Hide file tree
Showing 37 changed files with 128 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@
import org.elasticsearch.plugins.AnalysisPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.ScriptPlugin;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.threadpool.ThreadPool;
Expand All @@ -150,6 +151,7 @@
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.Supplier;

import static org.elasticsearch.plugins.AnalysisPlugin.requiresAnalysisSettings;

Expand All @@ -164,7 +166,8 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver expressionResolver) {
IndexNameExpressionResolver expressionResolver,
Supplier<RepositoriesService> repositoriesServiceSupplier) {
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, null);
plugin.createComponents(null, null, null, null, scriptService, null, 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, null);
plugin.createComponents(null, null, null, null, scriptService, null, 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 @@ -36,6 +36,7 @@
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.plugins.ActionPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestHandler;
import org.elasticsearch.script.ScriptService;
Expand Down Expand Up @@ -85,7 +86,8 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver expressionResolver) {
IndexNameExpressionResolver expressionResolver,
Supplier<RepositoriesService> repositoriesServiceSupplier) {
return Collections.singletonList(new ReindexSslConfig(environment.settings(), environment, resourceWatcherService));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.elasticsearch.http.HttpInfo;
import org.elasticsearch.plugins.ActionPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.rest.RestHeaderDefinition;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.script.ScriptService;
Expand All @@ -60,6 +61,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
Expand Down Expand Up @@ -154,7 +156,8 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver expressionResolver) {
IndexNameExpressionResolver expressionResolver,
Supplier<RepositoriesService> repositoriesServiceSupplier) {
testFilter.set(new ReindexFromRemoteWithAuthTests.TestFilter(threadPool));
return Collections.emptyList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.plugins.ClusterPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.threadpool.Scheduler;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.watcher.ResourceWatcherService;

import java.util.Collection;
import java.util.Collections;
import java.util.function.Supplier;

public class SystemdPlugin extends Plugin implements ClusterPlugin {

Expand Down Expand Up @@ -90,7 +92,8 @@ public Collection<Object> createComponents(
final Environment environment,
final NodeEnvironment nodeEnvironment,
final NamedWriteableRegistry namedWriteableRegistry,
final IndexNameExpressionResolver expressionResolver) {
final IndexNameExpressionResolver expressionResolver,
final Supplier<RepositoriesService> repositoriesServiceSupplier) {
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, null);
plugin.createComponents(null, null, threadPool, null, 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, null);
plugin.createComponents(null, null, threadPool, null, 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, null);
plugin.createComponents(null, null, threadPool, null, 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, null);
plugin.createComponents(null, null, threadPool, null, 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, null);
plugin.createComponents(null, null, threadPool, null, null, null, null, null, null, null, null);
if (Boolean.TRUE.toString().equals(esSDNotify)) {
assertNotNull(plugin.extender);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.elasticsearch.indices.TermsLookup;
import org.elasticsearch.plugins.ActionPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.rest.RestHeaderDefinition;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
Expand All @@ -67,6 +68,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Supplier;

import static java.util.Collections.singletonList;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
Expand Down Expand Up @@ -301,7 +303,8 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver indexNameExpressionResolver) {
IndexNameExpressionResolver indexNameExpressionResolver,
Supplier<RepositoriesService> repositoriesServiceSupplier) {
loggingFilter.set(new LoggingFilter(threadPool));
return Collections.emptyList();
}
Expand Down
5 changes: 4 additions & 1 deletion server/src/main/java/org/elasticsearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -477,10 +477,12 @@ protected Node(final Environment initialEnvironment,
systemIndexDescriptors,
forbidPrivateIndexSettings);

final SetOnce<RepositoriesService> repositoriesServiceReference = new SetOnce<>();
Collection<Object> pluginComponents = pluginsService.filterPlugins(Plugin.class).stream()
.flatMap(p -> p.createComponents(client, clusterService, threadPool, resourceWatcherService,
scriptService, xContentRegistry, environment, nodeEnvironment,
namedWriteableRegistry, clusterModule.getIndexNameExpressionResolver()).stream())
namedWriteableRegistry, clusterModule.getIndexNameExpressionResolver(),
repositoriesServiceReference::get).stream())
.collect(Collectors.toList());

ActionModule actionModule = new ActionModule(false, settings, clusterModule.getIndexNameExpressionResolver(),
Expand Down Expand Up @@ -517,6 +519,7 @@ protected Node(final Environment initialEnvironment,
RepositoriesModule repositoriesModule = new RepositoriesModule(this.environment,
pluginsService.filterPlugins(RepositoryPlugin.class), transportService, clusterService, threadPool, xContentRegistry);
RepositoriesService repositoryService = repositoriesModule.getRepositoryService();
repositoriesServiceReference.set(repositoryService);
SnapshotsService snapshotsService = new SnapshotsService(settings, clusterService,
clusterModule.getIndexNameExpressionResolver(), repositoryService, threadPool);
SnapshotShardsService snapshotShardsService = new SnapshotShardsService(settings, clusterService, repositoryService,
Expand Down
7 changes: 6 additions & 1 deletion server/src/main/java/org/elasticsearch/plugins/Plugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.threadpool.ExecutorBuilder;
import org.elasticsearch.threadpool.ThreadPool;
Expand All @@ -52,6 +53,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;

/**
Expand Down Expand Up @@ -116,12 +118,15 @@ public Collection<Class<? extends LifecycleComponent>> getGuiceServiceClasses()
* @param nodeEnvironment the node environment used coordinate access to the data paths
* @param namedWriteableRegistry the registry for {@link NamedWriteable} object parsing
* @param indexNameExpressionResolver A service that resolves expression to index and alias names
* @param repositoriesServiceSupplier A supplier for the service that manages snapshot repositories; will return null when this method
* is called, but will return the repositories service once the node is initialized.
*/
public Collection<Object> createComponents(Client client, ClusterService clusterService, ThreadPool threadPool,
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver indexNameExpressionResolver) {
IndexNameExpressionResolver indexNameExpressionResolver,
Supplier<RepositoriesService> repositoriesServiceSupplier) {
return Collections.emptyList();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@

package org.elasticsearch.plugins;

import java.util.Collections;
import java.util.Map;

import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.env.Environment;
import org.elasticsearch.repositories.RepositoriesModule;
import org.elasticsearch.repositories.Repository;

import java.util.Collections;
import java.util.Map;

/**
* An extension point for {@link Plugin} implementations to add custom snapshot repositories.
*/
Expand Down Expand Up @@ -60,12 +59,4 @@ default Map<String, Repository.Factory> getInternalRepositories(Environment env,
return Collections.emptyMap();
}

/**
* Passes down the current {@link RepositoriesModule} to repository plugins.
*
* @param module the current {@link RepositoriesModule}
*/
default void onRepositoriesModule(RepositoriesModule module) {
// NORELEASE
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ public RepositoriesModule(Environment env, List<RepositoryPlugin> repoPlugins, T
Map<String, Repository.Factory> internalRepositoryTypes = Collections.unmodifiableMap(internalFactories);
repositoriesService = new RepositoriesService(settings, clusterService, transportService, repositoryTypes,
internalRepositoryTypes, threadPool);

repoPlugins.forEach(rp -> rp.onRepositoriesModule(this));
}

public RepositoriesService getRepositoryService() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.elasticsearch.ingest.Processor;
import org.elasticsearch.plugins.IngestPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.ESSingleNodeTestCase;
import org.elasticsearch.threadpool.ThreadPool;
Expand All @@ -48,6 +49,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.function.Supplier;

import static org.hamcrest.Matchers.equalTo;

Expand Down Expand Up @@ -102,7 +104,8 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
ResourceWatcherService resourceWatcherService, ScriptService scriptService,
NamedXContentRegistry xContentRegistry, Environment environment,
NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
IndexNameExpressionResolver expressionResolver) {
IndexNameExpressionResolver expressionResolver,
Supplier<RepositoriesService> repositoriesServiceSupplier) {
this.threadPool = threadPool;
return Collections.emptyList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.threadpool.ThreadPool;
Expand All @@ -56,6 +57,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;

import static org.elasticsearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK;
import static org.elasticsearch.test.ESIntegTestCase.Scope.TEST;
Expand Down Expand Up @@ -224,7 +226,8 @@ public Collection<Object> createComponents(
final Environment environment,
final NodeEnvironment nodeEnvironment,
final NamedWriteableRegistry namedWriteableRegistry,
final IndexNameExpressionResolver indexNameExpressionResolver) {
final IndexNameExpressionResolver indexNameExpressionResolver,
final Supplier<RepositoriesService> repositoriesServiceSupplier) {
clusterService.addListener(event -> {
final ClusterState state = event.state();
if (state.getBlocks().hasGlobalBlock(STATE_NOT_RECOVERED_BLOCK)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.plugins.ClusterPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.hamcrest.CollectionAssertions;
Expand All @@ -62,6 +63,7 @@
import java.util.Collections;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;

import static org.elasticsearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
Expand Down Expand Up @@ -383,7 +385,8 @@ public Collection<Object> createComponents(
final Environment environment,
final NodeEnvironment nodeEnvironment,
final NamedWriteableRegistry namedWriteableRegistry,
final IndexNameExpressionResolver expressionResolver) {
final IndexNameExpressionResolver expressionResolver,
final Supplier<RepositoriesService> repositoriesServiceSupplier) {
clusterService.addListener(event -> {
final ClusterState state = event.state();
if (state.getBlocks().hasGlobalBlock(STATE_NOT_RECOVERED_BLOCK)) {
Expand Down
Loading

0 comments on commit 7941f4a

Please sign in to comment.