diff --git a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java index 09e967b00c4..f36fd199292 100644 --- a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -19,6 +19,7 @@ package org.elasticsearch.client.transport; +import org.apache.lucene.util.IOUtils; import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionModule; @@ -41,7 +42,6 @@ import org.elasticsearch.common.settings.SettingsModule; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.indices.breaker.CircuitBreakerService; -import org.elasticsearch.monitor.MonitorService; import org.elasticsearch.node.Node; import org.elasticsearch.node.internal.InternalSettingsPreparer; import org.elasticsearch.plugins.Plugin; @@ -52,6 +52,7 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.netty.NettyTransport; +import java.io.Closeable; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -118,10 +119,11 @@ public class TransportClient extends AbstractClient { public TransportClient build() { final PluginsService pluginsService = newPluginService(providedSettings); final Settings settings = pluginsService.updatedSettings(); + final List resourcesToClose = new ArrayList<>(); final ThreadPool threadPool = new ThreadPool(settings); + resourcesToClose.add(() -> ThreadPool.terminate(threadPool, 10, TimeUnit.SECONDS)); final NetworkService networkService = new NetworkService(settings); NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry(); - boolean success = false; try { ModulesBuilder modules = new ModulesBuilder(); // plugin modules must be added here, before others or we can get crazy injection errors... @@ -149,8 +151,12 @@ public class TransportClient extends AbstractClient { SettingsModule settingsModule = new SettingsModule(settings, additionalSettings, additionalSettingsFilter); CircuitBreakerService circuitBreakerService = Node.createCircuitBreakerService(settingsModule.getSettings(), settingsModule.getClusterSettings()); + resourcesToClose.add(circuitBreakerService); + BigArrays bigArrays = new BigArrays(settings, circuitBreakerService); + resourcesToClose.add(bigArrays); modules.add(settingsModule); modules.add((b -> { + b.bind(BigArrays.class).toInstance(bigArrays); b.bind(PluginsService.class).toInstance(pluginsService); b.bind(CircuitBreakerService.class).toInstance(circuitBreakerService); })); @@ -159,14 +165,11 @@ public class TransportClient extends AbstractClient { final TransportService transportService = injector.getInstance(TransportService.class); transportService.start(); transportService.acceptIncomingRequests(); - TransportClient transportClient = new TransportClient(injector); - success = true; + resourcesToClose.clear(); return transportClient; } finally { - if (!success) { - ThreadPool.terminate(threadPool, 10, TimeUnit.SECONDS); - } + IOUtils.closeWhileHandlingException(resourcesToClose); } } } @@ -261,24 +264,16 @@ public class TransportClient extends AbstractClient { */ @Override public void close() { - injector.getInstance(TransportClientNodesService.class).close(); - injector.getInstance(TransportService.class).close(); - try { - injector.getInstance(MonitorService.class).close(); - } catch (Exception e) { - // ignore, might not be bounded - } + List closeables = new ArrayList<>(); + closeables.add(injector.getInstance(TransportClientNodesService.class)); + closeables.add(injector.getInstance(TransportService.class)); for (Class plugin : injector.getInstance(PluginsService.class).nodeServices()) { - injector.getInstance(plugin).close(); + closeables.add(injector.getInstance(plugin)); } - try { - ThreadPool.terminate(injector.getInstance(ThreadPool.class), 10, TimeUnit.SECONDS); - } catch (Exception e) { - // ignore - } - - injector.getInstance(BigArrays.class).close(); + closeables.add(() -> ThreadPool.terminate(injector.getInstance(ThreadPool.class), 10, TimeUnit.SECONDS)); + closeables.add(injector.getInstance(BigArrays.class)); + IOUtils.closeWhileHandlingException(closeables); } @Override diff --git a/core/src/main/java/org/elasticsearch/client/transport/TransportClientNodesService.java b/core/src/main/java/org/elasticsearch/client/transport/TransportClientNodesService.java index 0781576524d..120a4711217 100644 --- a/core/src/main/java/org/elasticsearch/client/transport/TransportClientNodesService.java +++ b/core/src/main/java/org/elasticsearch/client/transport/TransportClientNodesService.java @@ -48,6 +48,7 @@ import org.elasticsearch.transport.TransportException; import org.elasticsearch.transport.TransportRequestOptions; import org.elasticsearch.transport.TransportService; +import java.io.Closeable; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -66,7 +67,7 @@ import static org.elasticsearch.common.unit.TimeValue.timeValueSeconds; /** * */ -public class TransportClientNodesService extends AbstractComponent { +public class TransportClientNodesService extends AbstractComponent implements Closeable { private final TimeValue nodesSamplerInterval; diff --git a/core/src/main/java/org/elasticsearch/common/util/BigArrays.java b/core/src/main/java/org/elasticsearch/common/util/BigArrays.java index 017881a9218..c2c8e242acc 100644 --- a/core/src/main/java/org/elasticsearch/common/util/BigArrays.java +++ b/core/src/main/java/org/elasticsearch/common/util/BigArrays.java @@ -373,12 +373,11 @@ public class BigArrays implements Releasable { final boolean checkBreaker; private final BigArrays circuitBreakingInstance; - @Inject public BigArrays(Settings settings, @Nullable final CircuitBreakerService breakerService) { // Checking the breaker is disabled if not specified this(new PageCacheRecycler(settings), breakerService, false); } - + // public for tests public BigArrays(PageCacheRecycler recycler, @Nullable final CircuitBreakerService breakerService, boolean checkBreaker) { this.checkBreaker = checkBreaker; this.recycler = recycler; diff --git a/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 2b2aab2ccd2..8815d18769d 100644 --- a/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -32,12 +32,9 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.component.AbstractComponent; -import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.FileSystemUtils; import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; @@ -66,7 +63,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -167,7 +163,6 @@ public final class NodeEnvironment extends AbstractComponent implements Closeabl public static final String NODE_LOCK_FILENAME = "node.lock"; public static final String UPGRADE_LOCK_FILENAME = "upgrade.lock"; - @Inject public NodeEnvironment(Settings settings, Environment environment) throws IOException { super(settings); diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index 68073261959..de77f02ef51 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -266,6 +266,8 @@ public class Node implements Closeable { CircuitBreakerService circuitBreakerService = createCircuitBreakerService(settingsModule.getSettings(), settingsModule.getClusterSettings()); resourcesToClose.add(circuitBreakerService); + BigArrays bigArrays = createBigArrays(settings, circuitBreakerService); + resourcesToClose.add(bigArrays); modules.add(settingsModule); modules.add(b -> { b.bind(PluginsService.class).toInstance(pluginsService); @@ -276,6 +278,7 @@ public class Node implements Closeable { b.bind(TribeService.class).toInstance(tribeService); b.bind(ResourceWatcherService.class).toInstance(resourceWatcherService); b.bind(CircuitBreakerService.class).toInstance(circuitBreakerService); + b.bind(BigArrays.class).toInstance(bigArrays); } ); injector = modules.createInjector(); @@ -626,4 +629,12 @@ public class Node implements Closeable { throw new IllegalArgumentException("Unknown circuit breaker type [" + type + "]"); } } + + /** + * Creates a new {@link BigArrays} instance used for this node. + * This method can be overwritten by subclasses to change their {@link BigArrays} implementation for instance for testing + */ + BigArrays createBigArrays(Settings settings, CircuitBreakerService circuitBreakerService) { + return new BigArrays(settings, circuitBreakerService); + } } diff --git a/core/src/main/java/org/elasticsearch/node/NodeModule.java b/core/src/main/java/org/elasticsearch/node/NodeModule.java index c5d087c0ef2..85cfe5bd6b2 100644 --- a/core/src/main/java/org/elasticsearch/node/NodeModule.java +++ b/core/src/main/java/org/elasticsearch/node/NodeModule.java @@ -37,9 +37,6 @@ public class NodeModule extends AbstractModule { private final MonitorService monitorService; private final ProcessorsRegistry.Builder processorsRegistryBuilder; - // pkg private so tests can mock - Class bigArraysImpl = BigArrays.class; - public NodeModule(Node node, MonitorService monitorService) { this.node = node; this.monitorService = monitorService; @@ -48,12 +45,6 @@ public class NodeModule extends AbstractModule { @Override protected void configure() { - if (bigArraysImpl == BigArrays.class) { - bind(BigArrays.class).asEagerSingleton(); - } else { - bind(BigArrays.class).to(bigArraysImpl).asEagerSingleton(); - } - bind(Node.class).toInstance(node); bind(MonitorService.class).toInstance(monitorService); bind(NodeService.class).asEagerSingleton(); diff --git a/test/framework/src/main/java/org/elasticsearch/common/util/MockBigArrays.java b/test/framework/src/main/java/org/elasticsearch/common/util/MockBigArrays.java index 26e021bf031..613445c2271 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/util/MockBigArrays.java +++ b/test/framework/src/main/java/org/elasticsearch/common/util/MockBigArrays.java @@ -74,7 +74,6 @@ public class MockBigArrays extends BigArrays { private final PageCacheRecycler recycler; private final CircuitBreakerService breakerService; - @Inject public MockBigArrays(Settings settings, CircuitBreakerService breakerService) { this(new MockPageCacheRecycler(settings), breakerService, false); } diff --git a/test/framework/src/main/java/org/elasticsearch/node/MockNode.java b/test/framework/src/main/java/org/elasticsearch/node/MockNode.java index 749f1f3279e..33077d5ffbd 100644 --- a/test/framework/src/main/java/org/elasticsearch/node/MockNode.java +++ b/test/framework/src/main/java/org/elasticsearch/node/MockNode.java @@ -19,8 +19,10 @@ package org.elasticsearch.node; -import org.elasticsearch.Version; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.common.util.MockBigArrays; +import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.node.internal.InternalSettingsPreparer; import org.elasticsearch.plugins.Plugin; @@ -35,15 +37,25 @@ import java.util.Collection; */ public class MockNode extends Node { + private final boolean mockBigArrays; private Collection> plugins; public MockNode(Settings settings, Collection> classpathPlugins) { super(InternalSettingsPreparer.prepareEnvironment(settings, null), classpathPlugins); this.plugins = classpathPlugins; + this.mockBigArrays = classpathPlugins.contains(NodeMocksPlugin.class); // if this plugin is present we mock bigarrays :) } public Collection> getPlugins() { return plugins; } + @Override + protected BigArrays createBigArrays(Settings settings, CircuitBreakerService circuitBreakerService) { + if (mockBigArrays) { + return new MockBigArrays(settings, circuitBreakerService); + } else { + return super.createBigArrays(settings, circuitBreakerService); + } + } } diff --git a/test/framework/src/main/java/org/elasticsearch/node/NodeMocksPlugin.java b/test/framework/src/main/java/org/elasticsearch/node/NodeMocksPlugin.java index a121ad46bcc..2ca1ac7804d 100644 --- a/test/framework/src/main/java/org/elasticsearch/node/NodeMocksPlugin.java +++ b/test/framework/src/main/java/org/elasticsearch/node/NodeMocksPlugin.java @@ -18,12 +18,7 @@ */ package org.elasticsearch.node; -import org.elasticsearch.common.util.MockBigArrays; import org.elasticsearch.plugins.Plugin; -public class NodeMocksPlugin extends Plugin { - - public void onModule(NodeModule module) { - module.bigArraysImpl = MockBigArrays.class; - } +public class NodeMocksPlugin extends Plugin { // just a marker plugin for MockNode to mock out BigArrays } diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 950653cba5e..025740dbe36 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -902,9 +902,12 @@ public final class InternalTestCluster extends TestCluster { @Override public void close() throws IOException { - resetClient(); - closed.set(true); - closeNode(); + try { + resetClient(); + } finally { + closed.set(true); + closeNode(); + } } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/NodeConfigurationSource.java b/test/framework/src/main/java/org/elasticsearch/test/NodeConfigurationSource.java index c54a00b174d..e04e840e525 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/NodeConfigurationSource.java +++ b/test/framework/src/main/java/org/elasticsearch/test/NodeConfigurationSource.java @@ -19,18 +19,10 @@ package org.elasticsearch.test; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.MockEngineFactoryPlugin; -import org.elasticsearch.node.NodeMocksPlugin; import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.search.MockSearchService; -import org.elasticsearch.test.store.MockFSIndexStore; -import org.elasticsearch.test.transport.AssertingLocalTransport; -import org.elasticsearch.test.transport.MockTransportService; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.List; public abstract class NodeConfigurationSource {