From: Simon Brandhof Date: Thu, 4 Aug 2016 15:56:35 +0000 (+0200) Subject: SONAR-7908 correctly close ES TransportClient X-Git-Tag: 6.1-RC1~450 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b6069267bdebb809c93b4fe23cb85cd2b6d3bf0a;p=sonarqube.git SONAR-7908 correctly close ES TransportClient in order to stop all the ES threads and to not generate Tomcat warnings: 2016.08.04 12:10:34 WARN web[o.a.c.l.WebappClassLoaderBase] The web application [ROOT] appears to have started a thread named [elasticsearch[Her][transport_client_worker][T#1]{New I/O worker #1}] but has failed to stop it. This is very likely to create a memory leak. Stack trace of thread: sun.nio.ch.KQueueArrayWrapper.kevent0(Native Method) sun.nio.ch.KQueueArrayWrapper.poll(KQueueArrayWrapper.java:198) sun.nio.ch.KQueueSelectorImpl.doSelect(KQueueSelectorImpl.java:117) sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86) sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97) org.jboss.netty.channel.socket.nio.SelectorUtil.select(SelectorUtil.java:68) org.jboss.netty.channel.socket.nio.AbstractNioSelector.select(AbstractNioSelector.java:434) org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:212) org.jboss.netty.channel.socket.nio.AbstractNioWorker.run(AbstractNioWorker.java:89) org.jboss.netty.channel.socket.nio.NioWorker.run(NioWorker.java:178) org.jboss.netty.util.ThreadRenamingRunnable.run(ThreadRenamingRunnable.java:108) org.jboss.netty.util.internal.DeadLockProofWorker$1.run(DeadLockProofWorker.java:42) java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) java.lang.Thread.run(Thread.java:745) --- diff --git a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java index 89e2fbb2fb2..c9a83fa9177 100644 --- a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java +++ b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java @@ -106,7 +106,7 @@ public class ComputeEngineContainerImplTest { COMPONENTS_IN_LEVEL_1_AT_CONSTRUCTION + 26 // level 1 + 46 // content of DaoModule - + 1 // content of EsSearchModule + + 2 // content of EsSearchModule + 55 // content of CorePropertyDefinitions + 1 // content of CePropertyDefinitions ); diff --git a/server/sonar-server/src/main/java/org/sonar/server/es/EsClient.java b/server/sonar-server/src/main/java/org/sonar/server/es/EsClient.java index 9516ae242e8..d20cba3706b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/es/EsClient.java +++ b/server/sonar-server/src/main/java/org/sonar/server/es/EsClient.java @@ -19,6 +19,7 @@ */ package org.sonar.server.es; +import java.io.Closeable; import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequestBuilder; import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsRequestBuilder; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequestBuilder; @@ -45,7 +46,6 @@ import org.elasticsearch.common.Priority; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.aggregations.AggregationBuilders; import org.elasticsearch.search.aggregations.metrics.max.Max; -import org.picocontainer.Startable; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.server.es.request.ProxyBulkRequestBuilder; @@ -74,7 +74,7 @@ import static java.util.Objects.requireNonNull; * Facade to connect to Elasticsearch node. Handles correctly errors (logging + exceptions * with context) and profiling of requests. */ -public class EsClient implements Startable { +public class EsClient implements Closeable { public static final Logger LOGGER = Loggers.get("es"); @@ -189,17 +189,12 @@ public class EsClient implements Startable { return (long) max.getValue(); } - @Override - public void start() { - // nothing to do + public Client nativeClient() { + return nativeClient; } @Override - public void stop() { + public void close() { nativeClient.close(); } - - public Client nativeClient() { - return nativeClient; - } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/es/EsClientStopper.java b/server/sonar-server/src/main/java/org/sonar/server/es/EsClientStopper.java new file mode 100644 index 00000000000..a241b17885f --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/es/EsClientStopper.java @@ -0,0 +1,53 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.es; + +import org.sonar.api.Startable; + +/** + * Workaround of a behaviour of picocontainer: components + * instantiated by {@link org.picocontainer.injectors.ProviderAdapter} + * can't have a lifecycle. The methods start() and stop() + * of {@link Startable} are not executed. + * The same behaviour exists for the {@link org.picocontainer.injectors.ProviderAdapter} + * itself. + * + * As {@link EsClientStopper} implements {@link Startable}, it can + * close {@link EsClient} when process shutdowns. + * + */ +public class EsClientStopper implements Startable { + + private final EsClient esClient; + + public EsClientStopper(EsClient esClient) { + this.esClient = esClient; + } + + @Override + public void start() { + // nothing to do + } + + @Override + public void stop() { + esClient.close(); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/search/EsSearchModule.java b/server/sonar-server/src/main/java/org/sonar/server/search/EsSearchModule.java index 2316f75122f..47d0b63dec8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/search/EsSearchModule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/search/EsSearchModule.java @@ -21,10 +21,12 @@ package org.sonar.server.search; import org.sonar.core.platform.Module; import org.sonar.server.es.EsClientProvider; +import org.sonar.server.es.EsClientStopper; public class EsSearchModule extends Module { @Override protected void configureModule() { add(new EsClientProvider()); + add(EsClientStopper.class); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/es/EsClientStopperTest.java b/server/sonar-server/src/test/java/org/sonar/server/es/EsClientStopperTest.java new file mode 100644 index 00000000000..9c6cad0904b --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/es/EsClientStopperTest.java @@ -0,0 +1,42 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.es; + +import org.junit.Test; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + + +public class EsClientStopperTest { + + private EsClient client = mock(EsClient.class); + private EsClientStopper underTest = new EsClientStopper(client); + + @Test + public void stop_client() { + underTest.start(); + verifyZeroInteractions(client); + + underTest.stop(); + verify(client).close(); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/es/EsClientTest.java b/server/sonar-server/src/test/java/org/sonar/server/es/EsClientTest.java index 940236651c2..c4a266efd77 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/es/EsClientTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/es/EsClientTest.java @@ -49,7 +49,6 @@ public class EsClientTest { @Test public void proxify_requests() { EsClient underTest = es.client(); - underTest.start(); assertThat(underTest.nativeClient()).isNotNull(); assertThat(underTest.prepareBulk()).isInstanceOf(ProxyBulkRequestBuilder.class); assertThat(underTest.prepareClusterStats()).isInstanceOf(ProxyClusterStatsRequestBuilder.class); @@ -70,6 +69,6 @@ public class EsClientTest { assertThat(underTest.prepareState()).isInstanceOf(ProxyClusterStateRequestBuilder.class); assertThat(underTest.prepareStats()).isInstanceOf(ProxyIndicesStatsRequestBuilder.class); - underTest.stop(); + underTest.close(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/es/EsTester.java b/server/sonar-server/src/test/java/org/sonar/server/es/EsTester.java index 6265854bc77..147b2c6ed31 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/es/EsTester.java +++ b/server/sonar-server/src/test/java/org/sonar/server/es/EsTester.java @@ -64,7 +64,6 @@ public class EsTester extends ExternalResource { @Override protected void before() throws Throwable { - client.start(); truncateIndices(); if (!indexDefinitions.isEmpty()) { @@ -84,7 +83,7 @@ public class EsTester extends ExternalResource { container.stopComponents(); } if (client != null) { - client.stop(); + client.close(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/search/EsSearchModuleTest.java b/server/sonar-server/src/test/java/org/sonar/server/search/EsSearchModuleTest.java index d2048977b0b..abf95a396c8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/search/EsSearchModuleTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/search/EsSearchModuleTest.java @@ -29,7 +29,7 @@ public class EsSearchModuleTest { public void verify_count_of_added_components() { ComponentContainer container = new ComponentContainer(); new EsSearchModule().configure(container); - assertThat(container.size()).isEqualTo(3); + assertThat(container.size()).isEqualTo(4); } }