You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by kr...@apache.org on 2022/05/05 17:14:06 UTC
[solr] branch main updated: SOLR-16182: Close ZkClientClusterStateProvider in tests to avoid thread leak (#837)
This is an automated email from the ASF dual-hosted git repository.
krisden pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 9ff7f428d05 SOLR-16182: Close ZkClientClusterStateProvider in tests to avoid thread leak (#837)
9ff7f428d05 is described below
commit 9ff7f428d0542190a0a450840505eac11d65e4d1
Author: Kevin Risden <ri...@users.noreply.github.com>
AuthorDate: Thu May 5 13:14:00 2022 -0400
SOLR-16182: Close ZkClientClusterStateProvider in tests to avoid thread leak (#837)
---
.../impl/CloudHttp2SolrClientBuilderTest.java | 25 +++++++++++-----
.../CloudHttp2SolrClientMultiConstructorTest.java | 5 +++-
.../solrj/impl/CloudHttp2SolrClientTest.java | 31 ++++++++++---------
.../solrj/impl/CloudSolrClientBuilderTest.java | 35 ++++++++++++++--------
.../impl/CloudSolrClientMultiConstructorTest.java | 10 +++++--
.../client/solrj/impl/CloudSolrClientTest.java | 33 ++++++++++++--------
6 files changed, 87 insertions(+), 52 deletions(-)
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java
index dd5b8a96df4..85062638463 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java
@@ -43,9 +43,12 @@ public class CloudHttp2SolrClientBuilderTest extends SolrTestCase {
new CloudHttp2SolrClient.Builder(
Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT))
.build()) {
- final String clientZkHost = ZkClientClusterStateProvider.from(createdClient).getZkHost();
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(createdClient)) {
+ final String clientZkHost = zkClientClusterStateProvider.getZkHost();
- assertTrue(clientZkHost.contains(ANY_ZK_HOST));
+ assertTrue(clientZkHost.contains(ANY_ZK_HOST));
+ }
}
}
@@ -56,10 +59,13 @@ public class CloudHttp2SolrClientBuilderTest extends SolrTestCase {
zkHostList.add(ANY_OTHER_ZK_HOST);
try (CloudHttp2SolrClient createdClient =
new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) {
- final String clientZkHost = ZkClientClusterStateProvider.from(createdClient).getZkHost();
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(createdClient)) {
+ final String clientZkHost = zkClientClusterStateProvider.getZkHost();
- assertTrue(clientZkHost.contains(ANY_ZK_HOST));
- assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST));
+ assertTrue(clientZkHost.contains(ANY_ZK_HOST));
+ assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST));
+ }
}
}
@@ -70,10 +76,13 @@ public class CloudHttp2SolrClientBuilderTest extends SolrTestCase {
zkHosts.add(ANY_OTHER_ZK_HOST);
try (CloudHttp2SolrClient createdClient =
new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) {
- final String clientZkHost = ZkClientClusterStateProvider.from(createdClient).getZkHost();
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(createdClient)) {
+ final String clientZkHost = zkClientClusterStateProvider.getZkHost();
- assertTrue(clientZkHost.contains(ANY_ZK_HOST));
- assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST));
+ assertTrue(clientZkHost.contains(ANY_ZK_HOST));
+ assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST));
+ }
}
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientMultiConstructorTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientMultiConstructorTest.java
index 5ee6acc2f35..2eb6b4a00a6 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientMultiConstructorTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientMultiConstructorTest.java
@@ -71,7 +71,10 @@ public class CloudHttp2SolrClientMultiConstructorTest extends SolrTestCase {
try (CloudHttp2SolrClient client =
new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot))
.build()) {
- assertEquals(sb.toString(), ZkClientClusterStateProvider.from(client).getZkHost());
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(client)) {
+ assertEquals(sb.toString(), zkClientClusterStateProvider.getZkHost());
+ }
}
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java
index e54f3e8d953..cd023603aa6 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java
@@ -75,9 +75,7 @@ import org.apache.solr.handler.admin.CoreAdminHandler;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.Rule;
import org.junit.Test;
-import org.junit.rules.ExpectedException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -824,27 +822,28 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase {
@Test
public void testShutdown() throws IOException {
try (CloudSolrClient client = getCloudSolrClient(DEAD_HOST_1)) {
- ZkClientClusterStateProvider.from(client).setZkConnectTimeout(100);
- client.connect();
- fail("Expected exception");
- } catch (SolrException e) {
- assertTrue(e.getCause() instanceof TimeoutException);
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(client)) {
+ zkClientClusterStateProvider.setZkConnectTimeout(100);
+ SolrException e = assertThrows(SolrException.class, client::connect);
+ assertTrue(e.getCause() instanceof TimeoutException);
+ }
}
}
- @Rule public ExpectedException exception = ExpectedException.none();
-
@Test
public void testWrongZkChrootTest() throws IOException {
- exception.expect(SolrException.class);
- exception.expectMessage("cluster not found/not ready");
- exception.expectMessage("Expected node '" + ZkStateReader.ALIASES + "' does not exist");
-
try (CloudSolrClient client =
getCloudSolrClient(cluster.getZkServer().getZkAddress() + "/xyz/foo")) {
- ZkClientClusterStateProvider.from(client).setZkClientTimeout(1000 * 60);
- client.connect();
- fail("Expected exception");
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(client)) {
+ zkClientClusterStateProvider.setZkClientTimeout(1000 * 60);
+ SolrException e = assertThrows(SolrException.class, client::connect);
+ assertTrue(e.getMessage().contains("cluster not found/not ready"));
+ assertTrue(
+ e.getMessage()
+ .contains("Expected node '" + ZkStateReader.ALIASES + "' does not exist"));
+ }
}
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientBuilderTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientBuilderTest.java
index 60e74ff0ee3..d4d90ae211e 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientBuilderTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientBuilderTest.java
@@ -34,9 +34,12 @@ public class CloudSolrClientBuilderTest extends SolrTestCase {
@Test
public void testSingleZkHostSpecified() throws IOException {
try (CloudSolrClient createdClient =
- new Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) {
- final String clientZkHost = ZkClientClusterStateProvider.from(createdClient).getZkHost();
- assertTrue(clientZkHost.contains(ANY_ZK_HOST));
+ new Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build(); ) {
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(createdClient)) {
+ final String clientZkHost = zkClientClusterStateProvider.getZkHost();
+ assertTrue(clientZkHost.contains(ANY_ZK_HOST));
+ }
}
}
@@ -46,9 +49,12 @@ public class CloudSolrClientBuilderTest extends SolrTestCase {
zkHostList.add(ANY_ZK_HOST);
zkHostList.add(ANY_OTHER_ZK_HOST);
try (CloudSolrClient createdClient = new Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) {
- final String clientZkHost = ZkClientClusterStateProvider.from(createdClient).getZkHost();
- assertTrue(clientZkHost.contains(ANY_ZK_HOST));
- assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST));
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(createdClient)) {
+ final String clientZkHost = zkClientClusterStateProvider.getZkHost();
+ assertTrue(clientZkHost.contains(ANY_ZK_HOST));
+ assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST));
+ }
}
}
@@ -58,9 +64,12 @@ public class CloudSolrClientBuilderTest extends SolrTestCase {
zkHosts.add(ANY_ZK_HOST);
zkHosts.add(ANY_OTHER_ZK_HOST);
try (CloudSolrClient createdClient = new Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) {
- final String clientZkHost = ZkClientClusterStateProvider.from(createdClient).getZkHost();
- assertTrue(clientZkHost.contains(ANY_ZK_HOST));
- assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST));
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(createdClient)) {
+ final String clientZkHost = zkClientClusterStateProvider.getZkHost();
+ assertTrue(clientZkHost.contains(ANY_ZK_HOST));
+ assertTrue(clientZkHost.contains(ANY_OTHER_ZK_HOST));
+ }
}
}
@@ -68,7 +77,7 @@ public class CloudSolrClientBuilderTest extends SolrTestCase {
public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throws IOException {
try (CloudSolrClient createdClient =
new Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) {
- assertTrue(createdClient.isUpdatesToLeaders() == true);
+ assertTrue(createdClient.isUpdatesToLeaders());
}
}
@@ -83,10 +92,12 @@ public class CloudSolrClientBuilderTest extends SolrTestCase {
@Test
@SuppressWarnings({"try"})
public void test0Timeouts() throws IOException {
- try (var createdClient =
+ try (CloudSolrClient createdClient =
new CloudLegacySolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.empty())
.withSocketTimeout(0)
.withConnectionTimeout(0)
- .build()) {}
+ .build()) {
+ assertNotNull(createdClient);
+ }
}
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientMultiConstructorTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientMultiConstructorTest.java
index 37abe2021a8..43a6cce0a44 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientMultiConstructorTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientMultiConstructorTest.java
@@ -71,7 +71,10 @@ public class CloudSolrClientMultiConstructorTest extends SolrTestCase {
try (CloudSolrClient client =
(new CloudSolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot))
.build())) {
- assertEquals(sb.toString(), ZkClientClusterStateProvider.from(client).getZkHost());
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(client)) {
+ assertEquals(sb.toString(), zkClientClusterStateProvider.getZkHost());
+ }
}
}
@@ -99,7 +102,10 @@ public class CloudSolrClientMultiConstructorTest extends SolrTestCase {
final Optional<String> chrootOption =
withChroot == false ? Optional.empty() : Optional.of(chroot);
try (var client = new CloudLegacySolrClient.Builder(hosts, chrootOption).build()) {
- assertEquals(sb.toString(), ZkClientClusterStateProvider.from(client).getZkHost());
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(client)) {
+ assertEquals(sb.toString(), zkClientClusterStateProvider.getZkHost());
+ }
}
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
index d0318b95ccb..c4036a42c74 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
@@ -826,9 +826,12 @@ public class CloudSolrClientTest extends SolrCloudTestCase {
@Test
public void testShutdown() throws IOException {
try (CloudSolrClient client = getCloudSolrClient(DEAD_HOST_1)) {
- ZkClientClusterStateProvider.from(client).setZkConnectTimeout(100);
- SolrException ex = expectThrows(SolrException.class, client::connect);
- assertTrue(ex.getCause() instanceof TimeoutException);
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(client)) {
+ zkClientClusterStateProvider.setZkConnectTimeout(100);
+ SolrException ex = expectThrows(SolrException.class, client::connect);
+ assertTrue(ex.getCause() instanceof TimeoutException);
+ }
}
}
@@ -838,16 +841,20 @@ public class CloudSolrClientTest extends SolrCloudTestCase {
public void testWrongZkChrootTest() throws IOException {
try (CloudSolrClient client =
getCloudSolrClient(cluster.getZkServer().getZkAddress() + "/xyz/foo")) {
- ZkClientClusterStateProvider.from(client).setZkConnectTimeout(1000 * 60);
- SolrException ex = expectThrows(SolrException.class, client::connect);
- MatcherAssert.assertThat(
- "Wrong error message for empty chRoot",
- ex.getMessage(),
- Matchers.containsString("cluster not found/not ready"));
- MatcherAssert.assertThat(
- "Wrong node missing message for empty chRoot",
- ex.getMessage(),
- Matchers.containsString("Expected node '" + ZkStateReader.ALIASES + "' does not exist"));
+ try (ZkClientClusterStateProvider zkClientClusterStateProvider =
+ ZkClientClusterStateProvider.from(client)) {
+ zkClientClusterStateProvider.setZkConnectTimeout(1000 * 60);
+ SolrException ex = expectThrows(SolrException.class, client::connect);
+ MatcherAssert.assertThat(
+ "Wrong error message for empty chRoot",
+ ex.getMessage(),
+ Matchers.containsString("cluster not found/not ready"));
+ MatcherAssert.assertThat(
+ "Wrong node missing message for empty chRoot",
+ ex.getMessage(),
+ Matchers.containsString(
+ "Expected node '" + ZkStateReader.ALIASES + "' does not exist"));
+ }
}
}