This is an automated email from the ASF dual-hosted git repository. krisden pushed a commit to branch branch_9_0 in repository https://gitbox.apache.org/repos/asf/solr.git
commit d0412413e6233615119abd05ad531c38981ec554 Author: Kevin Risden <[email protected]> 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")); + } } }
