From 17ba151c52e4baaa2a0a8941314f31b5b4f36edb Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Mon, 13 Oct 2025 12:00:50 -0500 Subject: [PATCH 01/27] Get rid of redundant Builder class on abstract CloudSolrClient --- .../org/apache/solr/cli/RunExampleTool.java | 3 +- .../consumer/KafkaCrossDcConsumer.java | 3 +- .../UsingSolrJRefGuideExamplesTest.java | 7 ++- .../client/solrj/impl/CloudSolrClient.java | 62 ------------------- .../impl/CloudHttp2SolrClientBuilderTest.java | 2 +- .../impl/CloudSolrClientBuilderTest.java | 13 ++-- .../CloudSolrClientMultiConstructorTest.java | 4 +- .../solrj/impl/HttpClusterStateSSLTest.java | 2 +- .../cloud/AbstractFullDistribZkTestBase.java | 2 +- 9 files changed, 19 insertions(+), 79 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java index 06547d4eb199..fd530bb28546 100644 --- a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java +++ b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java @@ -44,6 +44,7 @@ import org.apache.commons.exec.environment.EnvironmentUtils; import org.apache.commons.io.file.PathUtils; import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.common.SolrException; import org.apache.solr.common.util.EnvUtils; @@ -629,7 +630,7 @@ protected void runCloudExample(CommandLine cli) throws Exception { /** wait until the number of live nodes == numNodes. */ protected void waitToSeeLiveNodes(String zkHost, int numNodes) { try (CloudSolrClient cloudClient = - new CloudSolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()).build()) { cloudClient.connect(); Set liveNodes = cloudClient.getClusterState().getLiveNodes(); int numLiveNodes = (liveNodes != null) ? liveNodes.size() : 0; diff --git a/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java b/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java index bf1ba691f263..7e6e9f1bd654 100644 --- a/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java +++ b/solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/KafkaCrossDcConsumer.java @@ -43,6 +43,7 @@ import org.apache.kafka.common.serialization.StringDeserializer; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrResponse; +import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.common.SolrInputDocument; @@ -121,7 +122,7 @@ public void close() { protected CloudSolrClient createSolrClient() { log.debug("Creating new SolrClient..."); - return new CloudSolrClient.Builder( + return new CloudHttp2SolrClient.Builder( Collections.singletonList(zkConnectString), Optional.empty()) .build(); } diff --git a/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java b/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java index 5cf438775a1b..ad60010c125e 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java +++ b/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java @@ -31,6 +31,7 @@ import org.apache.solr.client.solrj.SolrQuery.ORDER; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.beans.Field; +import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; @@ -254,7 +255,7 @@ private SolrClient getBaseURLCloudSolrClient() { final List solrUrls = new ArrayList<>(); solrUrls.add("http://solr1:8983/solr"); solrUrls.add("http://solr2:8983/solr"); - return new CloudSolrClient.Builder(solrUrls).build(); + return new CloudHttp2SolrClient.Builder(solrUrls).build(); // end::solrj-cloudsolrclient-baseurl[] } @@ -264,7 +265,7 @@ private SolrClient getZookeeperNoRootCloudSolrClient() { zkServers.add("zookeeper1:2181"); zkServers.add("zookeeper2:2181"); zkServers.add("zookeeper3:2181"); - return new CloudSolrClient.Builder(zkServers, Optional.empty()).build(); + return new CloudHttp2SolrClient.Builder(zkServers, Optional.empty()).build(); // end::solrj-cloudsolrclient-zookeepernoroot[] } @@ -274,7 +275,7 @@ private SolrClient getZookeeperRootCloudSolrClient() { zkServers.add("zookeeper1:2181"); zkServers.add("zookeeper2:2181"); zkServers.add("zookeeper3:2181"); - return new CloudSolrClient.Builder(zkServers, Optional.of("/solr")).build(); + return new CloudHttp2SolrClient.Builder(zkServers, Optional.of("/solr")).build(); // end::solrj-cloudsolrclient-zookeeperroot[] } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 647071ef0bf8..b75bee952f07 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -120,68 +120,6 @@ public abstract class CloudSolrClient extends SolrClient { protected volatile Object[] locks = objectList(3); - /** Constructs {@link CloudSolrClient} instances from provided configuration. */ - public static class Builder extends CloudHttp2SolrClient.Builder { - - /** - * Provide a series of Solr URLs to be used when configuring {@link CloudSolrClient} instances. - * The solr client will use these urls to understand the cluster topology, which solr nodes are - * active etc. - * - *

Provided Solr URLs are expected to point to the root Solr path - * ("http://hostname:8983/solr"); they should not include any collections, cores, or other path - * components. - * - *

Usage example: - * - * 

-     *   final List<String> solrBaseUrls = new ArrayList<String>();
-     *   solrBaseUrls.add("http://solr1:8983/solr"); solrBaseUrls.add("http://solr2:8983/solr"); solrBaseUrls.add("http://solr3:8983/solr");
-     *   final SolrClient client = new CloudSolrClient.Builder(solrBaseUrls).build();
-     *
- */ - public Builder(List solrUrls) { - super(solrUrls); - } - - /** - * Provide a series of ZK hosts which will be used when configuring {@link CloudSolrClient} - * instances. This requires a dependency on {@code solr-solrj-zookeeper} which transitively - * depends on more JARs. The ZooKeeper based connection is the most reliable and performant - * means for CloudSolrClient to work. On the other hand, it means exposing ZooKeeper more - * broadly than to Solr nodes, which is a security risk. - * - *

Usage example when Solr stores data at the ZooKeeper root ('/'): - * - * 

-     *   final List<String> zkServers = new ArrayList<String>();
-     *   zkServers.add("zookeeper1:2181"); zkServers.add("zookeeper2:2181"); zkServers.add("zookeeper3:2181");
-     *   final SolrClient client = new CloudSolrClient.Builder(zkServers, Optional.empty()).build();
-     *
- * - * Usage example when Solr data is stored in a ZooKeeper chroot: - * - * 
-     *    final List<String> zkServers = new ArrayList<String>();
-     *    zkServers.add("zookeeper1:2181"); zkServers.add("zookeeper2:2181"); zkServers.add("zookeeper3:2181");
-     *    final SolrClient client = new CloudSolrClient.Builder(zkServers, Optional.of("/solr")).build();
-     *
- * - * @param zkHosts a List of at least one ZooKeeper host and port (e.g. "zookeeper1:2181") - * @param zkChroot the path to the root ZooKeeper node containing Solr data. Provide {@code - * java.util.Optional.empty()} if no ZK chroot is used. - */ - @Deprecated(since = "10.0") - public Builder(List zkHosts, Optional zkChroot) { - super(zkHosts, zkChroot); - } - - /** for an expert use-case */ - public Builder(ClusterStateProvider stateProvider) { - super(stateProvider); - } - } - static class StateCache extends ConcurrentHashMap { final AtomicLong puts = new AtomicLong(); final AtomicLong hits = new AtomicLong(); 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 2530beb34a09..ac07fb120284 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 @@ -230,7 +230,7 @@ private void testHttpClientConsistency( public void testCustomStateProvider() throws IOException { ZkClientClusterStateProvider stateProvider = mock(ZkClientClusterStateProvider.class); - try (CloudSolrClient cloudSolrClient = new CloudSolrClient.Builder(stateProvider).build()) { + try (CloudSolrClient cloudSolrClient = new CloudHttp2SolrClient.Builder(stateProvider).build()) { assertEquals(stateProvider, cloudSolrClient.getClusterStateProvider()); } verify(stateProvider, times(1)).close(); 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 348e8e1d65ec..4089faa454e9 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 @@ -24,7 +24,6 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; import org.apache.solr.SolrTestCase; -import org.apache.solr.client.solrj.impl.CloudSolrClient.Builder; import org.junit.Test; public class CloudSolrClientBuilderTest extends SolrTestCase { @@ -35,7 +34,7 @@ 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(); ) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build(); ) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -49,7 +48,7 @@ public void testSeveralZkHostsSpecifiedSingly() throws IOException { final List zkHostList = new ArrayList<>(); zkHostList.add(ANY_ZK_HOST); zkHostList.add(ANY_OTHER_ZK_HOST); - try (CloudSolrClient createdClient = new Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { + try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -64,7 +63,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { final ArrayList zkHosts = new ArrayList<>(); zkHosts.add(ANY_ZK_HOST); zkHosts.add(ANY_OTHER_ZK_HOST); - try (CloudSolrClient createdClient = new Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { + try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -77,7 +76,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { @Test public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throws IOException { try (CloudSolrClient createdClient = - new Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { assertTrue(createdClient.isUpdatesToLeaders()); } } @@ -85,7 +84,7 @@ public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throw @Test public void testIsDirectUpdatesToLeadersOnlyDefault() throws IOException { try (CloudSolrClient createdClient = - new Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { assertFalse(createdClient.isDirectUpdatesToLeadersOnly()); } } @@ -105,7 +104,7 @@ public void test0Timeouts() throws IOException { @Test public void testDefaultCollectionPassedFromBuilderToClient() throws IOException { try (CloudSolrClient createdClient = - new Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withDefaultCollection("aCollection") .build()) { assertEquals("aCollection", createdClient.getDefaultCollection()); 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 84c315637f30..dc540e496937 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 @@ -69,7 +69,7 @@ public void testZkConnectionStringSetterWithValidChroot() throws IOException { } try (CloudSolrClient client = - (new CloudSolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) + (new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) .build())) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(client)) { @@ -113,6 +113,6 @@ public void testZkConnectionStringConstructorWithValidChroot() throws IOExceptio public void testBadChroot() { final List zkHosts = new ArrayList<>(); zkHosts.add("host1:2181"); - new CloudSolrClient.Builder(zkHosts, Optional.of("foo")).build(); + new CloudHttp2SolrClient.Builder(zkHosts, Optional.of("foo")).build(); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java index 24e005c28c73..d160a5d43da4 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java @@ -83,7 +83,7 @@ public void testHttpClusterStateWithSSL() throws Exception { // verify the http derived cluster state (on the client side) agrees with what the server stored try (CloudSolrClient httpBasedCloudSolrClient = - new CloudSolrClient.Builder(Collections.singletonList(url0.toExternalForm())).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())).build()) { ClusterStateProvider csp = httpBasedCloudSolrClient.getClusterStateProvider(); assertTrue(csp instanceof Http2ClusterStateProvider); verifyUrlSchemeInClusterState(csp.getCollection(collectionId), expectedReplicas); diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java index 70fdbc57a87a..21e5c954d6fb 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java @@ -2154,7 +2154,7 @@ ZkStateReader.PULL_REPLICAS, getPullReplicaCount(), /** * This method may randomize unspecified aspects of the resulting SolrClient. Tests that do * not wish to have any randomized behavior should use the {@link - * org.apache.solr.client.solrj.impl.CloudSolrClient.Builder} class directly + * org.apache.solr.client.solrj.impl.CloudHttp2SolrClient.Builder} class directly */ public static CloudSolrClient getCloudSolrClient( String zkHost, From b5c6c996bdefdf1e40703e991bcd8200f62770a3 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Mon, 13 Oct 2025 14:29:17 -0500 Subject: [PATCH 02/27] Make CloudHttp2SolrClient generic; update references to remove compiler warnings --- .../java/org/apache/solr/cli/CLIUtils.java | 6 +- .../java/org/apache/solr/cli/ExportTool.java | 2 +- .../org/apache/solr/cli/HealthcheckTool.java | 3 +- .../org/apache/solr/cli/RunExampleTool.java | 3 +- .../org/apache/solr/cloud/ZkController.java | 5 +- .../solr/client/solrj/io/SolrClientCache.java | 4 +- .../solrj/impl/SolrClientCloudManager.java | 4 +- .../impl/SolrClientNodeStateProvider.java | 8 +- .../solrj/impl/CloudHttp2SolrClient.java | 75 ++++++++++--------- .../impl/CloudHttp2SolrClientBuilderTest.java | 73 +++++++++++++----- ...udHttp2SolrClientMultiConstructorTest.java | 4 +- .../impl/CloudHttp2SolrClientRetryTest.java | 2 +- .../solrj/impl/CloudHttp2SolrClientTest.java | 6 +- .../impl/CloudSolrClientBuilderTest.java | 12 +-- .../CloudSolrClientMultiConstructorTest.java | 4 +- .../solrj/impl/HttpClusterStateSSLTest.java | 4 +- .../SendUpdatesToLeadersOverrideTest.java | 6 +- .../java/org/apache/solr/SolrTestCaseJ4.java | 2 +- 18 files changed, 130 insertions(+), 93 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cli/CLIUtils.java b/solr/core/src/java/org/apache/solr/cli/CLIUtils.java index c04e02fc1602..fccbd4a84b33 100644 --- a/solr/core/src/java/org/apache/solr/cli/CLIUtils.java +++ b/solr/core/src/java/org/apache/solr/cli/CLIUtils.java @@ -261,13 +261,13 @@ public static SolrZkClient getSolrZkClient(CommandLine cli, String zkHost) throw .build(); } - public static CloudHttp2SolrClient getCloudHttp2SolrClient(String zkHost) { + public static CloudHttp2SolrClient getCloudHttp2SolrClient(String zkHost) { return getCloudHttp2SolrClient(zkHost, null); } - public static CloudHttp2SolrClient getCloudHttp2SolrClient( + public static CloudHttp2SolrClient getCloudHttp2SolrClient( String zkHost, Http2SolrClient.Builder builder) { - return new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()) + return new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()) .withHttpClientBuilder(builder) .build(); } diff --git a/solr/core/src/java/org/apache/solr/cli/ExportTool.java b/solr/core/src/java/org/apache/solr/cli/ExportTool.java index af2cf6ba590e..e23925635633 100644 --- a/solr/core/src/java/org/apache/solr/cli/ExportTool.java +++ b/solr/core/src/java/org/apache/solr/cli/ExportTool.java @@ -256,7 +256,7 @@ void fetchUniqueKey() throws SolrServerException, IOException { new Http2SolrClient.Builder().withOptionalBasicAuthCredentials(credentials); solrClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(baseurl)) + new CloudHttp2SolrClient.Builder(Collections.singletonList(baseurl)) .withHttpClientBuilder(builder) .build(); NamedList response = diff --git a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java index bf795a515965..c17f85c571d9 100644 --- a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java +++ b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java @@ -34,6 +34,7 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.client.solrj.request.GenericSolrRequest; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.cloud.ClusterState; @@ -88,7 +89,7 @@ public void runImpl(CommandLine cli) throws Exception { CLIO.err("Healthcheck tool only works in Solr Cloud mode."); runtime.exit(1); } - try (CloudHttp2SolrClient cloudSolrClient = CLIUtils.getCloudHttp2SolrClient(zkHost)) { + try (CloudHttp2SolrClient cloudSolrClient = CLIUtils.getCloudHttp2SolrClient(zkHost)) { echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ..."); cloudSolrClient.connect(); runCloudTool(cloudSolrClient, cli); diff --git a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java index fd530bb28546..4e15d459f997 100644 --- a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java +++ b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java @@ -46,6 +46,7 @@ import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.common.SolrException; import org.apache.solr.common.util.EnvUtils; import org.noggit.CharArr; @@ -630,7 +631,7 @@ protected void runCloudExample(CommandLine cli) throws Exception { /** wait until the number of live nodes == numNodes. */ protected void waitToSeeLiveNodes(String zkHost, int numNodes) { try (CloudSolrClient cloudClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()).build()) { cloudClient.connect(); Set liveNodes = cloudClient.getClusterState().getLiveNodes(); int numLiveNodes = (liveNodes != null) ? liveNodes.size() : 0; diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index bc41a64136ed..f6be7f9a18aa 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -62,6 +62,7 @@ import org.apache.solr.client.solrj.cloud.SolrCloudManager; import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder; import org.apache.solr.client.solrj.impl.SolrClientCloudManager; import org.apache.solr.client.solrj.impl.SolrZkClientTimeout; @@ -206,7 +207,7 @@ public String toString() { public final ZkStateReader zkStateReader; private SolrCloudManager cloudManager; - private CloudHttp2SolrClient cloudSolrClient; + private CloudHttp2SolrClient cloudSolrClient; private final ExecutorService zkConnectionListenerCallbackExecutor = ExecutorUtil.newMDCAwareSingleThreadExecutor( @@ -962,7 +963,7 @@ public SolrCloudManager getSolrCloudManager() { return cloudManager; } cloudSolrClient = - new CloudHttp2SolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader)) + new CloudHttp2SolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader)) .withHttpClient(cc.getDefaultHttpSolrClient()) .build(); cloudManager = new SolrClientCloudManager(cloudSolrClient, cc.getObjectCache()); diff --git a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java index ec94c7d5aa9a..6138a6be7fad 100644 --- a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java +++ b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java @@ -96,13 +96,13 @@ public synchronized CloudSolrClient getCloudSolrClient(String zkHost) { return client; } - private static CloudHttp2SolrClient newCloudHttp2SolrClient( + private static CloudHttp2SolrClient newCloudHttp2SolrClient( String zkHost, Http2SolrClient http2SolrClient, boolean canUseACLs, String basicAuthCredentials) { final List hosts = List.of(zkHost); - var builder = new CloudHttp2SolrClient.Builder(hosts, Optional.empty()); + var builder = new CloudHttp2SolrClient.Builder(hosts, Optional.empty()); builder.canUseZkACLs(canUseACLs); // using internal builder to ensure the internal client gets closed builder = diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientCloudManager.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientCloudManager.java index 30afd6bed570..0efd116869a6 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientCloudManager.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientCloudManager.java @@ -33,7 +33,7 @@ public class SolrClientCloudManager implements SolrCloudManager { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private final CloudHttp2SolrClient cloudSolrClient; + private final CloudHttp2SolrClient cloudSolrClient; private final ZkDistribStateManager stateManager; private final ZkStateReader zkStateReader; private final SolrZkClient zkClient; @@ -41,7 +41,7 @@ public class SolrClientCloudManager implements SolrCloudManager { private final boolean closeObjectCache; private volatile boolean isClosed; - public SolrClientCloudManager(CloudHttp2SolrClient client, ObjectCache objectCache) { + public SolrClientCloudManager(CloudHttp2SolrClient client, ObjectCache objectCache) { this.cloudSolrClient = client; this.zkStateReader = ZkStateReader.from(client); this.zkClient = zkStateReader.getZkClient(); diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java index e3b3e9cbcf57..51ab3540e00a 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java @@ -52,14 +52,14 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private final CloudHttp2SolrClient solrClient; + private final CloudHttp2SolrClient solrClient; protected final Map>>> nodeVsCollectionVsShardVsReplicaInfo = new HashMap<>(); @SuppressWarnings({"rawtypes"}) private Map nodeVsTags = new HashMap<>(); - public SolrClientNodeStateProvider(CloudHttp2SolrClient solrClient) { + public SolrClientNodeStateProvider(CloudHttp2SolrClient solrClient) { this.solrClient = solrClient; try { readReplicaDetails(); @@ -222,7 +222,7 @@ public String toString() { static class RemoteCallCtx { ZkClientClusterStateProvider zkClientClusterStateProvider; - CloudHttp2SolrClient cloudSolrClient; + CloudHttp2SolrClient cloudSolrClient; public final Map tags = new HashMap<>(); private final String node; public Map session; @@ -234,7 +234,7 @@ public boolean isNodeAlive(String node) { return true; } - public RemoteCallCtx(String node, CloudHttp2SolrClient cloudSolrClient) { + public RemoteCallCtx(String node, CloudHttp2SolrClient cloudSolrClient) { this.node = node; this.cloudSolrClient = cloudSolrClient; this.zkClientClusterStateProvider = diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index 028d8b675c58..e4fbe3f7941c 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -37,11 +37,11 @@ * @since solr 8.0 */ @SuppressWarnings("serial") -public class CloudHttp2SolrClient extends CloudSolrClient { +public class CloudHttp2SolrClient extends CloudSolrClient { private final ClusterStateProvider stateProvider; - private final LBHttp2SolrClient lbClient; - private final Http2SolrClient myClient; + private final LBHttp2SolrClient lbClient; + private final C myClient; private final boolean clientIsInternal; /** @@ -52,7 +52,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient { * * @param builder a {@link Http2SolrClient.Builder} with the options used to create the client. */ - protected CloudHttp2SolrClient(Builder builder) { + protected CloudHttp2SolrClient(Builder builder) { super(builder.shardLeadersOnly, builder.parallelUpdates, builder.directUpdatesToLeadersOnly); this.clientIsInternal = builder.httpClient == null; this.myClient = createOrGetHttpClientFromBuilder(builder); @@ -73,20 +73,20 @@ protected CloudHttp2SolrClient(Builder builder) { // locks. this.locks = objectList(builder.parallelCacheRefreshesLocks); - this.lbClient = new LBHttp2SolrClient.Builder(myClient).build(); + this.lbClient = new LBHttp2SolrClient.Builder<>(myClient).build(); } - - private Http2SolrClient createOrGetHttpClientFromBuilder(Builder builder) { + @SuppressWarnings("unchecked") + private C createOrGetHttpClientFromBuilder(Builder builder) { if (builder.httpClient != null) { - return builder.httpClient; + return (C) builder.httpClient; } else if (builder.internalClientBuilder != null) { - return builder.internalClientBuilder.build(); + return (C) builder.internalClientBuilder.build(); } else { - return new Http2SolrClient.Builder().build(); + return (C) new Http2SolrClient.Builder().build(); } } - private ClusterStateProvider createClusterStateProvider(Builder builder) { + private ClusterStateProvider createClusterStateProvider(Builder builder) { if (builder.stateProvider != null) { return builder.stateProvider; } else if (builder.zkHosts.isEmpty()) { @@ -96,7 +96,7 @@ private ClusterStateProvider createClusterStateProvider(Builder builder) { } } - private ClusterStateProvider createZkClusterStateProvider(Builder builder) { + private ClusterStateProvider createZkClusterStateProvider(Builder builder) { try { ClusterStateProvider stateProvider = ClusterStateProvider.newZkClusterStateProvider( @@ -113,7 +113,7 @@ private ClusterStateProvider createZkClusterStateProvider(Builder builder) { } private ClusterStateProvider createHttp2ClusterStateProvider( - List solrUrls, Http2SolrClient httpClient) { + List solrUrls, C httpClient) { try { return new Http2ClusterStateProvider<>(solrUrls, httpClient); } catch (Exception e) { @@ -148,7 +148,7 @@ public void close() throws IOException { } @Override - public LBHttp2SolrClient getLbClient() { + public LBHttp2SolrClient getLbClient() { return lbClient; } @@ -157,7 +157,7 @@ public ClusterStateProvider getClusterStateProvider() { return stateProvider; } - public Http2SolrClient getHttpClient() { + public C getHttpClient() { return myClient; } @@ -167,16 +167,16 @@ protected boolean wasCommError(Throwable rootCause) { } /** Constructs {@link CloudHttp2SolrClient} instances from provided configuration. */ - public static class Builder { + public static class Builder, C extends HttpSolrClientBase> { protected Collection zkHosts = new ArrayList<>(); protected List solrUrls = new ArrayList<>(); protected String zkChroot; - protected Http2SolrClient httpClient; + protected C httpClient; protected boolean shardLeadersOnly = true; protected boolean directUpdatesToLeadersOnly = false; protected boolean parallelUpdates = true; protected ClusterStateProvider stateProvider; - protected Http2SolrClient.Builder internalClientBuilder; + protected B internalClientBuilder; private RequestWriter requestWriter; private ResponseParser responseParser; private long retryExpiryTimeNano = @@ -206,6 +206,7 @@ public static class Builder { * final SolrClient client = new CloudHttp2SolrClient.Builder(solrBaseUrls).build(); * */ + @SuppressWarnings({"rawtypes"}) public Builder(List solrUrls) { this.solrUrls = solrUrls; } @@ -245,7 +246,7 @@ public Builder(ClusterStateProvider stateProvider) { } /** Whether to use the default ZK ACLs when building a ZK Client. */ - public Builder canUseZkACLs(boolean canUseZkACLs) { + public Builder canUseZkACLs(boolean canUseZkACLs) { this.canUseZkACLs = canUseZkACLs; return this; } @@ -257,7 +258,7 @@ public Builder canUseZkACLs(boolean canUseZkACLs) { * @see #sendUpdatesToAnyReplica * @see CloudSolrClient#isUpdatesToLeaders */ - public Builder sendUpdatesOnlyToShardLeaders() { + public Builder sendUpdatesOnlyToShardLeaders() { shardLeadersOnly = true; return this; } @@ -269,7 +270,7 @@ public Builder sendUpdatesOnlyToShardLeaders() { * @see #sendUpdatesOnlyToShardLeaders * @see CloudSolrClient#isUpdatesToLeaders */ - public Builder sendUpdatesToAnyReplica() { + public Builder sendUpdatesToAnyReplica() { shardLeadersOnly = false; return this; } @@ -284,7 +285,7 @@ public Builder sendUpdatesToAnyReplica() { * @see #sendDirectUpdatesToAnyShardReplica * @see CloudSolrClient#isDirectUpdatesToLeadersOnly */ - public Builder sendDirectUpdatesToShardLeadersOnly() { + public Builder sendDirectUpdatesToShardLeadersOnly() { directUpdatesToLeadersOnly = true; return this; } @@ -299,19 +300,19 @@ public Builder sendDirectUpdatesToShardLeadersOnly() { * @see #sendDirectUpdatesToShardLeadersOnly * @see CloudSolrClient#isDirectUpdatesToLeadersOnly */ - public Builder sendDirectUpdatesToAnyShardReplica() { + public Builder sendDirectUpdatesToAnyShardReplica() { directUpdatesToLeadersOnly = false; return this; } /** Provides a {@link RequestWriter} for created clients to use when handing requests. */ - public Builder withRequestWriter(RequestWriter requestWriter) { + public Builder withRequestWriter(RequestWriter requestWriter) { this.requestWriter = requestWriter; return this; } /** Provides a {@link ResponseParser} for created clients to use when handling requests. */ - public Builder withResponseParser(ResponseParser responseParser) { + public Builder withResponseParser(ResponseParser responseParser) { this.responseParser = responseParser; return this; } @@ -326,7 +327,7 @@ public Builder withResponseParser(ResponseParser responseParser) { * *

If not set, this defaults to 'true' and sends sub-requests in parallel. */ - public Builder withParallelUpdates(boolean parallelUpdates) { + public Builder withParallelUpdates(boolean parallelUpdates) { this.parallelUpdates = parallelUpdates; return this; } @@ -337,7 +338,7 @@ public Builder withParallelUpdates(boolean parallelUpdates) { * *

Defaults to 3. */ - public Builder withParallelCacheRefreshes(int parallelCacheRefreshesLocks) { + public Builder withParallelCacheRefreshes(int parallelCacheRefreshesLocks) { this.parallelCacheRefreshesLocks = parallelCacheRefreshesLocks; return this; } @@ -345,13 +346,13 @@ public Builder withParallelCacheRefreshes(int parallelCacheRefreshesLocks) { /** * This is the time to wait to re-fetch the state after getting the same state version from ZK */ - public Builder withRetryExpiryTime(long expiryTime, TimeUnit unit) { + public Builder withRetryExpiryTime(long expiryTime, TimeUnit unit) { this.retryExpiryTimeNano = TimeUnit.NANOSECONDS.convert(expiryTime, unit); return this; } /** Sets the default collection for request. */ - public Builder withDefaultCollection(String defaultCollection) { + public Builder withDefaultCollection(String defaultCollection) { this.defaultCollection = defaultCollection; return this; } @@ -361,7 +362,7 @@ public Builder withDefaultCollection(String defaultCollection) { * * @param timeToLive ttl value */ - public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { + public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { assert timeToLive > 0; this.timeToLiveSeconds = TimeUnit.SECONDS.convert(timeToLive, unit); return this; @@ -374,7 +375,7 @@ public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { * * @return this */ - public Builder withHttpClient(Http2SolrClient httpSolrClient) { + public Builder withHttpClient(C httpSolrClient) { if (this.internalClientBuilder != null) { throw new IllegalStateException( "The builder can't accept an httpClient AND an internalClientBuilder, only one of those can be provided"); @@ -391,7 +392,7 @@ public Builder withHttpClient(Http2SolrClient httpSolrClient) { * @param internalClientBuilder the builder to use for creating the internal http client. * @return this */ - public Builder withHttpClientBuilder(Http2SolrClient.Builder internalClientBuilder) { + public Builder withHttpClientBuilder(B internalClientBuilder) { if (this.httpClient != null) { throw new IllegalStateException( "The builder can't accept an httpClient AND an internalClientBuilder, only one of those can be provided"); @@ -401,7 +402,7 @@ public Builder withHttpClientBuilder(Http2SolrClient.Builder internalClientBuild } @Deprecated(since = "9.10") - public Builder withInternalClientBuilder(Http2SolrClient.Builder internalClientBuilder) { + public Builder withInternalClientBuilder(B internalClientBuilder) { return withHttpClientBuilder(internalClientBuilder); } @@ -411,7 +412,7 @@ public Builder withInternalClientBuilder(Http2SolrClient.Builder internalClientB * @param zkConnectTimeout timeout value * @param unit time unit */ - public Builder withZkConnectTimeout(int zkConnectTimeout, TimeUnit unit) { + public Builder withZkConnectTimeout(int zkConnectTimeout, TimeUnit unit) { this.zkConnectTimeout = Math.toIntExact(unit.toMillis(zkConnectTimeout)); return this; } @@ -422,13 +423,13 @@ public Builder withZkConnectTimeout(int zkConnectTimeout, TimeUnit unit) { * @param zkClientTimeout timeout value * @param unit time unit */ - public Builder withZkClientTimeout(int zkClientTimeout, TimeUnit unit) { + public Builder withZkClientTimeout(int zkClientTimeout, TimeUnit unit) { this.zkClientTimeout = Math.toIntExact(unit.toMillis(zkClientTimeout)); return this; } /** Create a {@link CloudHttp2SolrClient} based on the provided configuration. */ - public CloudHttp2SolrClient build() { + public CloudHttp2SolrClient build() { int providedOptions = 0; if (!zkHosts.isEmpty()) providedOptions++; if (!solrUrls.isEmpty()) providedOptions++; @@ -442,7 +443,7 @@ public CloudHttp2SolrClient build() { "One of zkHosts, solrUrls, or stateProvider must be specified."); } - return new CloudHttp2SolrClient(this); + return new CloudHttp2SolrClient<>(this); } } } 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 ac07fb120284..72dbf83cfc76 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 @@ -54,7 +54,7 @@ public static void setupCluster() throws Exception { @Test public void testSingleZkHostSpecified() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = @@ -72,7 +72,7 @@ public void testSeveralZkHostsSpecifiedSingly() throws IOException { zkHostList.add(ANY_ZK_HOST); zkHostList.add(ANY_OTHER_ZK_HOST); try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -89,7 +89,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { zkHosts.add(ANY_ZK_HOST); zkHosts.add(ANY_OTHER_ZK_HOST); try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -103,7 +103,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { @Test public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .build()) { assertTrue(createdClient.isUpdatesToLeaders()); @@ -113,7 +113,7 @@ public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throw @Test public void testIsDirectUpdatesToLeadersOnlyDefault() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .build()) { assertFalse(createdClient.isDirectUpdatesToLeadersOnly()); @@ -125,7 +125,7 @@ public void testExternalClientAndInternalBuilderTogether() { expectThrows( IllegalStateException.class, () -> - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClient(mock(Http2SolrClient.class)) .withHttpClientBuilder(mock(Http2SolrClient.Builder.class)) @@ -133,7 +133,7 @@ public void testExternalClientAndInternalBuilderTogether() { expectThrows( IllegalStateException.class, () -> - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClientBuilder(mock(Http2SolrClient.Builder.class)) .withHttpClient(mock(Http2SolrClient.class)) @@ -141,16 +141,16 @@ public void testExternalClientAndInternalBuilderTogether() { } @Test - public void testProvideInternalBuilder() throws IOException { + public void testProvideInternalJettyClientBuilder() throws IOException { Http2SolrClient http2Client = mock(Http2SolrClient.class); Http2SolrClient.Builder http2ClientBuilder = mock(Http2SolrClient.Builder.class); when(http2ClientBuilder.build()).thenReturn(http2Client); - CloudHttp2SolrClient.Builder clientBuilder = - new CloudHttp2SolrClient.Builder( + CloudHttp2SolrClient.Builder, HttpSolrClientBase> clientBuilder = + new CloudHttp2SolrClient.Builder<>( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClientBuilder(http2ClientBuilder); verify(http2ClientBuilder, never()).build(); - try (CloudHttp2SolrClient client = clientBuilder.build()) { + try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals(http2Client, client.getHttpClient()); verify(http2ClientBuilder, times(1)).build(); verify(http2Client, never()).close(); @@ -160,13 +160,46 @@ public void testProvideInternalBuilder() throws IOException { } @Test - public void testProvideExternalClient() throws IOException { + public void testProvideInternalJdkClientBuilder() throws IOException { + var http2Client = mock(HttpJdkSolrClient.class); + HttpJdkSolrClient.Builder http2ClientBuilder = mock(HttpJdkSolrClient.Builder.class); + when(http2ClientBuilder.build()).thenReturn(http2Client); + CloudHttp2SolrClient.Builder clientBuilder = + new CloudHttp2SolrClient.Builder( + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + .withHttpClientBuilder(http2ClientBuilder); + verify(http2ClientBuilder, never()).build(); + try (CloudHttp2SolrClient client = clientBuilder.build()) { + assertEquals(http2Client, client.getHttpClient()); + verify(http2ClientBuilder, times(1)).build(); + verify(http2Client, never()).close(); + } + // it's internal, should be closed when closing CloudSolrClient + verify(http2Client, times(1)).close(); + } + + @Test + public void testProvideExternalJettyClient() throws IOException { Http2SolrClient http2Client = mock(Http2SolrClient.class); - CloudHttp2SolrClient.Builder clientBuilder = - new CloudHttp2SolrClient.Builder( + CloudHttp2SolrClient.Builder clientBuilder = + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClient(http2Client); - try (CloudHttp2SolrClient client = clientBuilder.build()) { + try (CloudHttp2SolrClient client = clientBuilder.build()) { + assertEquals(http2Client, client.getHttpClient()); + } + // it's external, should be NOT closed when closing CloudSolrClient + verify(http2Client, never()).close(); + } + + @Test + public void testProvideExternalJdkClient() throws IOException { + HttpJdkSolrClient http2Client = mock(HttpJdkSolrClient.class); + CloudHttp2SolrClient.Builder clientBuilder = + new CloudHttp2SolrClient.Builder( + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + .withHttpClient(http2Client); + try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals(http2Client, client.getHttpClient()); } // it's external, should be NOT closed when closing CloudSolrClient @@ -175,8 +208,8 @@ public void testProvideExternalClient() throws IOException { @Test public void testDefaultCollectionPassedFromBuilderToClient() throws IOException { - try (CloudHttp2SolrClient createdClient = - new CloudHttp2SolrClient.Builder( + try (CloudHttp2SolrClient createdClient = + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withDefaultCollection("aCollection") .build()) { @@ -213,7 +246,7 @@ private void testHttpClientConsistency( Http2SolrClient httpClient, Http2SolrClient.Builder internalClientBuilder) throws IOException { - CloudHttp2SolrClient.Builder clientBuilder = new CloudHttp2SolrClient.Builder(solrUrls); + CloudHttp2SolrClient.Builder clientBuilder = new CloudHttp2SolrClient.Builder<>(solrUrls); if (httpClient != null) { clientBuilder.withHttpClient(httpClient); @@ -221,7 +254,7 @@ private void testHttpClientConsistency( clientBuilder.withHttpClientBuilder(internalClientBuilder); } - try (CloudHttp2SolrClient client = clientBuilder.build()) { + try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals( client.getHttpClient(), ((Http2ClusterStateProvider) client.getClusterStateProvider()).getHttpClient()); @@ -230,7 +263,7 @@ private void testHttpClientConsistency( public void testCustomStateProvider() throws IOException { ZkClientClusterStateProvider stateProvider = mock(ZkClientClusterStateProvider.class); - try (CloudSolrClient cloudSolrClient = new CloudHttp2SolrClient.Builder(stateProvider).build()) { + try (CloudSolrClient cloudSolrClient = new CloudHttp2SolrClient.Builder(stateProvider).build()) { assertEquals(stateProvider, cloudSolrClient.getClusterStateProvider()); } verify(stateProvider, times(1)).close(); 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 c4210bab7b4f..ccf57adc49b4 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 @@ -69,7 +69,7 @@ public void testZkConnectionStringConstructorWithValidChroot() throws IOExceptio } try (CloudSolrClient client = - new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) + new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) .build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(client)) { @@ -82,6 +82,6 @@ public void testZkConnectionStringConstructorWithValidChroot() throws IOExceptio public void testBadChroot() { final List zkHosts = new ArrayList<>(); zkHosts.add("host1:2181"); - new CloudHttp2SolrClient.Builder(zkHosts, Optional.of("foo")).build(); + new CloudHttp2SolrClient.Builder(zkHosts, Optional.of("foo")).build(); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index 39f10b2c324e..a399cd857646 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java @@ -53,7 +53,7 @@ public static void setupCluster() throws Exception { public void testRetry() throws Exception { String collectionName = "testRetry"; try (CloudSolrClient solrClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .build()) { CollectionAdminRequest.createCollection(collectionName, 1, 1).process(solrClient); 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 fb721a06ea23..70def143d70f 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 @@ -112,9 +112,9 @@ public static void setupCluster() throws Exception { final List solrUrls = new ArrayList<>(); solrUrls.add(cluster.getJettySolrRunner(0).getBaseUrl().toString()); - httpBasedCloudSolrClient = new CloudHttp2SolrClient.Builder(solrUrls).build(); + httpBasedCloudSolrClient = new CloudHttp2SolrClient.Builder(solrUrls).build(); zkBasedCloudSolrClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .build(); } @@ -312,7 +312,7 @@ public void testHttpCspPerf() throws Exception { private CloudSolrClient createHttpCSPBasedCloudSolrClient() { final List solrUrls = new ArrayList<>(); solrUrls.add(cluster.getJettySolrRunner(0).getBaseUrl().toString()); - return new CloudHttp2SolrClient.Builder(solrUrls).build(); + return new CloudHttp2SolrClient.Builder(solrUrls).build(); } @Test 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 4089faa454e9..2c101b86811b 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,7 +34,7 @@ public class CloudSolrClientBuilderTest extends SolrTestCase { @Test public void testSingleZkHostSpecified() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build(); ) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build(); ) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -48,7 +48,7 @@ public void testSeveralZkHostsSpecifiedSingly() throws IOException { final List zkHostList = new ArrayList<>(); zkHostList.add(ANY_ZK_HOST); zkHostList.add(ANY_OTHER_ZK_HOST); - try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { + try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -63,7 +63,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { final ArrayList zkHosts = new ArrayList<>(); zkHosts.add(ANY_ZK_HOST); zkHosts.add(ANY_OTHER_ZK_HOST); - try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { + try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -76,7 +76,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { @Test public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { assertTrue(createdClient.isUpdatesToLeaders()); } } @@ -84,7 +84,7 @@ public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throw @Test public void testIsDirectUpdatesToLeadersOnlyDefault() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { assertFalse(createdClient.isDirectUpdatesToLeadersOnly()); } } @@ -104,7 +104,7 @@ public void test0Timeouts() throws IOException { @Test public void testDefaultCollectionPassedFromBuilderToClient() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withDefaultCollection("aCollection") .build()) { assertEquals("aCollection", createdClient.getDefaultCollection()); 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 dc540e496937..ee769ec1e765 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 @@ -69,7 +69,7 @@ public void testZkConnectionStringSetterWithValidChroot() throws IOException { } try (CloudSolrClient client = - (new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) + (new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) .build())) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(client)) { @@ -113,6 +113,6 @@ public void testZkConnectionStringConstructorWithValidChroot() throws IOExceptio public void testBadChroot() { final List zkHosts = new ArrayList<>(); zkHosts.add("host1:2181"); - new CloudHttp2SolrClient.Builder(zkHosts, Optional.of("foo")).build(); + new CloudHttp2SolrClient.Builder(zkHosts, Optional.of("foo")).build(); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java index d160a5d43da4..d23987059ecc 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java @@ -83,7 +83,7 @@ public void testHttpClusterStateWithSSL() throws Exception { // verify the http derived cluster state (on the client side) agrees with what the server stored try (CloudSolrClient httpBasedCloudSolrClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())).build()) { ClusterStateProvider csp = httpBasedCloudSolrClient.getClusterStateProvider(); assertTrue(csp instanceof Http2ClusterStateProvider); verifyUrlSchemeInClusterState(csp.getCollection(collectionId), expectedReplicas); @@ -91,7 +91,7 @@ public void testHttpClusterStateWithSSL() throws Exception { // http2 try (CloudSolrClient http2BasedClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())) + new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())) .build()) { ClusterStateProvider csp = http2BasedClient.getClusterStateProvider(); assertTrue(csp instanceof Http2ClusterStateProvider); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java index 89c674b01eea..cd8a6c2b71d0 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java @@ -214,7 +214,7 @@ public void testBuilderImplicitBehavior() throws Exception { assertTrue(client.isUpdatesToLeaders()); } try (CloudSolrClient client = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .build()) { assertTrue(client.isUpdatesToLeaders()); @@ -245,7 +245,7 @@ public void testLegacyClientThatDoesNotDefaultToLeaders() throws Exception { public void testHttp2ClientThatDefaultsToLeaders() throws Exception { try (CloudSolrClient client = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .sendUpdatesOnlyToShardLeaders() .build()) { @@ -256,7 +256,7 @@ public void testHttp2ClientThatDefaultsToLeaders() throws Exception { public void testHttp2ClientThatDoesNotDefaultToLeaders() throws Exception { try (CloudSolrClient client = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .sendUpdatesToAnyReplica() .build()) { diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index f2b08870f529..16d0dca87219 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java @@ -2527,7 +2527,7 @@ public static Object skewed(Object likely, Object unlikely) { * A variant of {@link org.apache.solr.client.solrj.impl.CloudHttp2SolrClient.Builder} that will * randomize some internal settings. */ - public static class RandomizingCloudHttp2SolrClientBuilder extends CloudHttp2SolrClient.Builder { + public static class RandomizingCloudHttp2SolrClientBuilder extends CloudHttp2SolrClient.Builder { public RandomizingCloudHttp2SolrClientBuilder(List zkHosts, Optional zkChroot) { super(zkHosts, zkChroot); From f55e6e430d80a1321335fa287d191e4de01b3837 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Mon, 13 Oct 2025 15:50:57 -0500 Subject: [PATCH 03/27] remove generics; test with jdk client --- .../java/org/apache/solr/cli/CLIUtils.java | 6 +- .../java/org/apache/solr/cli/ExportTool.java | 2 +- .../org/apache/solr/cli/HealthcheckTool.java | 2 +- .../org/apache/solr/cli/RunExampleTool.java | 2 +- .../org/apache/solr/cloud/ZkController.java | 4 +- .../solr/client/solrj/io/SolrClientCache.java | 4 +- .../solrj/impl/SolrClientCloudManager.java | 4 +- .../impl/SolrClientNodeStateProvider.java | 10 +-- .../solrj/impl/CloudHttp2SolrClient.java | 73 +++++++++---------- .../impl/CloudHttp2SolrClientBuilderTest.java | 48 ++++++------ ...udHttp2SolrClientMultiConstructorTest.java | 4 +- .../impl/CloudHttp2SolrClientRetryTest.java | 2 +- .../solrj/impl/CloudHttp2SolrClientTest.java | 41 ++++++++--- .../impl/CloudSolrClientBuilderTest.java | 12 +-- .../CloudSolrClientMultiConstructorTest.java | 4 +- .../solrj/impl/HttpClusterStateSSLTest.java | 4 +- .../SendUpdatesToLeadersOverrideTest.java | 6 +- .../java/org/apache/solr/SolrTestCaseJ4.java | 2 +- 18 files changed, 125 insertions(+), 105 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cli/CLIUtils.java b/solr/core/src/java/org/apache/solr/cli/CLIUtils.java index fccbd4a84b33..c04e02fc1602 100644 --- a/solr/core/src/java/org/apache/solr/cli/CLIUtils.java +++ b/solr/core/src/java/org/apache/solr/cli/CLIUtils.java @@ -261,13 +261,13 @@ public static SolrZkClient getSolrZkClient(CommandLine cli, String zkHost) throw .build(); } - public static CloudHttp2SolrClient getCloudHttp2SolrClient(String zkHost) { + public static CloudHttp2SolrClient getCloudHttp2SolrClient(String zkHost) { return getCloudHttp2SolrClient(zkHost, null); } - public static CloudHttp2SolrClient getCloudHttp2SolrClient( + public static CloudHttp2SolrClient getCloudHttp2SolrClient( String zkHost, Http2SolrClient.Builder builder) { - return new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()) + return new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()) .withHttpClientBuilder(builder) .build(); } diff --git a/solr/core/src/java/org/apache/solr/cli/ExportTool.java b/solr/core/src/java/org/apache/solr/cli/ExportTool.java index e23925635633..af2cf6ba590e 100644 --- a/solr/core/src/java/org/apache/solr/cli/ExportTool.java +++ b/solr/core/src/java/org/apache/solr/cli/ExportTool.java @@ -256,7 +256,7 @@ void fetchUniqueKey() throws SolrServerException, IOException { new Http2SolrClient.Builder().withOptionalBasicAuthCredentials(credentials); solrClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(baseurl)) + new CloudHttp2SolrClient.Builder(Collections.singletonList(baseurl)) .withHttpClientBuilder(builder) .build(); NamedList response = diff --git a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java index c17f85c571d9..bcca704384bd 100644 --- a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java +++ b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java @@ -89,7 +89,7 @@ public void runImpl(CommandLine cli) throws Exception { CLIO.err("Healthcheck tool only works in Solr Cloud mode."); runtime.exit(1); } - try (CloudHttp2SolrClient cloudSolrClient = CLIUtils.getCloudHttp2SolrClient(zkHost)) { + try (CloudHttp2SolrClient cloudSolrClient = CLIUtils.getCloudHttp2SolrClient(zkHost)) { echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ..."); cloudSolrClient.connect(); runCloudTool(cloudSolrClient, cli); diff --git a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java index 4e15d459f997..148fa4df0ea5 100644 --- a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java +++ b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java @@ -631,7 +631,7 @@ protected void runCloudExample(CommandLine cli) throws Exception { /** wait until the number of live nodes == numNodes. */ protected void waitToSeeLiveNodes(String zkHost, int numNodes) { try (CloudSolrClient cloudClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()).build()) { cloudClient.connect(); Set liveNodes = cloudClient.getClusterState().getLiveNodes(); int numLiveNodes = (liveNodes != null) ? liveNodes.size() : 0; diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index f6be7f9a18aa..603381ef99c7 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -207,7 +207,7 @@ public String toString() { public final ZkStateReader zkStateReader; private SolrCloudManager cloudManager; - private CloudHttp2SolrClient cloudSolrClient; + private CloudHttp2SolrClient cloudSolrClient; private final ExecutorService zkConnectionListenerCallbackExecutor = ExecutorUtil.newMDCAwareSingleThreadExecutor( @@ -963,7 +963,7 @@ public SolrCloudManager getSolrCloudManager() { return cloudManager; } cloudSolrClient = - new CloudHttp2SolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader)) + new CloudHttp2SolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader)) .withHttpClient(cc.getDefaultHttpSolrClient()) .build(); cloudManager = new SolrClientCloudManager(cloudSolrClient, cc.getObjectCache()); diff --git a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java index 6138a6be7fad..ec94c7d5aa9a 100644 --- a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java +++ b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java @@ -96,13 +96,13 @@ public synchronized CloudSolrClient getCloudSolrClient(String zkHost) { return client; } - private static CloudHttp2SolrClient newCloudHttp2SolrClient( + private static CloudHttp2SolrClient newCloudHttp2SolrClient( String zkHost, Http2SolrClient http2SolrClient, boolean canUseACLs, String basicAuthCredentials) { final List hosts = List.of(zkHost); - var builder = new CloudHttp2SolrClient.Builder(hosts, Optional.empty()); + var builder = new CloudHttp2SolrClient.Builder(hosts, Optional.empty()); builder.canUseZkACLs(canUseACLs); // using internal builder to ensure the internal client gets closed builder = diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientCloudManager.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientCloudManager.java index 0efd116869a6..30afd6bed570 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientCloudManager.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientCloudManager.java @@ -33,7 +33,7 @@ public class SolrClientCloudManager implements SolrCloudManager { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private final CloudHttp2SolrClient cloudSolrClient; + private final CloudHttp2SolrClient cloudSolrClient; private final ZkDistribStateManager stateManager; private final ZkStateReader zkStateReader; private final SolrZkClient zkClient; @@ -41,7 +41,7 @@ public class SolrClientCloudManager implements SolrCloudManager { private final boolean closeObjectCache; private volatile boolean isClosed; - public SolrClientCloudManager(CloudHttp2SolrClient client, ObjectCache objectCache) { + public SolrClientCloudManager(CloudHttp2SolrClient client, ObjectCache objectCache) { this.cloudSolrClient = client; this.zkStateReader = ZkStateReader.from(client); this.zkClient = zkStateReader.getZkClient(); diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java index 51ab3540e00a..dabe6b80bd3f 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java @@ -52,14 +52,14 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private final CloudHttp2SolrClient solrClient; + private final CloudHttp2SolrClient solrClient; protected final Map>>> nodeVsCollectionVsShardVsReplicaInfo = new HashMap<>(); @SuppressWarnings({"rawtypes"}) private Map nodeVsTags = new HashMap<>(); - public SolrClientNodeStateProvider(CloudHttp2SolrClient solrClient) { + public SolrClientNodeStateProvider(CloudHttp2SolrClient solrClient) { this.solrClient = solrClient; try { readReplicaDetails(); @@ -222,7 +222,7 @@ public String toString() { static class RemoteCallCtx { ZkClientClusterStateProvider zkClientClusterStateProvider; - CloudHttp2SolrClient cloudSolrClient; + CloudHttp2SolrClient cloudSolrClient; public final Map tags = new HashMap<>(); private final String node; public Map session; @@ -234,7 +234,7 @@ public boolean isNodeAlive(String node) { return true; } - public RemoteCallCtx(String node, CloudHttp2SolrClient cloudSolrClient) { + public RemoteCallCtx(String node, CloudHttp2SolrClient cloudSolrClient) { this.node = node; this.cloudSolrClient = cloudSolrClient; this.zkClientClusterStateProvider = @@ -289,7 +289,7 @@ public SimpleSolrResponse invoke(String solrNode, String path, SolrParams params request.setResponseParser(new JavaBinResponseParser()); try { - return cloudSolrClient.getHttpClient().requestWithBaseUrl(url, request::process); + return ((Http2SolrClient) cloudSolrClient.getHttpClient()).requestWithBaseUrl(url, request::process); //NOCOMMIT } catch (SolrServerException | IOException e) { throw new SolrException(ErrorCode.SERVER_ERROR, "Fetching replica metrics failed", e); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index e4fbe3f7941c..d9b6aee78875 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -37,11 +37,11 @@ * @since solr 8.0 */ @SuppressWarnings("serial") -public class CloudHttp2SolrClient extends CloudSolrClient { +public class CloudHttp2SolrClient extends CloudSolrClient { private final ClusterStateProvider stateProvider; - private final LBHttp2SolrClient lbClient; - private final C myClient; + private final LBHttp2SolrClient lbClient; + private final HttpSolrClientBase myClient; private final boolean clientIsInternal; /** @@ -52,7 +52,7 @@ public class CloudHttp2SolrClient extends CloudSol * * @param builder a {@link Http2SolrClient.Builder} with the options used to create the client. */ - protected CloudHttp2SolrClient(Builder builder) { + protected CloudHttp2SolrClient(Builder builder) { super(builder.shardLeadersOnly, builder.parallelUpdates, builder.directUpdatesToLeadersOnly); this.clientIsInternal = builder.httpClient == null; this.myClient = createOrGetHttpClientFromBuilder(builder); @@ -75,18 +75,18 @@ protected CloudHttp2SolrClient(Builder builder) { this.lbClient = new LBHttp2SolrClient.Builder<>(myClient).build(); } - @SuppressWarnings("unchecked") - private C createOrGetHttpClientFromBuilder(Builder builder) { + + private HttpSolrClientBase createOrGetHttpClientFromBuilder(Builder builder) { if (builder.httpClient != null) { - return (C) builder.httpClient; + return builder.httpClient; } else if (builder.internalClientBuilder != null) { - return (C) builder.internalClientBuilder.build(); + return builder.internalClientBuilder.build(); } else { - return (C) new Http2SolrClient.Builder().build(); + return new Http2SolrClient.Builder().build(); } } - private ClusterStateProvider createClusterStateProvider(Builder builder) { + private ClusterStateProvider createClusterStateProvider(Builder builder) { if (builder.stateProvider != null) { return builder.stateProvider; } else if (builder.zkHosts.isEmpty()) { @@ -96,7 +96,7 @@ private ClusterStateProvider createClusterStateProvider(Builder builder) { } } - private ClusterStateProvider createZkClusterStateProvider(Builder builder) { + private ClusterStateProvider createZkClusterStateProvider(Builder builder) { try { ClusterStateProvider stateProvider = ClusterStateProvider.newZkClusterStateProvider( @@ -113,7 +113,7 @@ private ClusterStateProvider createZkClusterStateProvider(Builder builder) } private ClusterStateProvider createHttp2ClusterStateProvider( - List solrUrls, C httpClient) { + List solrUrls, HttpSolrClientBase httpClient) { try { return new Http2ClusterStateProvider<>(solrUrls, httpClient); } catch (Exception e) { @@ -148,7 +148,7 @@ public void close() throws IOException { } @Override - public LBHttp2SolrClient getLbClient() { + public LBHttp2SolrClient getLbClient() { return lbClient; } @@ -157,7 +157,7 @@ public ClusterStateProvider getClusterStateProvider() { return stateProvider; } - public C getHttpClient() { + public HttpSolrClientBase getHttpClient() { return myClient; } @@ -167,16 +167,16 @@ protected boolean wasCommError(Throwable rootCause) { } /** Constructs {@link CloudHttp2SolrClient} instances from provided configuration. */ - public static class Builder, C extends HttpSolrClientBase> { + public static class Builder { protected Collection zkHosts = new ArrayList<>(); protected List solrUrls = new ArrayList<>(); protected String zkChroot; - protected C httpClient; + protected HttpSolrClientBase httpClient; protected boolean shardLeadersOnly = true; protected boolean directUpdatesToLeadersOnly = false; protected boolean parallelUpdates = true; protected ClusterStateProvider stateProvider; - protected B internalClientBuilder; + protected HttpSolrClientBuilderBase internalClientBuilder; private RequestWriter requestWriter; private ResponseParser responseParser; private long retryExpiryTimeNano = @@ -206,7 +206,6 @@ public static class Builder, C extends * final SolrClient client = new CloudHttp2SolrClient.Builder(solrBaseUrls).build(); * */ - @SuppressWarnings({"rawtypes"}) public Builder(List solrUrls) { this.solrUrls = solrUrls; } @@ -246,7 +245,7 @@ public Builder(ClusterStateProvider stateProvider) { } /** Whether to use the default ZK ACLs when building a ZK Client. */ - public Builder canUseZkACLs(boolean canUseZkACLs) { + public Builder canUseZkACLs(boolean canUseZkACLs) { this.canUseZkACLs = canUseZkACLs; return this; } @@ -258,7 +257,7 @@ public Builder canUseZkACLs(boolean canUseZkACLs) { * @see #sendUpdatesToAnyReplica * @see CloudSolrClient#isUpdatesToLeaders */ - public Builder sendUpdatesOnlyToShardLeaders() { + public Builder sendUpdatesOnlyToShardLeaders() { shardLeadersOnly = true; return this; } @@ -270,7 +269,7 @@ public Builder sendUpdatesOnlyToShardLeaders() { * @see #sendUpdatesOnlyToShardLeaders * @see CloudSolrClient#isUpdatesToLeaders */ - public Builder sendUpdatesToAnyReplica() { + public Builder sendUpdatesToAnyReplica() { shardLeadersOnly = false; return this; } @@ -285,7 +284,7 @@ public Builder sendUpdatesToAnyReplica() { * @see #sendDirectUpdatesToAnyShardReplica * @see CloudSolrClient#isDirectUpdatesToLeadersOnly */ - public Builder sendDirectUpdatesToShardLeadersOnly() { + public Builder sendDirectUpdatesToShardLeadersOnly() { directUpdatesToLeadersOnly = true; return this; } @@ -300,19 +299,19 @@ public Builder sendDirectUpdatesToShardLeadersOnly() { * @see #sendDirectUpdatesToShardLeadersOnly * @see CloudSolrClient#isDirectUpdatesToLeadersOnly */ - public Builder sendDirectUpdatesToAnyShardReplica() { + public Builder sendDirectUpdatesToAnyShardReplica() { directUpdatesToLeadersOnly = false; return this; } /** Provides a {@link RequestWriter} for created clients to use when handing requests. */ - public Builder withRequestWriter(RequestWriter requestWriter) { + public Builder withRequestWriter(RequestWriter requestWriter) { this.requestWriter = requestWriter; return this; } /** Provides a {@link ResponseParser} for created clients to use when handling requests. */ - public Builder withResponseParser(ResponseParser responseParser) { + public Builder withResponseParser(ResponseParser responseParser) { this.responseParser = responseParser; return this; } @@ -327,7 +326,7 @@ public Builder withResponseParser(ResponseParser responseParser) { * *

If not set, this defaults to 'true' and sends sub-requests in parallel. */ - public Builder withParallelUpdates(boolean parallelUpdates) { + public Builder withParallelUpdates(boolean parallelUpdates) { this.parallelUpdates = parallelUpdates; return this; } @@ -338,7 +337,7 @@ public Builder withParallelUpdates(boolean parallelUpdates) { * *

Defaults to 3. */ - public Builder withParallelCacheRefreshes(int parallelCacheRefreshesLocks) { + public Builder withParallelCacheRefreshes(int parallelCacheRefreshesLocks) { this.parallelCacheRefreshesLocks = parallelCacheRefreshesLocks; return this; } @@ -346,13 +345,13 @@ public Builder withParallelCacheRefreshes(int parallelCacheRefreshesLocks) /** * This is the time to wait to re-fetch the state after getting the same state version from ZK */ - public Builder withRetryExpiryTime(long expiryTime, TimeUnit unit) { + public Builder withRetryExpiryTime(long expiryTime, TimeUnit unit) { this.retryExpiryTimeNano = TimeUnit.NANOSECONDS.convert(expiryTime, unit); return this; } /** Sets the default collection for request. */ - public Builder withDefaultCollection(String defaultCollection) { + public Builder withDefaultCollection(String defaultCollection) { this.defaultCollection = defaultCollection; return this; } @@ -362,7 +361,7 @@ public Builder withDefaultCollection(String defaultCollection) { * * @param timeToLive ttl value */ - public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { + public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { assert timeToLive > 0; this.timeToLiveSeconds = TimeUnit.SECONDS.convert(timeToLive, unit); return this; @@ -375,7 +374,7 @@ public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { * * @return this */ - public Builder withHttpClient(C httpSolrClient) { + public Builder withHttpClient(HttpSolrClientBase httpSolrClient) { if (this.internalClientBuilder != null) { throw new IllegalStateException( "The builder can't accept an httpClient AND an internalClientBuilder, only one of those can be provided"); @@ -392,7 +391,7 @@ public Builder withHttpClient(C httpSolrClient) { * @param internalClientBuilder the builder to use for creating the internal http client. * @return this */ - public Builder withHttpClientBuilder(B internalClientBuilder) { + public Builder withHttpClientBuilder(HttpSolrClientBuilderBase internalClientBuilder) { if (this.httpClient != null) { throw new IllegalStateException( "The builder can't accept an httpClient AND an internalClientBuilder, only one of those can be provided"); @@ -402,7 +401,7 @@ public Builder withHttpClientBuilder(B internalClientBuilder) { } @Deprecated(since = "9.10") - public Builder withInternalClientBuilder(B internalClientBuilder) { + public Builder withInternalClientBuilder(HttpSolrClientBuilderBase internalClientBuilder) { return withHttpClientBuilder(internalClientBuilder); } @@ -412,7 +411,7 @@ public Builder withInternalClientBuilder(B internalClientBuilder) { * @param zkConnectTimeout timeout value * @param unit time unit */ - public Builder withZkConnectTimeout(int zkConnectTimeout, TimeUnit unit) { + public Builder withZkConnectTimeout(int zkConnectTimeout, TimeUnit unit) { this.zkConnectTimeout = Math.toIntExact(unit.toMillis(zkConnectTimeout)); return this; } @@ -423,13 +422,13 @@ public Builder withZkConnectTimeout(int zkConnectTimeout, TimeUnit unit) { * @param zkClientTimeout timeout value * @param unit time unit */ - public Builder withZkClientTimeout(int zkClientTimeout, TimeUnit unit) { + public Builder withZkClientTimeout(int zkClientTimeout, TimeUnit unit) { this.zkClientTimeout = Math.toIntExact(unit.toMillis(zkClientTimeout)); return this; } /** Create a {@link CloudHttp2SolrClient} based on the provided configuration. */ - public CloudHttp2SolrClient build() { + public CloudHttp2SolrClient build() { int providedOptions = 0; if (!zkHosts.isEmpty()) providedOptions++; if (!solrUrls.isEmpty()) providedOptions++; @@ -443,7 +442,7 @@ public CloudHttp2SolrClient build() { "One of zkHosts, solrUrls, or stateProvider must be specified."); } - return new CloudHttp2SolrClient<>(this); + return new CloudHttp2SolrClient(this); } } } 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 72dbf83cfc76..f3e3804bc8ff 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 @@ -54,7 +54,7 @@ public static void setupCluster() throws Exception { @Test public void testSingleZkHostSpecified() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = @@ -72,7 +72,7 @@ public void testSeveralZkHostsSpecifiedSingly() throws IOException { zkHostList.add(ANY_ZK_HOST); zkHostList.add(ANY_OTHER_ZK_HOST); try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -89,7 +89,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { zkHosts.add(ANY_ZK_HOST); zkHosts.add(ANY_OTHER_ZK_HOST); try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -103,7 +103,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { @Test public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .build()) { assertTrue(createdClient.isUpdatesToLeaders()); @@ -113,7 +113,7 @@ public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throw @Test public void testIsDirectUpdatesToLeadersOnlyDefault() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .build()) { assertFalse(createdClient.isDirectUpdatesToLeadersOnly()); @@ -125,7 +125,7 @@ public void testExternalClientAndInternalBuilderTogether() { expectThrows( IllegalStateException.class, () -> - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClient(mock(Http2SolrClient.class)) .withHttpClientBuilder(mock(Http2SolrClient.Builder.class)) @@ -133,7 +133,7 @@ public void testExternalClientAndInternalBuilderTogether() { expectThrows( IllegalStateException.class, () -> - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClientBuilder(mock(Http2SolrClient.Builder.class)) .withHttpClient(mock(Http2SolrClient.class)) @@ -145,12 +145,12 @@ public void testProvideInternalJettyClientBuilder() throws IOException { Http2SolrClient http2Client = mock(Http2SolrClient.class); Http2SolrClient.Builder http2ClientBuilder = mock(Http2SolrClient.Builder.class); when(http2ClientBuilder.build()).thenReturn(http2Client); - CloudHttp2SolrClient.Builder, HttpSolrClientBase> clientBuilder = - new CloudHttp2SolrClient.Builder<>( + CloudHttp2SolrClient.Builder clientBuilder = + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClientBuilder(http2ClientBuilder); verify(http2ClientBuilder, never()).build(); - try (CloudHttp2SolrClient client = clientBuilder.build()) { + try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals(http2Client, client.getHttpClient()); verify(http2ClientBuilder, times(1)).build(); verify(http2Client, never()).close(); @@ -164,12 +164,12 @@ public void testProvideInternalJdkClientBuilder() throws IOException { var http2Client = mock(HttpJdkSolrClient.class); HttpJdkSolrClient.Builder http2ClientBuilder = mock(HttpJdkSolrClient.Builder.class); when(http2ClientBuilder.build()).thenReturn(http2Client); - CloudHttp2SolrClient.Builder clientBuilder = - new CloudHttp2SolrClient.Builder( + CloudHttp2SolrClient.Builder clientBuilder = + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClientBuilder(http2ClientBuilder); verify(http2ClientBuilder, never()).build(); - try (CloudHttp2SolrClient client = clientBuilder.build()) { + try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals(http2Client, client.getHttpClient()); verify(http2ClientBuilder, times(1)).build(); verify(http2Client, never()).close(); @@ -181,11 +181,11 @@ public void testProvideInternalJdkClientBuilder() throws IOException { @Test public void testProvideExternalJettyClient() throws IOException { Http2SolrClient http2Client = mock(Http2SolrClient.class); - CloudHttp2SolrClient.Builder clientBuilder = - new CloudHttp2SolrClient.Builder( + CloudHttp2SolrClient.Builder clientBuilder = + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClient(http2Client); - try (CloudHttp2SolrClient client = clientBuilder.build()) { + try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals(http2Client, client.getHttpClient()); } // it's external, should be NOT closed when closing CloudSolrClient @@ -195,11 +195,11 @@ public void testProvideExternalJettyClient() throws IOException { @Test public void testProvideExternalJdkClient() throws IOException { HttpJdkSolrClient http2Client = mock(HttpJdkSolrClient.class); - CloudHttp2SolrClient.Builder clientBuilder = - new CloudHttp2SolrClient.Builder( + CloudHttp2SolrClient.Builder clientBuilder = + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClient(http2Client); - try (CloudHttp2SolrClient client = clientBuilder.build()) { + try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals(http2Client, client.getHttpClient()); } // it's external, should be NOT closed when closing CloudSolrClient @@ -208,8 +208,8 @@ public void testProvideExternalJdkClient() throws IOException { @Test public void testDefaultCollectionPassedFromBuilderToClient() throws IOException { - try (CloudHttp2SolrClient createdClient = - new CloudHttp2SolrClient.Builder( + try (CloudHttp2SolrClient createdClient = + new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withDefaultCollection("aCollection") .build()) { @@ -246,7 +246,7 @@ private void testHttpClientConsistency( Http2SolrClient httpClient, Http2SolrClient.Builder internalClientBuilder) throws IOException { - CloudHttp2SolrClient.Builder clientBuilder = new CloudHttp2SolrClient.Builder<>(solrUrls); + CloudHttp2SolrClient.Builder clientBuilder = new CloudHttp2SolrClient.Builder(solrUrls); if (httpClient != null) { clientBuilder.withHttpClient(httpClient); @@ -254,7 +254,7 @@ private void testHttpClientConsistency( clientBuilder.withHttpClientBuilder(internalClientBuilder); } - try (CloudHttp2SolrClient client = clientBuilder.build()) { + try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals( client.getHttpClient(), ((Http2ClusterStateProvider) client.getClusterStateProvider()).getHttpClient()); @@ -263,7 +263,7 @@ private void testHttpClientConsistency( public void testCustomStateProvider() throws IOException { ZkClientClusterStateProvider stateProvider = mock(ZkClientClusterStateProvider.class); - try (CloudSolrClient cloudSolrClient = new CloudHttp2SolrClient.Builder(stateProvider).build()) { + try (CloudSolrClient cloudSolrClient = new CloudHttp2SolrClient.Builder(stateProvider).build()) { assertEquals(stateProvider, cloudSolrClient.getClusterStateProvider()); } verify(stateProvider, times(1)).close(); 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 ccf57adc49b4..c4210bab7b4f 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 @@ -69,7 +69,7 @@ public void testZkConnectionStringConstructorWithValidChroot() throws IOExceptio } try (CloudSolrClient client = - new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) + new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) .build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(client)) { @@ -82,6 +82,6 @@ public void testZkConnectionStringConstructorWithValidChroot() throws IOExceptio public void testBadChroot() { final List zkHosts = new ArrayList<>(); zkHosts.add("host1:2181"); - new CloudHttp2SolrClient.Builder(zkHosts, Optional.of("foo")).build(); + new CloudHttp2SolrClient.Builder(zkHosts, Optional.of("foo")).build(); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index a399cd857646..39f10b2c324e 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java @@ -53,7 +53,7 @@ public static void setupCluster() throws Exception { public void testRetry() throws Exception { String collectionName = "testRetry"; try (CloudSolrClient solrClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .build()) { CollectionAdminRequest.createCollection(collectionName, 1, 1).process(solrClient); 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 70def143d70f..508bd7359389 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 @@ -94,8 +94,9 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { private static final int TIMEOUT = 30; private static final int NODE_COUNT = 3; - private static CloudSolrClient httpBasedCloudSolrClient = null; - private static CloudSolrClient zkBasedCloudSolrClient = null; + private static CloudHttp2SolrClient httpJettyBasedCloudSolrClient = null; + private static CloudHttp2SolrClient httpJdkBasedCloudSolrClient = null; + private static CloudHttp2SolrClient zkBasedCloudSolrClient = null; @BeforeClass public static void setupCluster() throws Exception { @@ -112,19 +113,39 @@ public static void setupCluster() throws Exception { final List solrUrls = new ArrayList<>(); solrUrls.add(cluster.getJettySolrRunner(0).getBaseUrl().toString()); - httpBasedCloudSolrClient = new CloudHttp2SolrClient.Builder(solrUrls).build(); + + httpJettyBasedCloudSolrClient = new CloudHttp2SolrClient.Builder(solrUrls).withHttpClientBuilder(new Http2SolrClient.Builder()).build(); + assertTrue(httpJettyBasedCloudSolrClient.getHttpClient() instanceof Http2SolrClient); + assertTrue(httpJettyBasedCloudSolrClient.getClusterStateProvider() instanceof Http2ClusterStateProvider); + assertTrue(((Http2ClusterStateProvider) httpJettyBasedCloudSolrClient.getClusterStateProvider()).getHttpClient() instanceof Http2SolrClient); + + httpJdkBasedCloudSolrClient = new CloudHttp2SolrClient.Builder(solrUrls).withHttpClientBuilder(new HttpJdkSolrClient.Builder().withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT)).build(); + assertTrue(httpJdkBasedCloudSolrClient.getHttpClient() instanceof HttpJdkSolrClient); + assertTrue(httpJdkBasedCloudSolrClient.getClusterStateProvider() instanceof Http2ClusterStateProvider); + assertTrue(((Http2ClusterStateProvider) httpJdkBasedCloudSolrClient.getClusterStateProvider()).getHttpClient() instanceof HttpJdkSolrClient); + zkBasedCloudSolrClient = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .build(); + assertTrue(zkBasedCloudSolrClient.getHttpClient() instanceof Http2SolrClient); + assertTrue(zkBasedCloudSolrClient.getClusterStateProvider() instanceof ZkClientClusterStateProvider); + } @AfterClass public static void tearDownAfterClass() throws Exception { - if (httpBasedCloudSolrClient != null) { + if (httpJettyBasedCloudSolrClient != null) { + try { + httpJettyBasedCloudSolrClient.close(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + if (httpJdkBasedCloudSolrClient != null) { try { - httpBasedCloudSolrClient.close(); + httpJdkBasedCloudSolrClient.close(); } catch (IOException e) { throw new RuntimeException(e); } @@ -138,14 +159,14 @@ public static void tearDownAfterClass() throws Exception { } shutdownCluster(); - httpBasedCloudSolrClient = null; + httpJettyBasedCloudSolrClient = null; zkBasedCloudSolrClient = null; } /** Randomly return the cluster's ZK based CSC, or HttpClusterProvider based CSC. */ private CloudSolrClient getRandomClient() { // return random().nextBoolean()? zkBasedCloudSolrClient : httpBasedCloudSolrClient; - return httpBasedCloudSolrClient; + return httpJdkBasedCloudSolrClient; //NOCOMMit } @Test @@ -312,7 +333,7 @@ public void testHttpCspPerf() throws Exception { private CloudSolrClient createHttpCSPBasedCloudSolrClient() { final List solrUrls = new ArrayList<>(); solrUrls.add(cluster.getJettySolrRunner(0).getBaseUrl().toString()); - return new CloudHttp2SolrClient.Builder(solrUrls).build(); + return new CloudHttp2SolrClient.Builder(solrUrls).build(); } @Test @@ -1007,7 +1028,7 @@ public void testInitializationWithSolrUrls() throws Exception { CollectionAdminRequest.createCollection(COLLECTION, "conf", 2, 1) .process(cluster.getSolrClient()); cluster.waitForActiveCollection(COLLECTION, 2, 2); - CloudSolrClient client = httpBasedCloudSolrClient; + CloudSolrClient client = httpJettyBasedCloudSolrClient; SolrInputDocument doc = new SolrInputDocument("id", "1", "title_s", "my doc"); new UpdateRequest().add(doc).commit(client, COLLECTION); assertEquals(1, client.query(COLLECTION, params("q", "*:*")).getResults().getNumFound()); 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 2c101b86811b..4089faa454e9 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,7 +34,7 @@ public class CloudSolrClientBuilderTest extends SolrTestCase { @Test public void testSingleZkHostSpecified() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build(); ) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build(); ) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -48,7 +48,7 @@ public void testSeveralZkHostsSpecifiedSingly() throws IOException { final List zkHostList = new ArrayList<>(); zkHostList.add(ANY_ZK_HOST); zkHostList.add(ANY_OTHER_ZK_HOST); - try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { + try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -63,7 +63,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { final ArrayList zkHosts = new ArrayList<>(); zkHosts.add(ANY_ZK_HOST); zkHosts.add(ANY_OTHER_ZK_HOST); - try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { + try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -76,7 +76,7 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { @Test public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { assertTrue(createdClient.isUpdatesToLeaders()); } } @@ -84,7 +84,7 @@ public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throw @Test public void testIsDirectUpdatesToLeadersOnlyDefault() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { assertFalse(createdClient.isDirectUpdatesToLeadersOnly()); } } @@ -104,7 +104,7 @@ public void test0Timeouts() throws IOException { @Test public void testDefaultCollectionPassedFromBuilderToClient() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withDefaultCollection("aCollection") .build()) { assertEquals("aCollection", createdClient.getDefaultCollection()); 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 ee769ec1e765..dc540e496937 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 @@ -69,7 +69,7 @@ public void testZkConnectionStringSetterWithValidChroot() throws IOException { } try (CloudSolrClient client = - (new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) + (new CloudHttp2SolrClient.Builder(new ArrayList<>(hosts), Optional.ofNullable(clientChroot)) .build())) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(client)) { @@ -113,6 +113,6 @@ public void testZkConnectionStringConstructorWithValidChroot() throws IOExceptio public void testBadChroot() { final List zkHosts = new ArrayList<>(); zkHosts.add("host1:2181"); - new CloudHttp2SolrClient.Builder(zkHosts, Optional.of("foo")).build(); + new CloudHttp2SolrClient.Builder(zkHosts, Optional.of("foo")).build(); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java index d23987059ecc..d160a5d43da4 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java @@ -83,7 +83,7 @@ public void testHttpClusterStateWithSSL() throws Exception { // verify the http derived cluster state (on the client side) agrees with what the server stored try (CloudSolrClient httpBasedCloudSolrClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())).build()) { ClusterStateProvider csp = httpBasedCloudSolrClient.getClusterStateProvider(); assertTrue(csp instanceof Http2ClusterStateProvider); verifyUrlSchemeInClusterState(csp.getCollection(collectionId), expectedReplicas); @@ -91,7 +91,7 @@ public void testHttpClusterStateWithSSL() throws Exception { // http2 try (CloudSolrClient http2BasedClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())) + new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())) .build()) { ClusterStateProvider csp = http2BasedClient.getClusterStateProvider(); assertTrue(csp instanceof Http2ClusterStateProvider); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java index cd8a6c2b71d0..89c674b01eea 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SendUpdatesToLeadersOverrideTest.java @@ -214,7 +214,7 @@ public void testBuilderImplicitBehavior() throws Exception { assertTrue(client.isUpdatesToLeaders()); } try (CloudSolrClient client = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .build()) { assertTrue(client.isUpdatesToLeaders()); @@ -245,7 +245,7 @@ public void testLegacyClientThatDoesNotDefaultToLeaders() throws Exception { public void testHttp2ClientThatDefaultsToLeaders() throws Exception { try (CloudSolrClient client = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .sendUpdatesOnlyToShardLeaders() .build()) { @@ -256,7 +256,7 @@ public void testHttp2ClientThatDefaultsToLeaders() throws Exception { public void testHttp2ClientThatDoesNotDefaultToLeaders() throws Exception { try (CloudSolrClient client = - new CloudHttp2SolrClient.Builder( + new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .sendUpdatesToAnyReplica() .build()) { diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index 16d0dca87219..f2b08870f529 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java @@ -2527,7 +2527,7 @@ public static Object skewed(Object likely, Object unlikely) { * A variant of {@link org.apache.solr.client.solrj.impl.CloudHttp2SolrClient.Builder} that will * randomize some internal settings. */ - public static class RandomizingCloudHttp2SolrClientBuilder extends CloudHttp2SolrClient.Builder { + public static class RandomizingCloudHttp2SolrClientBuilder extends CloudHttp2SolrClient.Builder { public RandomizingCloudHttp2SolrClientBuilder(List zkHosts, Optional zkChroot) { super(zkHosts, zkChroot); From 4f3157720fff82c8f981f4a138ffdc28720d59da Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Tue, 14 Oct 2025 14:32:26 -0500 Subject: [PATCH 04/27] Enhance JDK client to track if it has issued a HEAD request by unique base URI --- .../client/solrj/impl/HttpJdkSolrClient.java | 28 ++++++++++--------- .../solrj/impl/HttpJdkSolrClientTest.java | 18 +++++++++--- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index 15e686d57730..54ca3b8b8b7d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -34,11 +34,13 @@ import java.time.Duration; import java.time.temporal.ChronoUnit; import java.util.Collection; +import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; @@ -379,17 +381,6 @@ protected boolean maybeTryHeadRequest(String url) { if (forceHttp11 || url == null || url.toLowerCase(Locale.ROOT).startsWith("https://")) { return true; } - return maybeTryHeadRequestSync(url); - } - - protected volatile boolean headRequested; // must be threadsafe - private boolean headSucceeded; // must be threadsafe - - private synchronized boolean maybeTryHeadRequestSync(String url) { - if (headRequested) { - return headSucceeded; - } - URI uriNoQueryParams; try { uriNoQueryParams = new URI(url); @@ -397,6 +388,17 @@ private synchronized boolean maybeTryHeadRequestSync(String url) { // If the url is invalid, let a subsequent request try again. return false; } + return maybeTryHeadRequestSync(uriNoQueryParams); + } + + protected Map headSucceededByBaseUri = new HashMap<>(); // use only in synchronized method + + private synchronized boolean maybeTryHeadRequestSync(URI uriNoQueryParams) { + Boolean headSucceeded = headSucceededByBaseUri.get(uriNoQueryParams); + if (headSucceeded !=null) { + return headSucceeded; + } + HttpRequest.Builder headReqB = HttpRequest.newBuilder(uriNoQueryParams) .method("HEAD", HttpRequest.BodyPublishers.noBody()) @@ -406,7 +408,7 @@ private synchronized boolean maybeTryHeadRequestSync(String url) { httpClient.send(headReqB.build(), HttpResponse.BodyHandlers.discarding()); headSucceeded = true; } catch (IOException ioe) { - log.warn("Could not issue HEAD request to {} ", url, ioe); + log.warn("Could not issue HEAD request to {} ", uriNoQueryParams, ioe); headSucceeded = false; } catch (InterruptedException ie) { Thread.currentThread().interrupt(); @@ -414,7 +416,7 @@ private synchronized boolean maybeTryHeadRequestSync(String url) { } finally { // The HEAD request is tried only once. All future requests will skip this check. - headRequested = true; + headSucceededByBaseUri.put(uriNoQueryParams, headSucceeded); if (!headSucceeded) { log.info("All unencrypted POST requests with a chunked body will use http/1.1"); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index e6a47680bc87..9bd418e7e6c4 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -20,6 +20,8 @@ import java.io.IOException; import java.net.CookieHandler; import java.net.CookieManager; +import java.net.URI; +import java.net.URISyntaxException; import java.net.http.HttpClient; import java.util.Arrays; import java.util.Collections; @@ -328,9 +330,9 @@ private void testUpdateXml(boolean http11) throws Exception { testUpdate(client, HttpSolrClientTestBase.WT.XML, "application/xml; charset=UTF-8", value); if (http11) { assertEquals(HttpClient.Version.HTTP_1_1, client.httpClient.version()); - assertFalse( + assertNull( "The HEAD request should not be performed if already forcing Http/1.1.", - client.headRequested); + client.headSucceededByBaseUri.get(baseUri())); } else { assertEquals(HttpClient.Version.HTTP_2, client.httpClient.version()); } @@ -504,7 +506,7 @@ public void testMaybeTryHeadRequestHasContentType() throws Exception { assertTrue(client.maybeTryHeadRequest(url)); // if https, the client won't attempt a HEAD request - if (client.headRequested) { + if (!client.headSucceededByBaseUri.isEmpty()) { assertEquals("head", DebugServlet.lastMethod); assertTrue(DebugServlet.headers.containsKey("content-type")); } @@ -524,7 +526,15 @@ private void queryToHelpJdkReleaseThreads(HttpJdkSolrClient client) throws Excep private void assertNoHeadRequestWithSsl(HttpJdkSolrClient client) { if (isSSLMode()) { - assertFalse("The HEAD request should not be performed if using SSL.", client.headRequested); + assertNull("The HEAD request should not be performed if using SSL.", client.headSucceededByBaseUri.get(baseUri())); + } + } + + private URI baseUri() { + try { + return new URI(getBaseUrl()); + } catch(URISyntaxException e) { + throw new RuntimeException(e); } } From 69236759f0041fb2386baf19973a10280a9e9abe Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Tue, 14 Oct 2025 14:47:29 -0500 Subject: [PATCH 05/27] fix test randomization --- .../client/solrj/impl/CloudHttp2SolrClientTest.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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 508bd7359389..bb521d43608f 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 @@ -165,8 +165,14 @@ public static void tearDownAfterClass() throws Exception { /** Randomly return the cluster's ZK based CSC, or HttpClusterProvider based CSC. */ private CloudSolrClient getRandomClient() { - // return random().nextBoolean()? zkBasedCloudSolrClient : httpBasedCloudSolrClient; - return httpJdkBasedCloudSolrClient; //NOCOMMit + int randInt = random().nextInt(2); + if(randInt==0) { + return zkBasedCloudSolrClient; + } + if(randInt==1) { + return httpJettyBasedCloudSolrClient; + } + return httpJdkBasedCloudSolrClient; } @Test From dc9f389744def072a91064492a85d28633d7c735 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Tue, 14 Oct 2025 14:48:15 -0500 Subject: [PATCH 06/27] make it final --- .../org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index 54ca3b8b8b7d..367c7a157d9f 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -391,7 +391,7 @@ protected boolean maybeTryHeadRequest(String url) { return maybeTryHeadRequestSync(uriNoQueryParams); } - protected Map headSucceededByBaseUri = new HashMap<>(); // use only in synchronized method + protected final Map headSucceededByBaseUri = new HashMap<>(); // use only in synchronized method private synchronized boolean maybeTryHeadRequestSync(URI uriNoQueryParams) { Boolean headSucceeded = headSucceededByBaseUri.get(uriNoQueryParams); From d4bea5936b4d84668eb7377f9aa3ccf04c18ee73 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Tue, 14 Oct 2025 14:55:12 -0500 Subject: [PATCH 07/27] use jdk client somtimes in retry test --- .../solrj/impl/CloudHttp2SolrClientRetryTest.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index 39f10b2c324e..eaa905aa2f45 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java @@ -51,11 +51,15 @@ public static void setupCluster() throws Exception { @Test public void testRetry() throws Exception { - String collectionName = "testRetry"; - try (CloudSolrClient solrClient = - new CloudHttp2SolrClient.Builder( - Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) - .build()) { + + // Randomly decide to use either the Jetty Http Client or the JDK Http Client + var jettyClientBuilder = new Http2SolrClient.Builder(); + var jdkClientBuilder = new HttpJdkSolrClient.Builder().withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); + var cloudSolrclientBuilder = new CloudHttp2SolrClient.Builder(Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()); + cloudSolrclientBuilder.withHttpClientBuilder(random().nextBoolean() ? jettyClientBuilder : jdkClientBuilder); + + try (CloudSolrClient solrClient = cloudSolrclientBuilder.build()) { + String collectionName = "testRetry"; CollectionAdminRequest.createCollection(collectionName, 1, 1).process(solrClient); solrClient.add(collectionName, new SolrInputDocument("id", "1")); From 1fe02cb5795d7f7066604395485df8edfffe69f7 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Tue, 14 Oct 2025 14:56:54 -0500 Subject: [PATCH 08/27] tidy --- .../org/apache/solr/cli/HealthcheckTool.java | 1 - .../org/apache/solr/cli/RunExampleTool.java | 4 +- .../org/apache/solr/cloud/ZkController.java | 1 - .../UsingSolrJRefGuideExamplesTest.java | 1 - .../impl/SolrClientNodeStateProvider.java | 3 +- .../solrj/impl/CloudHttp2SolrClient.java | 37 ++++++++--------- .../client/solrj/impl/CloudSolrClient.java | 1 - .../client/solrj/impl/HttpJdkSolrClient.java | 6 +-- .../impl/CloudHttp2SolrClientBuilderTest.java | 7 ++-- .../impl/CloudHttp2SolrClientRetryTest.java | 10 +++-- .../solrj/impl/CloudHttp2SolrClientTest.java | 40 ++++++++++++++----- .../impl/CloudSolrClientBuilderTest.java | 21 +++++++--- .../solrj/impl/HttpClusterStateSSLTest.java | 3 +- .../solrj/impl/HttpJdkSolrClientTest.java | 6 ++- 14 files changed, 87 insertions(+), 54 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java index bcca704384bd..bf795a515965 100644 --- a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java +++ b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java @@ -34,7 +34,6 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; -import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.client.solrj.request.GenericSolrRequest; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.cloud.ClusterState; diff --git a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java index 148fa4df0ea5..5be8b5af4e00 100644 --- a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java +++ b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java @@ -46,7 +46,6 @@ import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; -import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.common.SolrException; import org.apache.solr.common.util.EnvUtils; import org.noggit.CharArr; @@ -631,7 +630,8 @@ protected void runCloudExample(CommandLine cli) throws Exception { /** wait until the number of live nodes == numNodes. */ protected void waitToSeeLiveNodes(String zkHost, int numNodes) { try (CloudSolrClient cloudClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), Optional.empty()) + .build()) { cloudClient.connect(); Set liveNodes = cloudClient.getClusterState().getLiveNodes(); int numLiveNodes = (liveNodes != null) ? liveNodes.size() : 0; diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index 603381ef99c7..bc41a64136ed 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -62,7 +62,6 @@ import org.apache.solr.client.solrj.cloud.SolrCloudManager; import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; -import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder; import org.apache.solr.client.solrj.impl.SolrClientCloudManager; import org.apache.solr.client.solrj.impl.SolrZkClientTimeout; diff --git a/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java b/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java index ad60010c125e..b9975eba2cca 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java +++ b/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java @@ -32,7 +32,6 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.beans.Field; import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; -import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.response.CollectionAdminResponse; diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java index dabe6b80bd3f..1f4e1468fd13 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java @@ -289,7 +289,8 @@ public SimpleSolrResponse invoke(String solrNode, String path, SolrParams params request.setResponseParser(new JavaBinResponseParser()); try { - return ((Http2SolrClient) cloudSolrClient.getHttpClient()).requestWithBaseUrl(url, request::process); //NOCOMMIT + return ((Http2SolrClient) cloudSolrClient.getHttpClient()) + .requestWithBaseUrl(url, request::process); // NOCOMMIT } catch (SolrServerException | IOException e) { throw new SolrException(ErrorCode.SERVER_ERROR, "Fetching replica metrics failed", e); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index d9b6aee78875..8a20a7b12f4d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -176,7 +176,7 @@ public static class Builder { protected boolean directUpdatesToLeadersOnly = false; protected boolean parallelUpdates = true; protected ClusterStateProvider stateProvider; - protected HttpSolrClientBuilderBase internalClientBuilder; + protected HttpSolrClientBuilderBase internalClientBuilder; private RequestWriter requestWriter; private ResponseParser responseParser; private long retryExpiryTimeNano = @@ -245,7 +245,7 @@ public Builder(ClusterStateProvider stateProvider) { } /** Whether to use the default ZK ACLs when building a ZK Client. */ - public Builder canUseZkACLs(boolean canUseZkACLs) { + public Builder canUseZkACLs(boolean canUseZkACLs) { this.canUseZkACLs = canUseZkACLs; return this; } @@ -257,7 +257,7 @@ public Builder canUseZkACLs(boolean canUseZkACLs) { * @see #sendUpdatesToAnyReplica * @see CloudSolrClient#isUpdatesToLeaders */ - public Builder sendUpdatesOnlyToShardLeaders() { + public Builder sendUpdatesOnlyToShardLeaders() { shardLeadersOnly = true; return this; } @@ -269,7 +269,7 @@ public Builder sendUpdatesOnlyToShardLeaders() { * @see #sendUpdatesOnlyToShardLeaders * @see CloudSolrClient#isUpdatesToLeaders */ - public Builder sendUpdatesToAnyReplica() { + public Builder sendUpdatesToAnyReplica() { shardLeadersOnly = false; return this; } @@ -284,7 +284,7 @@ public Builder sendUpdatesToAnyReplica() { * @see #sendDirectUpdatesToAnyShardReplica * @see CloudSolrClient#isDirectUpdatesToLeadersOnly */ - public Builder sendDirectUpdatesToShardLeadersOnly() { + public Builder sendDirectUpdatesToShardLeadersOnly() { directUpdatesToLeadersOnly = true; return this; } @@ -299,19 +299,19 @@ public Builder sendDirectUpdatesToShardLeadersOnly() { * @see #sendDirectUpdatesToShardLeadersOnly * @see CloudSolrClient#isDirectUpdatesToLeadersOnly */ - public Builder sendDirectUpdatesToAnyShardReplica() { + public Builder sendDirectUpdatesToAnyShardReplica() { directUpdatesToLeadersOnly = false; return this; } /** Provides a {@link RequestWriter} for created clients to use when handing requests. */ - public Builder withRequestWriter(RequestWriter requestWriter) { + public Builder withRequestWriter(RequestWriter requestWriter) { this.requestWriter = requestWriter; return this; } /** Provides a {@link ResponseParser} for created clients to use when handling requests. */ - public Builder withResponseParser(ResponseParser responseParser) { + public Builder withResponseParser(ResponseParser responseParser) { this.responseParser = responseParser; return this; } @@ -326,7 +326,7 @@ public Builder withResponseParser(ResponseParser responseParser) { * *

If not set, this defaults to 'true' and sends sub-requests in parallel. */ - public Builder withParallelUpdates(boolean parallelUpdates) { + public Builder withParallelUpdates(boolean parallelUpdates) { this.parallelUpdates = parallelUpdates; return this; } @@ -337,7 +337,7 @@ public Builder withParallelUpdates(boolean parallelUpdates) { * *

Defaults to 3. */ - public Builder withParallelCacheRefreshes(int parallelCacheRefreshesLocks) { + public Builder withParallelCacheRefreshes(int parallelCacheRefreshesLocks) { this.parallelCacheRefreshesLocks = parallelCacheRefreshesLocks; return this; } @@ -345,13 +345,13 @@ public Builder withParallelCacheRefreshes(int parallelCacheRefreshesLocks) { /** * This is the time to wait to re-fetch the state after getting the same state version from ZK */ - public Builder withRetryExpiryTime(long expiryTime, TimeUnit unit) { + public Builder withRetryExpiryTime(long expiryTime, TimeUnit unit) { this.retryExpiryTimeNano = TimeUnit.NANOSECONDS.convert(expiryTime, unit); return this; } /** Sets the default collection for request. */ - public Builder withDefaultCollection(String defaultCollection) { + public Builder withDefaultCollection(String defaultCollection) { this.defaultCollection = defaultCollection; return this; } @@ -361,7 +361,7 @@ public Builder withDefaultCollection(String defaultCollection) { * * @param timeToLive ttl value */ - public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { + public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { assert timeToLive > 0; this.timeToLiveSeconds = TimeUnit.SECONDS.convert(timeToLive, unit); return this; @@ -374,7 +374,7 @@ public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { * * @return this */ - public Builder withHttpClient(HttpSolrClientBase httpSolrClient) { + public Builder withHttpClient(HttpSolrClientBase httpSolrClient) { if (this.internalClientBuilder != null) { throw new IllegalStateException( "The builder can't accept an httpClient AND an internalClientBuilder, only one of those can be provided"); @@ -391,7 +391,7 @@ public Builder withHttpClient(HttpSolrClientBase httpSolrClient) { * @param internalClientBuilder the builder to use for creating the internal http client. * @return this */ - public Builder withHttpClientBuilder(HttpSolrClientBuilderBase internalClientBuilder) { + public Builder withHttpClientBuilder(HttpSolrClientBuilderBase internalClientBuilder) { if (this.httpClient != null) { throw new IllegalStateException( "The builder can't accept an httpClient AND an internalClientBuilder, only one of those can be provided"); @@ -401,7 +401,8 @@ public Builder withHttpClientBuilder(HttpSolrClientBuilderBase internalCli } @Deprecated(since = "9.10") - public Builder withInternalClientBuilder(HttpSolrClientBuilderBase internalClientBuilder) { + public Builder withInternalClientBuilder( + HttpSolrClientBuilderBase internalClientBuilder) { return withHttpClientBuilder(internalClientBuilder); } @@ -411,7 +412,7 @@ public Builder withInternalClientBuilder(HttpSolrClientBuilderBase interna * @param zkConnectTimeout timeout value * @param unit time unit */ - public Builder withZkConnectTimeout(int zkConnectTimeout, TimeUnit unit) { + public Builder withZkConnectTimeout(int zkConnectTimeout, TimeUnit unit) { this.zkConnectTimeout = Math.toIntExact(unit.toMillis(zkConnectTimeout)); return this; } @@ -422,7 +423,7 @@ public Builder withZkConnectTimeout(int zkConnectTimeout, TimeUnit unit) { * @param zkClientTimeout timeout value * @param unit time unit */ - public Builder withZkClientTimeout(int zkClientTimeout, TimeUnit unit) { + public Builder withZkClientTimeout(int zkClientTimeout, TimeUnit unit) { this.zkClientTimeout = Math.toIntExact(unit.toMillis(zkClientTimeout)); return this; } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index b75bee952f07..ebe1fddf0ee5 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -33,7 +33,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Random; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index 367c7a157d9f..811aff5901a6 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -40,7 +40,6 @@ import java.util.Objects; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; @@ -391,11 +390,12 @@ protected boolean maybeTryHeadRequest(String url) { return maybeTryHeadRequestSync(uriNoQueryParams); } - protected final Map headSucceededByBaseUri = new HashMap<>(); // use only in synchronized method + protected final Map headSucceededByBaseUri = + new HashMap<>(); // use only in synchronized method private synchronized boolean maybeTryHeadRequestSync(URI uriNoQueryParams) { Boolean headSucceeded = headSucceededByBaseUri.get(uriNoQueryParams); - if (headSucceeded !=null) { + if (headSucceeded != null) { return headSucceeded; } 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 f3e3804bc8ff..38c935eb1217 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 @@ -166,7 +166,7 @@ public void testProvideInternalJdkClientBuilder() throws IOException { when(http2ClientBuilder.build()).thenReturn(http2Client); CloudHttp2SolrClient.Builder clientBuilder = new CloudHttp2SolrClient.Builder( - Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClientBuilder(http2ClientBuilder); verify(http2ClientBuilder, never()).build(); try (CloudHttp2SolrClient client = clientBuilder.build()) { @@ -197,7 +197,7 @@ public void testProvideExternalJdkClient() throws IOException { HttpJdkSolrClient http2Client = mock(HttpJdkSolrClient.class); CloudHttp2SolrClient.Builder clientBuilder = new CloudHttp2SolrClient.Builder( - Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withHttpClient(http2Client); try (CloudHttp2SolrClient client = clientBuilder.build()) { assertEquals(http2Client, client.getHttpClient()); @@ -263,7 +263,8 @@ private void testHttpClientConsistency( public void testCustomStateProvider() throws IOException { ZkClientClusterStateProvider stateProvider = mock(ZkClientClusterStateProvider.class); - try (CloudSolrClient cloudSolrClient = new CloudHttp2SolrClient.Builder(stateProvider).build()) { + try (CloudSolrClient cloudSolrClient = + new CloudHttp2SolrClient.Builder(stateProvider).build()) { assertEquals(stateProvider, cloudSolrClient.getClusterStateProvider()); } verify(stateProvider, times(1)).close(); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index eaa905aa2f45..87ffa1d9baca 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java @@ -54,9 +54,13 @@ public void testRetry() throws Exception { // Randomly decide to use either the Jetty Http Client or the JDK Http Client var jettyClientBuilder = new Http2SolrClient.Builder(); - var jdkClientBuilder = new HttpJdkSolrClient.Builder().withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); - var cloudSolrclientBuilder = new CloudHttp2SolrClient.Builder(Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()); - cloudSolrclientBuilder.withHttpClientBuilder(random().nextBoolean() ? jettyClientBuilder : jdkClientBuilder); + var jdkClientBuilder = + new HttpJdkSolrClient.Builder().withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); + var cloudSolrclientBuilder = + new CloudHttp2SolrClient.Builder( + Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()); + cloudSolrclientBuilder.withHttpClientBuilder( + random().nextBoolean() ? jettyClientBuilder : jdkClientBuilder); try (CloudSolrClient solrClient = cloudSolrclientBuilder.build()) { String collectionName = "testRetry"; 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 bb521d43608f..244d4f300a6c 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 @@ -114,23 +114,41 @@ public static void setupCluster() throws Exception { final List solrUrls = new ArrayList<>(); solrUrls.add(cluster.getJettySolrRunner(0).getBaseUrl().toString()); - httpJettyBasedCloudSolrClient = new CloudHttp2SolrClient.Builder(solrUrls).withHttpClientBuilder(new Http2SolrClient.Builder()).build(); + httpJettyBasedCloudSolrClient = + new CloudHttp2SolrClient.Builder(solrUrls) + .withHttpClientBuilder(new Http2SolrClient.Builder()) + .build(); assertTrue(httpJettyBasedCloudSolrClient.getHttpClient() instanceof Http2SolrClient); - assertTrue(httpJettyBasedCloudSolrClient.getClusterStateProvider() instanceof Http2ClusterStateProvider); - assertTrue(((Http2ClusterStateProvider) httpJettyBasedCloudSolrClient.getClusterStateProvider()).getHttpClient() instanceof Http2SolrClient); - - httpJdkBasedCloudSolrClient = new CloudHttp2SolrClient.Builder(solrUrls).withHttpClientBuilder(new HttpJdkSolrClient.Builder().withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT)).build(); + assertTrue( + httpJettyBasedCloudSolrClient.getClusterStateProvider() + instanceof Http2ClusterStateProvider); + assertTrue( + ((Http2ClusterStateProvider) httpJettyBasedCloudSolrClient.getClusterStateProvider()) + .getHttpClient() + instanceof Http2SolrClient); + + httpJdkBasedCloudSolrClient = + new CloudHttp2SolrClient.Builder(solrUrls) + .withHttpClientBuilder( + new HttpJdkSolrClient.Builder() + .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT)) + .build(); assertTrue(httpJdkBasedCloudSolrClient.getHttpClient() instanceof HttpJdkSolrClient); - assertTrue(httpJdkBasedCloudSolrClient.getClusterStateProvider() instanceof Http2ClusterStateProvider); - assertTrue(((Http2ClusterStateProvider) httpJdkBasedCloudSolrClient.getClusterStateProvider()).getHttpClient() instanceof HttpJdkSolrClient); + assertTrue( + httpJdkBasedCloudSolrClient.getClusterStateProvider() + instanceof Http2ClusterStateProvider); + assertTrue( + ((Http2ClusterStateProvider) httpJdkBasedCloudSolrClient.getClusterStateProvider()) + .getHttpClient() + instanceof HttpJdkSolrClient); zkBasedCloudSolrClient = new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) .build(); assertTrue(zkBasedCloudSolrClient.getHttpClient() instanceof Http2SolrClient); - assertTrue(zkBasedCloudSolrClient.getClusterStateProvider() instanceof ZkClientClusterStateProvider); - + assertTrue( + zkBasedCloudSolrClient.getClusterStateProvider() instanceof ZkClientClusterStateProvider); } @AfterClass @@ -166,10 +184,10 @@ public static void tearDownAfterClass() throws Exception { /** Randomly return the cluster's ZK based CSC, or HttpClusterProvider based CSC. */ private CloudSolrClient getRandomClient() { int randInt = random().nextInt(2); - if(randInt==0) { + if (randInt == 0) { return zkBasedCloudSolrClient; } - if(randInt==1) { + if (randInt == 1) { return httpJettyBasedCloudSolrClient; } return httpJdkBasedCloudSolrClient; 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 4089faa454e9..10e9c68b033c 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,7 +34,9 @@ public class CloudSolrClientBuilderTest extends SolrTestCase { @Test public void testSingleZkHostSpecified() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build(); ) { + new CloudHttp2SolrClient.Builder( + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + .build(); ) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -48,7 +50,8 @@ public void testSeveralZkHostsSpecifiedSingly() throws IOException { final List zkHostList = new ArrayList<>(); zkHostList.add(ANY_ZK_HOST); zkHostList.add(ANY_OTHER_ZK_HOST); - try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { + try (CloudSolrClient createdClient = + new CloudHttp2SolrClient.Builder(zkHostList, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -63,7 +66,8 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { final ArrayList zkHosts = new ArrayList<>(); zkHosts.add(ANY_ZK_HOST); zkHosts.add(ANY_OTHER_ZK_HOST); - try (CloudSolrClient createdClient = new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { + try (CloudSolrClient createdClient = + new CloudHttp2SolrClient.Builder(zkHosts, Optional.of(ANY_CHROOT)).build()) { try (ZkClientClusterStateProvider zkClientClusterStateProvider = ZkClientClusterStateProvider.from(createdClient)) { final String clientZkHost = zkClientClusterStateProvider.getZkHost(); @@ -76,7 +80,9 @@ public void testSeveralZkHostsSpecifiedTogether() throws IOException { @Test public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder( + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + .build()) { assertTrue(createdClient.isUpdatesToLeaders()); } } @@ -84,7 +90,9 @@ public void testByDefaultConfiguresClientToSendUpdatesOnlyToShardLeaders() throw @Test public void testIsDirectUpdatesToLeadersOnlyDefault() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)).build()) { + new CloudHttp2SolrClient.Builder( + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + .build()) { assertFalse(createdClient.isDirectUpdatesToLeadersOnly()); } } @@ -104,7 +112,8 @@ public void test0Timeouts() throws IOException { @Test public void testDefaultCollectionPassedFromBuilderToClient() throws IOException { try (CloudSolrClient createdClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + new CloudHttp2SolrClient.Builder( + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withDefaultCollection("aCollection") .build()) { assertEquals("aCollection", createdClient.getDefaultCollection()); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java index d160a5d43da4..b791a0286d10 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateSSLTest.java @@ -83,7 +83,8 @@ public void testHttpClusterStateWithSSL() throws Exception { // verify the http derived cluster state (on the client side) agrees with what the server stored try (CloudSolrClient httpBasedCloudSolrClient = - new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())).build()) { + new CloudHttp2SolrClient.Builder(Collections.singletonList(url0.toExternalForm())) + .build()) { ClusterStateProvider csp = httpBasedCloudSolrClient.getClusterStateProvider(); assertTrue(csp instanceof Http2ClusterStateProvider); verifyUrlSchemeInClusterState(csp.getCollection(collectionId), expectedReplicas); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index 9bd418e7e6c4..8616cca08e95 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -526,14 +526,16 @@ private void queryToHelpJdkReleaseThreads(HttpJdkSolrClient client) throws Excep private void assertNoHeadRequestWithSsl(HttpJdkSolrClient client) { if (isSSLMode()) { - assertNull("The HEAD request should not be performed if using SSL.", client.headSucceededByBaseUri.get(baseUri())); + assertNull( + "The HEAD request should not be performed if using SSL.", + client.headSucceededByBaseUri.get(baseUri())); } } private URI baseUri() { try { return new URI(getBaseUrl()); - } catch(URISyntaxException e) { + } catch (URISyntaxException e) { throw new RuntimeException(e); } } From dd98e518a09d276a943949ed1b7e7cb5c62608e0 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 09:48:10 -0500 Subject: [PATCH 09/27] remove system.out.println --- .../solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index 87ffa1d9baca..0ff1e4bfc97f 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java @@ -79,7 +79,6 @@ public void testRetry() throws Exception { new GenericSolrRequest(METHOD.GET, "/admin/metrics", SolrRequestType.ADMIN, params); NamedList namedList = solrClient.request(metricsRequest); - System.out.println(namedList); NamedList metrics = (NamedList) namedList.get("metrics"); assertEquals(1L, metrics.get(updateRequestCountKey)); @@ -96,7 +95,6 @@ public void testRetry() throws Exception { } namedList = solrClient.request(metricsRequest); - System.out.println(namedList); metrics = (NamedList) namedList.get("metrics"); assertEquals(2L, metrics.get(updateRequestCountKey)); } From 06b72c94e1ac832f70c65dac130dd0de17beacaf Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 11:06:58 -0500 Subject: [PATCH 10/27] Require that sCNSP users pass-in CSC with Http2SolrCLient --- .../solr/client/solrj/impl/SolrClientNodeStateProvider.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java index 1f4e1468fd13..971d98749723 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java @@ -60,6 +60,9 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter private Map nodeVsTags = new HashMap<>(); public SolrClientNodeStateProvider(CloudHttp2SolrClient solrClient) { + if(!(solrClient.getHttpClient() instanceof Http2SolrClient)) { + throw new IllegalArgumentException("The passed-in Cloud Solr Client must delegate to " + Http2SolrClient.class); + } this.solrClient = solrClient; try { readReplicaDetails(); @@ -289,8 +292,7 @@ public SimpleSolrResponse invoke(String solrNode, String path, SolrParams params request.setResponseParser(new JavaBinResponseParser()); try { - return ((Http2SolrClient) cloudSolrClient.getHttpClient()) - .requestWithBaseUrl(url, request::process); // NOCOMMIT + return ((Http2SolrClient) cloudSolrClient.getHttpClient()).requestWithBaseUrl(url, request::process); } catch (SolrServerException | IOException e) { throw new SolrException(ErrorCode.SERVER_ERROR, "Fetching replica metrics failed", e); } From f4e8d04efcc081ddded1d64cf39b00971504709a Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 11:10:52 -0500 Subject: [PATCH 11/27] javadoc update --- .../solr/client/solrj/impl/CloudHttp2SolrClient.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index 8a20a7b12f4d..86dc7b739ede 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -30,9 +30,9 @@ import org.apache.solr.common.SolrException; /** - * SolrJ client class to communicate with SolrCloud using Http2SolrClient. Instances of this class - * communicate with Zookeeper to discover Solr endpoints for SolrCloud collections, and then use the - * {@link LBHttp2SolrClient} to issue requests. + * SolrJ client class to communicate with SolrCloud using an Http/2-capable Solr Client. Instances + * of this class communicate with Zookeeper to discover Solr endpoints for SolrCloud collections, + * and then use the {@link LBHttp2SolrClient} to issue requests. * * @since solr 8.0 */ @@ -50,7 +50,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient { * every shard in a collection, there is no single point of failure. Updates will be sent to shard * leaders by default. * - * @param builder a {@link Http2SolrClient.Builder} with the options used to create the client. + * @param builder a {@link CloudHttp2SolrClient.Builder} with the options used to create the client. */ protected CloudHttp2SolrClient(Builder builder) { super(builder.shardLeadersOnly, builder.parallelUpdates, builder.directUpdatesToLeadersOnly); From 35831c5a7dbae37ad63c571d40092900e7605952 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 11:35:49 -0500 Subject: [PATCH 12/27] tidy --- .../client/solrj/impl/SolrClientNodeStateProvider.java | 8 +++++--- .../solr/client/solrj/impl/CloudHttp2SolrClient.java | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java index 971d98749723..5959910e3301 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java @@ -60,8 +60,9 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter private Map nodeVsTags = new HashMap<>(); public SolrClientNodeStateProvider(CloudHttp2SolrClient solrClient) { - if(!(solrClient.getHttpClient() instanceof Http2SolrClient)) { - throw new IllegalArgumentException("The passed-in Cloud Solr Client must delegate to " + Http2SolrClient.class); + if (!(solrClient.getHttpClient() instanceof Http2SolrClient)) { + throw new IllegalArgumentException( + "The passed-in Cloud Solr Client must delegate to " + Http2SolrClient.class); } this.solrClient = solrClient; try { @@ -292,7 +293,8 @@ public SimpleSolrResponse invoke(String solrNode, String path, SolrParams params request.setResponseParser(new JavaBinResponseParser()); try { - return ((Http2SolrClient) cloudSolrClient.getHttpClient()).requestWithBaseUrl(url, request::process); + return ((Http2SolrClient) cloudSolrClient.getHttpClient()) + .requestWithBaseUrl(url, request::process); } catch (SolrServerException | IOException e) { throw new SolrException(ErrorCode.SERVER_ERROR, "Fetching replica metrics failed", e); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index 86dc7b739ede..137c8990e60b 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -50,7 +50,8 @@ public class CloudHttp2SolrClient extends CloudSolrClient { * every shard in a collection, there is no single point of failure. Updates will be sent to shard * leaders by default. * - * @param builder a {@link CloudHttp2SolrClient.Builder} with the options used to create the client. + * @param builder a {@link CloudHttp2SolrClient.Builder} with the options used to create the + * client. */ protected CloudHttp2SolrClient(Builder builder) { super(builder.shardLeadersOnly, builder.parallelUpdates, builder.directUpdatesToLeadersOnly); From 0e50350daf69ddec313781d4f1999ee42ddcd220 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 11:47:28 -0500 Subject: [PATCH 13/27] if not client or builder passed, decide between H2SC or HJSc based on if classpath contans Jetty Http --- .../solr/client/solrj/impl/CloudHttp2SolrClient.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index 137c8990e60b..0f9738d6e09b 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -18,6 +18,7 @@ package org.apache.solr.client.solrj.impl; import java.io.IOException; +import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -28,6 +29,8 @@ import org.apache.solr.client.solrj.request.RequestWriter; import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.common.SolrException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * SolrJ client class to communicate with SolrCloud using an Http/2-capable Solr Client. Instances @@ -38,6 +41,7 @@ */ @SuppressWarnings("serial") public class CloudHttp2SolrClient extends CloudSolrClient { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private final ClusterStateProvider stateProvider; private final LBHttp2SolrClient lbClient; @@ -83,6 +87,13 @@ private HttpSolrClientBase createOrGetHttpClientFromBuilder(Builder builder) { } else if (builder.internalClientBuilder != null) { return builder.internalClientBuilder.build(); } else { + try { + Class.forName("org.eclipse.jetty.client.HttpClient"); + } catch(ClassNotFoundException e) { + log.debug("Using {} as the delegate http client", HttpJdkSolrClient.class.getName()); + return new HttpJdkSolrClient.Builder().build(); + } + log.debug("Using {} as the delegate http client", Http2SolrClient.class.getName()); return new Http2SolrClient.Builder().build(); } } From 474fe5e455c660c1eba47ec20d99d9c2791e737d Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 11:49:00 -0500 Subject: [PATCH 14/27] tidy --- .../org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index 0f9738d6e09b..59cc42b6ff54 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -89,7 +89,7 @@ private HttpSolrClientBase createOrGetHttpClientFromBuilder(Builder builder) { } else { try { Class.forName("org.eclipse.jetty.client.HttpClient"); - } catch(ClassNotFoundException e) { + } catch (ClassNotFoundException e) { log.debug("Using {} as the delegate http client", HttpJdkSolrClient.class.getName()); return new HttpJdkSolrClient.Builder().build(); } From 9835ec0cbaaac773d7b146c80bf66c4bde7a6e0b Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 12:00:40 -0500 Subject: [PATCH 15/27] make precommit happy --- .../apache/solr/client/solrj/impl/CloudHttp2SolrClient.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index 59cc42b6ff54..48f1d2d8d7cd 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -90,10 +90,10 @@ private HttpSolrClientBase createOrGetHttpClientFromBuilder(Builder builder) { try { Class.forName("org.eclipse.jetty.client.HttpClient"); } catch (ClassNotFoundException e) { - log.debug("Using {} as the delegate http client", HttpJdkSolrClient.class.getName()); + log.debug("Using {} as the delegate http client", HttpJdkSolrClient.class); return new HttpJdkSolrClient.Builder().build(); } - log.debug("Using {} as the delegate http client", Http2SolrClient.class.getName()); + log.debug("Using {} as the delegate http client", Http2SolrClient.class); return new Http2SolrClient.Builder().build(); } } From 398a9b6f6b558d0297851e8aaeb71699279b8916 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 13:59:04 -0500 Subject: [PATCH 16/27] fix two more classes that require http2solrclient delegate w/csc --- .../solr/client/solrj/impl/NodeValueFetcher.java | 2 +- .../solrj/impl/SolrClientNodeStateProvider.java | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java index 30e614a61bcd..0c129e9c756c 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java @@ -181,7 +181,7 @@ private void getRemoteMetricsFromTags( String baseUrl = ctx.zkClientClusterStateProvider.getZkStateReader().getBaseUrlForNodeName(ctx.getNode()); SimpleSolrResponse rsp = - ctx.cloudSolrClient.getHttpClient().requestWithBaseUrl(baseUrl, req::process); + ctx.http2SolrClient().requestWithBaseUrl(baseUrl, req::process); // TODO come up with a better solution to stream this response instead of loading in memory try (InputStream prometheusStream = (InputStream) rsp.getResponse().get(STREAM_KEY)) { diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java index 792fa0f501bf..7e21f30daf1d 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java @@ -220,8 +220,7 @@ static void processMetricStream( try (InputStream in = (InputStream) - ctx.cloudSolrClient - .getHttpClient() + ctx.http2SolrClient() .requestWithBaseUrl(baseUrl, req::process) .getResponse() .get(STREAM_KEY)) { @@ -261,12 +260,20 @@ public boolean isNodeAlive(String node) { } public RemoteCallCtx(String node, CloudHttp2SolrClient cloudSolrClient) { + if (!(cloudSolrClient.getHttpClient() instanceof Http2SolrClient)) { + throw new IllegalArgumentException( + "The passed-in Cloud Solr Client must delegate to " + Http2SolrClient.class); + } this.node = node; this.cloudSolrClient = cloudSolrClient; this.zkClientClusterStateProvider = (ZkClientClusterStateProvider) cloudSolrClient.getClusterStateProvider(); } + protected Http2SolrClient http2SolrClient() { + return (Http2SolrClient) cloudSolrClient.getHttpClient(); + } + /** * Will attempt to call {@link #invoke(String, String, SolrParams)} up to five times, retrying * on any IO Exceptions From d2fbe96b686202d469eeac1170fe018b201ca229 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 14:04:25 -0500 Subject: [PATCH 17/27] flub the metrics assert a little! --- .../client/solrj/impl/CloudHttp2SolrClientRetryTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index f963ca8c012e..90a2619a92ff 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java @@ -50,8 +50,12 @@ public void testRetry() throws Exception { // Randomly decide to use either the Jetty Http Client or the JDK Http Client var jettyClientBuilder = new Http2SolrClient.Builder(); + + // forcing Http/1.1 to avoid an extra HEAD request with the first update. + // (This causes the counts to be 1 greater than what we test for here.) var jdkClientBuilder = - new HttpJdkSolrClient.Builder().withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); + new HttpJdkSolrClient.Builder().useHttp1_1(true).withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); + var cloudSolrclientBuilder = new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()); From 1bf822355ab4ba55873064b331f60215a9e057bb Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 14:09:22 -0500 Subject: [PATCH 18/27] tidy --- gradle/libs.versions.toml | 2 +- .../org/apache/solr/client/solrj/impl/NodeValueFetcher.java | 3 +-- .../solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java | 4 +++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 87ea8f0c9e8f..14435a7242f9 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -487,9 +487,9 @@ opentelemetry-exporter-sender-okhttp = { module = "io.opentelemetry:opentelemetr opentelemetry-runtime-telemetry = { module = "io.opentelemetry.instrumentation:opentelemetry-runtime-telemetry-java17", version.ref = "opentelemetry-runtime-telemetry" } opentelemetry-sdk = { module = "io.opentelemetry:opentelemetry-sdk", version.ref = "opentelemetry" } opentelemetry-sdk-extension-autoconfigure = { module = "io.opentelemetry:opentelemetry-sdk-extension-autoconfigure", version.ref = "opentelemetry" } +opentelemetry-sdk-metrics = { module = "io.opentelemetry:opentelemetry-sdk-metrics", version.ref = "opentelemetry" } opentelemetry-sdk-testing = { module = "io.opentelemetry:opentelemetry-sdk-testing", version.ref = "opentelemetry" } opentelemetry-sdk-trace = { module = "io.opentelemetry:opentelemetry-sdk-trace", version.ref = "opentelemetry" } -opentelemetry-sdk-metrics = { module = "io.opentelemetry:opentelemetry-sdk-metrics", version.ref = "opentelemetry" } osgi-annotation = { module = "org.osgi:osgi.annotation", version.ref = "osgi-annotation" } oshai-logging = { module = "io.github.oshai:kotlin-logging", version.ref = "oshai-logging" } # @keep transitive dependency for version alignment diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java index 0c129e9c756c..b9a2e439d51f 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java @@ -180,8 +180,7 @@ private void getRemoteMetricsFromTags( String baseUrl = ctx.zkClientClusterStateProvider.getZkStateReader().getBaseUrlForNodeName(ctx.getNode()); - SimpleSolrResponse rsp = - ctx.http2SolrClient().requestWithBaseUrl(baseUrl, req::process); + SimpleSolrResponse rsp = ctx.http2SolrClient().requestWithBaseUrl(baseUrl, req::process); // TODO come up with a better solution to stream this response instead of loading in memory try (InputStream prometheusStream = (InputStream) rsp.getResponse().get(STREAM_KEY)) { diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index 90a2619a92ff..8c703f7d5ce3 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java @@ -54,7 +54,9 @@ public void testRetry() throws Exception { // forcing Http/1.1 to avoid an extra HEAD request with the first update. // (This causes the counts to be 1 greater than what we test for here.) var jdkClientBuilder = - new HttpJdkSolrClient.Builder().useHttp1_1(true).withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); + new HttpJdkSolrClient.Builder() + .useHttp1_1(true) + .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); var cloudSolrclientBuilder = new CloudHttp2SolrClient.Builder( From da6654921aa0d9510d75962d9a4fdb39606a4d00 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 14:24:48 -0500 Subject: [PATCH 19/27] fix randomization bug - suggested by copilot --- .../apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 fd45460b8b11..002aff440ee7 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 @@ -181,7 +181,7 @@ public static void tearDownAfterClass() throws Exception { /** Randomly return the cluster's ZK based CSC, or HttpClusterProvider based CSC. */ private CloudSolrClient getRandomClient() { - int randInt = random().nextInt(2); + int randInt = random().nextInt(3); if (randInt == 0) { return zkBasedCloudSolrClient; } From 26ad6a9f8c5f543d20fc1cf6de405f521e5d391f Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 14:40:21 -0500 Subject: [PATCH 20/27] really strip of query params - suggested by copilot --- .../org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index 811aff5901a6..5e01d35ed601 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -382,7 +382,9 @@ protected boolean maybeTryHeadRequest(String url) { } URI uriNoQueryParams; try { - uriNoQueryParams = new URI(url); + var uriWithParams = new URI(url); + var uriNoPQueryParamsStr = uriWithParams.getScheme() + "://" + uriWithParams.getAuthority() + uriWithParams.getPath(); + uriNoQueryParams = new URI(uriNoPQueryParamsStr); } catch (URISyntaxException e) { // If the url is invalid, let a subsequent request try again. return false; From 10de7991f524e8ddb314f9dc5f9d690ac0288d72 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 14:41:41 -0500 Subject: [PATCH 21/27] null the variable on teardown - suggested by copilot --- .../apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java | 1 + 1 file changed, 1 insertion(+) 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 002aff440ee7..e5224d3b15ad 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 @@ -176,6 +176,7 @@ public static void tearDownAfterClass() throws Exception { shutdownCluster(); httpJettyBasedCloudSolrClient = null; + httpJdkBasedCloudSolrClient = null; zkBasedCloudSolrClient = null; } From 042eae64ff5eca5027d494f1d42aaba33e7f7854 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 15 Oct 2025 14:53:30 -0500 Subject: [PATCH 22/27] tidy --- .../apache/solr/client/solrj/impl/HttpJdkSolrClient.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index 5e01d35ed601..fa7d4b75b511 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -383,7 +383,11 @@ protected boolean maybeTryHeadRequest(String url) { URI uriNoQueryParams; try { var uriWithParams = new URI(url); - var uriNoPQueryParamsStr = uriWithParams.getScheme() + "://" + uriWithParams.getAuthority() + uriWithParams.getPath(); + var uriNoPQueryParamsStr = + uriWithParams.getScheme() + + "://" + + uriWithParams.getAuthority() + + uriWithParams.getPath(); uriNoQueryParams = new URI(uriNoPQueryParamsStr); } catch (URISyntaxException e) { // If the url is invalid, let a subsequent request try again. From 6ca39b430824b230e09d0aaac650c2f596ebd511 Mon Sep 17 00:00:00 2001 From: James Dyer Date: Thu, 16 Oct 2025 14:52:11 -0500 Subject: [PATCH 23/27] Update solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../solr/client/solrj/impl/HttpJdkSolrClient.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index fa7d4b75b511..2464b4584941 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -383,12 +383,15 @@ protected boolean maybeTryHeadRequest(String url) { URI uriNoQueryParams; try { var uriWithParams = new URI(url); - var uriNoPQueryParamsStr = - uriWithParams.getScheme() - + "://" - + uriWithParams.getAuthority() - + uriWithParams.getPath(); - uriNoQueryParams = new URI(uriNoPQueryParamsStr); + uriNoQueryParams = new URI( + uriWithParams.getScheme(), + uriWithParams.getUserInfo(), + uriWithParams.getHost(), + uriWithParams.getPort(), + uriWithParams.getPath() == null ? "" : uriWithParams.getPath(), + null, + null + ); } catch (URISyntaxException e) { // If the url is invalid, let a subsequent request try again. return false; From fc91fbee469a7cc92d768b454a4fd34a206cdd05 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Thu, 16 Oct 2025 15:11:34 -0500 Subject: [PATCH 24/27] - make doc comments genric (suggested by copilot) - decide default delegate client class in static intalizer (suggested by dsmiley) - add unit test for the cae where jetty client shuld be default delegat client (cannot be tested so easy for djdk client) --- .../solrj/impl/CloudHttp2SolrClient.java | 27 ++++++++++++------- .../impl/CloudHttp2SolrClientBuilderTest.java | 11 ++++++++ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index 48f1d2d8d7cd..e2be8514f50e 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -48,6 +48,18 @@ public class CloudHttp2SolrClient extends CloudSolrClient { private final HttpSolrClientBase myClient; private final boolean clientIsInternal; + private static final boolean JETTY_CLIENT_AVAILABLE; + + static { + boolean jettyClientAvailable = true; + try { + Class.forName("org.eclipse.jetty.client.HttpClient"); + } catch (ClassNotFoundException e) { + jettyClientAvailable = false; + } + JETTY_CLIENT_AVAILABLE = jettyClientAvailable; + } + /** * Create a new client object that connects to Zookeeper and is always aware of the SolrCloud * state. If there is a fully redundant Zookeeper quorum and SolrCloud has enough replicas for @@ -86,15 +98,12 @@ private HttpSolrClientBase createOrGetHttpClientFromBuilder(Builder builder) { return builder.httpClient; } else if (builder.internalClientBuilder != null) { return builder.internalClientBuilder.build(); - } else { - try { - Class.forName("org.eclipse.jetty.client.HttpClient"); - } catch (ClassNotFoundException e) { - log.debug("Using {} as the delegate http client", HttpJdkSolrClient.class); - return new HttpJdkSolrClient.Builder().build(); - } + } else if(JETTY_CLIENT_AVAILABLE) { log.debug("Using {} as the delegate http client", Http2SolrClient.class); return new Http2SolrClient.Builder().build(); + } else { + log.debug("Using {} as the delegate http client", Http2SolrClient.class); + return new HttpJdkSolrClient.Builder().build(); } } @@ -380,7 +389,7 @@ public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { } /** - * Set the internal {@link Http2SolrClient}. + * Set the internal client Solr HTTP client. * *

Note: closing the client instance is the responsibility of the caller. * @@ -396,7 +405,7 @@ public Builder withHttpClient(HttpSolrClientBase httpSolrClient) { } /** - * If provided, the CloudHttp2SolrClient will build it's internal Http2SolrClient using this + * If provided, the CloudHttp2SolrClient will build it's internal client using this * builder (instead of the empty default one). Providing this builder allows users to configure * the internal clients (authentication, timeouts, etc.). * 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 38c935eb1217..327bace51314 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 @@ -206,6 +206,17 @@ public void testProvideExternalJdkClient() throws IOException { verify(http2Client, never()).close(); } + @Test + public void testDefaultClientUsesJetty() throws IOException { + try (CloudHttp2SolrClient createdClient = + new CloudHttp2SolrClient.Builder( + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + .build()) { + assertTrue(createdClient.getHttpClient() instanceof Http2SolrClient); + assertTrue(createdClient.getLbClient().solrClient instanceof Http2SolrClient); + } + } + @Test public void testDefaultCollectionPassedFromBuilderToClient() throws IOException { try (CloudHttp2SolrClient createdClient = From d47d92221e54332978631f728749343ad42c1cd9 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Thu, 16 Oct 2025 15:25:37 -0500 Subject: [PATCH 25/27] tidy --- .../solrj/impl/CloudHttp2SolrClient.java | 8 ++++---- .../client/solrj/impl/HttpJdkSolrClient.java | 18 +++++++++--------- .../impl/CloudHttp2SolrClientBuilderTest.java | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index e2be8514f50e..1c63a1d3705f 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -98,7 +98,7 @@ private HttpSolrClientBase createOrGetHttpClientFromBuilder(Builder builder) { return builder.httpClient; } else if (builder.internalClientBuilder != null) { return builder.internalClientBuilder.build(); - } else if(JETTY_CLIENT_AVAILABLE) { + } else if (JETTY_CLIENT_AVAILABLE) { log.debug("Using {} as the delegate http client", Http2SolrClient.class); return new Http2SolrClient.Builder().build(); } else { @@ -405,9 +405,9 @@ public Builder withHttpClient(HttpSolrClientBase httpSolrClient) { } /** - * If provided, the CloudHttp2SolrClient will build it's internal client using this - * builder (instead of the empty default one). Providing this builder allows users to configure - * the internal clients (authentication, timeouts, etc.). + * If provided, the CloudHttp2SolrClient will build it's internal client using this builder + * (instead of the empty default one). Providing this builder allows users to configure the + * internal clients (authentication, timeouts, etc.). * * @param internalClientBuilder the builder to use for creating the internal http client. * @return this diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index 2464b4584941..5f3fa63fe151 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -383,15 +383,15 @@ protected boolean maybeTryHeadRequest(String url) { URI uriNoQueryParams; try { var uriWithParams = new URI(url); - uriNoQueryParams = new URI( - uriWithParams.getScheme(), - uriWithParams.getUserInfo(), - uriWithParams.getHost(), - uriWithParams.getPort(), - uriWithParams.getPath() == null ? "" : uriWithParams.getPath(), - null, - null - ); + uriNoQueryParams = + new URI( + uriWithParams.getScheme(), + uriWithParams.getUserInfo(), + uriWithParams.getHost(), + uriWithParams.getPort(), + uriWithParams.getPath() == null ? "" : uriWithParams.getPath(), + null, + null); } catch (URISyntaxException e) { // If the url is invalid, let a subsequent request try again. return false; 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 327bace51314..6a59a6d7c72b 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 @@ -210,10 +210,10 @@ public void testProvideExternalJdkClient() throws IOException { public void testDefaultClientUsesJetty() throws IOException { try (CloudHttp2SolrClient createdClient = new CloudHttp2SolrClient.Builder( - Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) + Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .build()) { assertTrue(createdClient.getHttpClient() instanceof Http2SolrClient); - assertTrue(createdClient.getLbClient().solrClient instanceof Http2SolrClient); + assertTrue(createdClient.getLbClient().solrClient instanceof Http2SolrClient); } } From 2c83145bb32cfb16bca2782af17258decafb60fc Mon Sep 17 00:00:00 2001 From: James Dyer Date: Fri, 17 Oct 2025 10:37:39 -0500 Subject: [PATCH 26/27] Update solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java Co-authored-by: David Smiley --- .../org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index 1c63a1d3705f..58b29aceda31 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -102,7 +102,7 @@ private HttpSolrClientBase createOrGetHttpClientFromBuilder(Builder builder) { log.debug("Using {} as the delegate http client", Http2SolrClient.class); return new Http2SolrClient.Builder().build(); } else { - log.debug("Using {} as the delegate http client", Http2SolrClient.class); + log.debug("Using {} as the delegate http client", HttpJdkSolrClient.class); return new HttpJdkSolrClient.Builder().build(); } } From 7c5717f8d0953946975a928251f40fab818aacb1 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Fri, 17 Oct 2025 10:40:20 -0500 Subject: [PATCH 27/27] fix awkward wording --- .../org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java index 58b29aceda31..4b3ac046ac22 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java @@ -389,7 +389,7 @@ public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { } /** - * Set the internal client Solr HTTP client. + * Set the internal Solr HTTP client. * *

Note: closing the client instance is the responsibility of the caller. *