This is an automated email from the ASF dual-hosted git repository. jdyer pushed a commit to branch branch_10_0 in repository https://gitbox.apache.org/repos/asf/solr.git
commit 15112d1ec33b7f0c9aa0c5b7a20cf6ae9b5ee751 Author: James Dyer <[email protected]> AuthorDate: Fri Oct 17 10:59:10 2025 -0500 SOLR-17771: Have a CloudSolrClient that works with HttpJdkSolrClient (#3774) --- .../java/org/apache/solr/cli/RunExampleTool.java | 4 +- .../manager/consumer/KafkaCrossDcConsumer.java | 3 +- .../examples/UsingSolrJRefGuideExamplesTest.java | 8 +-- .../solr/client/solrj/impl/NodeValueFetcher.java | 3 +- .../solrj/impl/SolrClientNodeStateProvider.java | 18 +++++- .../client/solrj/impl/CloudHttp2SolrClient.java | 64 +++++++++++++++------- .../solr/client/solrj/impl/CloudSolrClient.java | 63 --------------------- .../solr/client/solrj/impl/HttpJdkSolrClient.java | 39 ++++++++----- .../impl/CloudHttp2SolrClientBuilderTest.java | 51 ++++++++++++++++- .../solrj/impl/CloudHttp2SolrClientRetryTest.java | 27 +++++++-- .../solrj/impl/CloudHttp2SolrClientTest.java | 64 +++++++++++++++++++--- .../solrj/impl/CloudSolrClientBuilderTest.java | 22 +++++--- .../impl/CloudSolrClientMultiConstructorTest.java | 4 +- .../client/solrj/impl/HttpClusterStateSSLTest.java | 3 +- .../client/solrj/impl/HttpJdkSolrClientTest.java | 20 +++++-- .../solr/cloud/AbstractFullDistribZkTestBase.java | 2 +- 16 files changed, 253 insertions(+), 142 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 06547d4eb19..5be8b5af4e0 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.Executor; 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,8 @@ public class RunExampleTool extends ToolBase { /** 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<String> 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 bf1ba691f26..7e6e9f1bd65 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.errors.WakeupException; 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 class KafkaCrossDcConsumer extends Consumer.CrossDcConsumer { 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 5cf438775a1..b9975eba2cc 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java +++ b/solr/solr-ref-guide/modules/deployment-guide/examples/UsingSolrJRefGuideExamplesTest.java @@ -31,7 +31,7 @@ import org.apache.solr.client.solrj.SolrQuery; 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.CloudSolrClient; +import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient; import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.response.CollectionAdminResponse; @@ -254,7 +254,7 @@ public class UsingSolrJRefGuideExamplesTest extends SolrCloudTestCase { final List<String> 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 +264,7 @@ public class UsingSolrJRefGuideExamplesTest extends SolrCloudTestCase { 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 +274,7 @@ public class UsingSolrJRefGuideExamplesTest extends SolrCloudTestCase { 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-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 30e614a61bc..b9a2e439d51 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 @@ public class NodeValueFetcher { String baseUrl = ctx.zkClientClusterStateProvider.getZkStateReader().getBaseUrlForNodeName(ctx.getNode()); - SimpleSolrResponse rsp = - ctx.cloudSolrClient.getHttpClient().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-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 8ae96ad9e9e..7e21f30daf1 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 @@ -59,6 +59,10 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter private Map<String, 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(); @@ -216,8 +220,7 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter try (InputStream in = (InputStream) - ctx.cloudSolrClient - .getHttpClient() + ctx.http2SolrClient() .requestWithBaseUrl(baseUrl, req::process) .getResponse() .get(STREAM_KEY)) { @@ -257,12 +260,20 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter } 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 @@ -311,7 +322,8 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter request.setResponseParser(new JavaBinResponseParser()); try { - return 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 028d8b675c5..4b3ac046ac2 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,29 +29,45 @@ import org.apache.solr.client.solrj.impl.SolrZkClientTimeout.SolrZkClientTimeout 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 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 */ @SuppressWarnings("serial") public class CloudHttp2SolrClient extends CloudSolrClient { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private final ClusterStateProvider stateProvider; - private final LBHttp2SolrClient<Http2SolrClient> lbClient; - private final Http2SolrClient myClient; + private final LBHttp2SolrClient<HttpSolrClientBase> lbClient; + 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 * 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); @@ -73,16 +90,20 @@ public class CloudHttp2SolrClient extends CloudSolrClient { // locks. this.locks = objectList(builder.parallelCacheRefreshesLocks); - this.lbClient = new LBHttp2SolrClient.Builder<Http2SolrClient>(myClient).build(); + this.lbClient = new LBHttp2SolrClient.Builder<>(myClient).build(); } - private Http2SolrClient createOrGetHttpClientFromBuilder(Builder builder) { + private HttpSolrClientBase createOrGetHttpClientFromBuilder(Builder builder) { if (builder.httpClient != null) { return builder.httpClient; } else if (builder.internalClientBuilder != null) { return builder.internalClientBuilder.build(); - } else { + } 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", HttpJdkSolrClient.class); + return new HttpJdkSolrClient.Builder().build(); } } @@ -113,7 +134,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient { } private ClusterStateProvider createHttp2ClusterStateProvider( - List<String> solrUrls, Http2SolrClient httpClient) { + List<String> solrUrls, HttpSolrClientBase httpClient) { try { return new Http2ClusterStateProvider<>(solrUrls, httpClient); } catch (Exception e) { @@ -148,7 +169,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient { } @Override - public LBHttp2SolrClient<Http2SolrClient> getLbClient() { + public LBHttp2SolrClient<?> getLbClient() { return lbClient; } @@ -157,7 +178,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient { return stateProvider; } - public Http2SolrClient getHttpClient() { + public HttpSolrClientBase getHttpClient() { return myClient; } @@ -171,12 +192,12 @@ public class CloudHttp2SolrClient extends CloudSolrClient { protected Collection<String> zkHosts = new ArrayList<>(); protected List<String> solrUrls = new ArrayList<>(); protected String zkChroot; - protected Http2SolrClient httpClient; + protected HttpSolrClientBase httpClient; protected boolean shardLeadersOnly = true; protected boolean directUpdatesToLeadersOnly = false; protected boolean parallelUpdates = true; protected ClusterStateProvider stateProvider; - protected Http2SolrClient.Builder internalClientBuilder; + protected HttpSolrClientBuilderBase<?, ?> internalClientBuilder; private RequestWriter requestWriter; private ResponseParser responseParser; private long retryExpiryTimeNano = @@ -368,13 +389,13 @@ public class CloudHttp2SolrClient extends CloudSolrClient { } /** - * Set the internal {@link Http2SolrClient}. + * Set the internal Solr HTTP client. * * <p>Note: closing the client instance is the responsibility of the caller. * * @return this */ - public Builder withHttpClient(Http2SolrClient 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"); @@ -384,14 +405,14 @@ public class CloudHttp2SolrClient extends CloudSolrClient { } /** - * If provided, the CloudHttp2SolrClient will build it's internal Http2SolrClient 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 */ - public Builder withHttpClientBuilder(Http2SolrClient.Builder 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 +422,8 @@ public class CloudHttp2SolrClient extends CloudSolrClient { } @Deprecated(since = "9.10") - public Builder withInternalClientBuilder(Http2SolrClient.Builder internalClientBuilder) { + public Builder withInternalClientBuilder( + HttpSolrClientBuilderBase<?, ?> internalClientBuilder) { return withHttpClientBuilder(internalClientBuilder); } 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 647071ef0bf..ebe1fddf0ee 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.Iterator; 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; @@ -120,68 +119,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. - * - * <p>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. - * - * <p>Usage example: - * - * <pre> - * 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(); - * </pre> - */ - public Builder(List<String> 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. - * - * <p>Usage example when Solr stores data at the ZooKeeper root ('/'): - * - * <pre> - * 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(); - * </pre> - * - * Usage example when Solr data is stored in a ZooKeeper chroot: - * - * <pre> - * 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(); - * </pre> - * - * @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<String> zkHosts, Optional<String> zkChroot) { - super(zkHosts, zkChroot); - } - - /** for an expert use-case */ - public Builder(ClusterStateProvider stateProvider) { - super(stateProvider); - } - } - static class StateCache extends ConcurrentHashMap<String, ExpiringCachedDocCollection> { final AtomicLong puts = new AtomicLong(); final AtomicLong hits = new AtomicLong(); 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 15e686d5773..5f3fa63fe15 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,6 +34,7 @@ import java.net.http.HttpTimeoutException; 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; @@ -379,24 +380,34 @@ public class HttpJdkSolrClient extends HttpSolrClientBase { 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); + var uriWithParams = new URI(url); + 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; } + return maybeTryHeadRequestSync(uriNoQueryParams); + } + + protected final Map<URI, Boolean> 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 +417,7 @@ public class HttpJdkSolrClient extends HttpSolrClientBase { 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 +425,7 @@ public class HttpJdkSolrClient extends HttpSolrClientBase { } 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/CloudHttp2SolrClientBuilderTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java index 2530beb34a0..6a59a6d7c72 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 @@ -141,7 +141,7 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { } @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); @@ -160,7 +160,26 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { } @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( @@ -173,6 +192,31 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { 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 + 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 = @@ -230,7 +274,8 @@ public class CloudHttp2SolrClientBuilderTest extends SolrCloudTestCase { 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/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index 07d829b2718..8c703f7d5ce 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 @@ -47,13 +47,28 @@ public class CloudHttp2SolrClientRetryTest extends SolrCloudTestCase { @Test public void testRetry() throws Exception { - String collectionName = "testRetry"; - String prometheusMetric = - "solr_core_requests_total{category=\"UPDATE\",collection=\"testRetry\",core=\"testRetry_shard1_replica_n1\",handler=\"/update\",otel_scope_name=\"org.apache.solr\",replica_type=\"NRT\",shard=\"shard1\"}"; - try (CloudSolrClient solrClient = + + // 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() + .useHttp1_1(true) + .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); + + var cloudSolrclientBuilder = new CloudHttp2SolrClient.Builder( - Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) - .build()) { + Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()); + cloudSolrclientBuilder.withHttpClientBuilder( + random().nextBoolean() ? jettyClientBuilder : jdkClientBuilder); + + try (CloudSolrClient solrClient = cloudSolrclientBuilder.build()) { + String collectionName = "testRetry"; + String prometheusMetric = + "solr_core_requests_total{category=\"UPDATE\",collection=\"testRetry\",core=\"testRetry_shard1_replica_n1\",handler=\"/update\",otel_scope_name=\"org.apache.solr\",replica_type=\"NRT\",shard=\"shard1\"}"; + CollectionAdminRequest.createCollection(collectionName, 1, 1).process(solrClient); solrClient.add(collectionName, new SolrInputDocument("id", "1")); 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 4fa485bd634..e5224d3b15a 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 @@ -92,8 +92,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 { @@ -110,19 +111,57 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { final List<String> 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( 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); } @@ -136,14 +175,21 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { } shutdownCluster(); - httpBasedCloudSolrClient = null; + httpJettyBasedCloudSolrClient = null; + httpJdkBasedCloudSolrClient = 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; + int randInt = random().nextInt(3); + if (randInt == 0) { + return zkBasedCloudSolrClient; + } + if (randInt == 1) { + return httpJettyBasedCloudSolrClient; + } + return httpJdkBasedCloudSolrClient; } @Test @@ -956,7 +1002,7 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { 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 348e8e1d65e..10e9c68b033 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.List; 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,9 @@ 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 +50,8 @@ public class CloudSolrClientBuilderTest extends SolrTestCase { final List<String> 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 +66,8 @@ public class CloudSolrClientBuilderTest extends SolrTestCase { final ArrayList<String> 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 +80,9 @@ public class CloudSolrClientBuilderTest extends SolrTestCase { @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 +90,9 @@ public class CloudSolrClientBuilderTest extends SolrTestCase { @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 +112,8 @@ public class CloudSolrClientBuilderTest extends SolrTestCase { @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 84c315637f3..dc540e49693 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 class CloudSolrClientMultiConstructorTest extends SolrTestCase { } 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 class CloudSolrClientMultiConstructorTest extends SolrTestCase { public void testBadChroot() { final List<String> 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 24e005c28c7..b791a0286d1 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 class HttpClusterStateSSLTest extends SolrCloudTestCase { // 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/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 e6a47680bc8..8616cca08e9 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 @@ package org.apache.solr.client.solrj.impl; 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 @@ public class HttpJdkSolrClientTest extends HttpSolrClientTestBase { 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 class HttpJdkSolrClientTest extends HttpSolrClientTestBase { 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,17 @@ public class HttpJdkSolrClientTest extends HttpSolrClientTestBase { 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); } } 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 70fdbc57a87..21e5c954d6f 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 @@ public abstract class AbstractFullDistribZkTestBase extends AbstractDistribZkTes /** * This method <i>may</i> 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,
