From de70e424d10f8cc54948a869970d6cc01e6925a6 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Mon, 19 May 2025 09:28:03 -0400 Subject: [PATCH 01/14] Harmonize SolrJ timeouts --- .../solr/client/solrj/io/SolrClientCache.java | 15 ++--- .../impl/ConcurrentUpdateHttp2SolrClient.java | 2 +- .../client/solrj/impl/Http2SolrClient.java | 58 ++++++++----------- .../client/solrj/impl/HttpJdkSolrClient.java | 20 +++---- .../client/solrj/impl/HttpSolrClientBase.java | 21 ++----- .../solrj/impl/HttpSolrClientBuilderBase.java | 31 ++++++---- .../solrj/impl/Http2SolrClientTest.java | 18 +++--- 7 files changed, 75 insertions(+), 90 deletions(-) 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 7550d1a35c4..b1a58c76852 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 @@ -205,19 +205,14 @@ private static Http2SolrClient.Builder newHttp2SolrClientBuilder( .withDefaultCollection(URLUtil.extractCoreFromCoreUrl(url)); if (http2SolrClient != null) { builder.withHttpClient(http2SolrClient); + // cannot set idleTimeout or connection timeout + // nocommit + } else { + builder.withIdleTimeout(minSocketTimeout, TimeUnit.MILLISECONDS); + builder.withConnectionTimeout(minConnTimeout, TimeUnit.MILLISECONDS); } builder.withOptionalBasicAuthCredentials(basicAuthCredentials); - long idleTimeout = minSocketTimeout; - if (builder.getIdleTimeoutMillis() != null) { - idleTimeout = Math.max(idleTimeout, builder.getIdleTimeoutMillis()); - } - builder.withIdleTimeout(idleTimeout, TimeUnit.MILLISECONDS); - long connTimeout = minConnTimeout; - if (builder.getConnectionTimeout() != null) { - connTimeout = Math.max(idleTimeout, builder.getConnectionTimeout()); - } - builder.withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS); return builder; } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java index 5603fac07e9..f995d589214 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java @@ -256,7 +256,7 @@ void sendUpdateStream() throws Exception { } Response response = - responseListener.get(client.getIdleTimeout(), TimeUnit.MILLISECONDS); + responseListener.get(client.getRequestTimeoutMillis(), TimeUnit.MILLISECONDS); rspBody = responseListener.getInputStream(); int statusCode = response.getStatus(); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java index 3f154cfdc01..95f4349360e 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java @@ -94,15 +94,12 @@ import org.slf4j.MDC; /** - * Difference between this {@link Http2SolrClient} and {@link HttpSolrClient}: + * An HTTP {@link SolrClient} using Jetty {@link HttpClient}. This is Solr's most mature client for + * direct HTTP. * - * + *

Despite the name, this client supports HTTP 1.1 and 2 -- toggle with {@link + * HttpSolrClientBuilderBase#useHttp1_1(boolean)}. In retrospect, the name should have been {@code + * HttpJettySolrClient}. */ public class Http2SolrClient extends HttpSolrClientBase { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -129,6 +126,12 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { super(serverBaseUrl, builder); if (builder.httpClient != null) { + if (builder.followRedirects != null + || builder.connectionTimeoutMillis != null + || builder.idleTimeoutMillis != null) { + throw new IllegalArgumentException( + "You cannot provide the HttpClient and also specify options that are used to build a new client"); + } this.httpClient = builder.httpClient; if (this.executor == null) { this.executor = builder.executor; @@ -252,7 +255,9 @@ private HttpClient createHttpClient(Builder builder) { httpClient.setMaxRequestsQueuedPerDestination( asyncTracker.getMaxRequestsQueuedPerDestination()); httpClient.setUserAgentField(new HttpField(HttpHeader.USER_AGENT, USER_AGENT)); - httpClient.setIdleTimeout(idleTimeoutMillis); + httpClient.setConnectTimeout(builder.getConnectionTimeoutMillis()); + httpClient.setIdleTimeout(builder.getIdleTimeoutMillis()); + // note: request timeout is set per request if (builder.cookieStore != null) { httpClient.setCookieStore(builder.cookieStore); @@ -261,8 +266,6 @@ private HttpClient createHttpClient(Builder builder) { this.authenticationStore = new AuthenticationStoreHolder(); httpClient.setAuthenticationStore(this.authenticationStore); - httpClient.setConnectTimeout(builder.connectionTimeoutMillis); - setupProxy(builder, httpClient); try { @@ -329,6 +332,11 @@ public void setAuthenticationStore(AuthenticationStore authenticationStore) { this.authenticationStore.updateAuthenticationStore(authenticationStore); } + /** (visible for testing) */ + public long getIdleTimeout() { + return getHttpClient().getIdleTimeout(); + } + public static class OutStream implements Closeable { private final String origCollection; private final SolrParams origParams; @@ -514,7 +522,7 @@ public NamedList request(SolrRequest solrRequest, String collection) try { InputStreamResponseListener listener = new InputStreamReleaseTrackingResponseListener(); req = sendRequest(makeRequest(solrRequest, url, false), listener); - Response response = listener.get(idleTimeoutMillis, TimeUnit.MILLISECONDS); + Response response = listener.get(requestTimeoutMillis, TimeUnit.MILLISECONDS); url = req.getURI().toString(); InputStream is = listener.getInputStream(); return processErrorsAndResponse(solrRequest, response, is, url); @@ -622,11 +630,8 @@ private void setBasicAuthHeader(SolrRequest solrRequest, Request req) { private void decorateRequest(Request req, SolrRequest solrRequest, boolean isAsync) { req.headers(headers -> headers.remove(HttpHeader.ACCEPT_ENCODING)); - if (requestTimeoutMillis > 0) { - req.timeout(requestTimeoutMillis, TimeUnit.MILLISECONDS); - } else { - req.timeout(idleTimeoutMillis, TimeUnit.MILLISECONDS); - } + req.timeout(requestTimeoutMillis, TimeUnit.MILLISECONDS); + if (solrRequest.getUserPrincipal() != null) { req.attribute(REQ_PRINCIPAL_KEY, solrRequest.getUserPrincipal()); } @@ -1020,12 +1025,6 @@ public Http2SolrClient build() { if (cookieStore == null) { cookieStore = getDefaultCookieStore(); } - if (idleTimeoutMillis == null || idleTimeoutMillis <= 0) { - idleTimeoutMillis = (long) HttpClientUtil.DEFAULT_SO_TIMEOUT; - } - if (connectionTimeoutMillis == null) { - connectionTimeoutMillis = (long) HttpClientUtil.DEFAULT_CONNECT_TIMEOUT; - } if (keyStoreReloadIntervalSecs != null && keyStoreReloadIntervalSecs > 0 @@ -1085,18 +1084,12 @@ public Builder withHttpClient(Http2SolrClient http2SolrClient) { if (this.basicAuthAuthorizationStr == null) { this.basicAuthAuthorizationStr = http2SolrClient.basicAuthAuthorizationStr; } - if (this.followRedirects == null) { - this.followRedirects = http2SolrClient.httpClient.isFollowRedirects(); - } - if (this.idleTimeoutMillis == null) { - this.idleTimeoutMillis = http2SolrClient.idleTimeoutMillis; + if (this.requestTimeoutMillis == null) { + this.requestTimeoutMillis = http2SolrClient.requestTimeoutMillis; } if (this.requestWriter == null) { this.requestWriter = http2SolrClient.requestWriter; } - if (this.requestTimeoutMillis == null) { - this.requestTimeoutMillis = http2SolrClient.requestTimeoutMillis; - } if (this.responseParser == null) { this.responseParser = http2SolrClient.parser; } @@ -1104,8 +1097,7 @@ public Builder withHttpClient(Http2SolrClient http2SolrClient) { this.urlParamNames = http2SolrClient.urlParamNames; } if (this.listenerFactory == null) { - this.listenerFactory = new ArrayList(); - http2SolrClient.listenerFactory.forEach(this.listenerFactory::add); + this.listenerFactory = new ArrayList<>(http2SolrClient.listenerFactory); } if (this.executor == null) { this.executor = http2SolrClient.executor; 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 c72708b39e7..3a3160da496 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 @@ -86,12 +86,18 @@ public class HttpJdkSolrClient extends HttpSolrClientBase { protected HttpJdkSolrClient(String serverBaseUrl, HttpJdkSolrClient.Builder builder) { super(serverBaseUrl, builder); + HttpClient.Builder b = HttpClient.newBuilder(); HttpClient.Redirect followRedirects = Boolean.TRUE.equals(builder.followRedirects) ? HttpClient.Redirect.NORMAL : HttpClient.Redirect.NEVER; - HttpClient.Builder b = HttpClient.newBuilder().followRedirects(followRedirects); + b.followRedirects(followRedirects); + + b.connectTimeout(Duration.of(builder.getConnectionTimeoutMillis(), ChronoUnit.MILLIS)); + // note: idle timeout isn't used for the JDK client + // note: request timeout is set per request + if (builder.sslContext != null) { b.sslContext(builder.sslContext); } @@ -419,11 +425,7 @@ private synchronized boolean maybeTryHeadRequestSync(String url) { } private void decorateRequest(HttpRequest.Builder reqb, SolrRequest solrRequest) { - if (requestTimeoutMillis > 0) { - reqb.timeout(Duration.of(requestTimeoutMillis, ChronoUnit.MILLIS)); - } else if (idleTimeoutMillis > 0) { - reqb.timeout(Duration.of(idleTimeoutMillis, ChronoUnit.MILLIS)); - } + reqb.timeout(Duration.of(requestTimeoutMillis, ChronoUnit.MILLIS)); reqb.header("User-Agent", USER_AGENT); setBasicAuthHeader(solrRequest, reqb); Map headers = solrRequest.getHeaders(); @@ -553,12 +555,6 @@ public Builder(String baseSolrUrl) { @Override public HttpJdkSolrClient build() { - if (idleTimeoutMillis == null || idleTimeoutMillis <= 0) { - idleTimeoutMillis = (long) HttpClientUtil.DEFAULT_SO_TIMEOUT; - } - if (connectionTimeoutMillis == null) { - connectionTimeoutMillis = (long) HttpClientUtil.DEFAULT_CONNECT_TIMEOUT; - } return new HttpJdkSolrClient(baseSolrUrl, this); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java index 15b7f12deba..584ad98c8c6 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java @@ -57,8 +57,6 @@ public abstract class HttpSolrClientBase extends SolrClient { /** The URL of the Solr server. */ protected final String serverBaseUrl; - protected final long idleTimeoutMillis; - protected final long requestTimeoutMillis; protected final Set urlParamNames; @@ -85,11 +83,7 @@ protected HttpSolrClientBase(String serverBaseUrl, HttpSolrClientBuilderBase getUrlParamNames() { return urlParamNames; } + + long getRequestTimeoutMillis() { + return requestTimeoutMillis; + } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java index 19025c9b6ec..cf630d02fbd 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java @@ -115,38 +115,49 @@ public B withMaxConnectionsPerHost(int max) { return (B) this; } + /** + * The max time a connection can be idle (that is, without traffic of bytes in either direction). + * Sometimes called a "socket timeout". Zero means infinite. Note: not applicable to the JDK + * HttpClient. + */ @SuppressWarnings("unchecked") public B withIdleTimeout(long idleConnectionTimeout, TimeUnit unit) { this.idleTimeoutMillis = TimeUnit.MILLISECONDS.convert(idleConnectionTimeout, unit); return (B) this; } - public Long getIdleTimeoutMillis() { - return idleTimeoutMillis; + public long getIdleTimeoutMillis() { + return idleTimeoutMillis != null + ? (idleTimeoutMillis > 0 ? idleTimeoutMillis : Long.MAX_VALUE) + : HttpClientUtil.DEFAULT_SO_TIMEOUT; } + /** The max time a connection can take to connect to destinations. Zero means infinite. */ @SuppressWarnings("unchecked") public B withConnectionTimeout(long connectionTimeout, TimeUnit unit) { this.connectionTimeoutMillis = TimeUnit.MILLISECONDS.convert(connectionTimeout, unit); return (B) this; } - public Long getConnectionTimeout() { - return connectionTimeoutMillis; + public long getConnectionTimeoutMillis() { + return connectionTimeoutMillis != null + ? (connectionTimeoutMillis > 0 ? connectionTimeoutMillis : Long.MAX_VALUE) + : HttpClientUtil.DEFAULT_CONNECT_TIMEOUT; } - /** - * Set a timeout in milliseconds for requests issued by this client. - * - * @param requestTimeout The timeout in milliseconds - * @return this Builder. - */ + /** Set a timeout for requests to receive a response. Zero means infinite. */ @SuppressWarnings("unchecked") public B withRequestTimeout(long requestTimeout, TimeUnit unit) { this.requestTimeoutMillis = TimeUnit.MILLISECONDS.convert(requestTimeout, unit); return (B) this; } + public long getRequestTimeoutMillis() { + return requestTimeoutMillis != null && requestTimeoutMillis >= 0 + ? (requestTimeoutMillis > 0 ? requestTimeoutMillis : Long.MAX_VALUE) + : getIdleTimeoutMillis(); + } + /** * If true, prefer http1.1 over http2. If not set, the default is determined by system property * 'solr.http1'. Otherwise, false. diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java index 43c04076c38..3ae16ca0704 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java @@ -661,14 +661,16 @@ public void testIdleTimeoutWithHttpClient() { assertEquals(oldClient.getIdleTimeout(), onlyBaseUrlChangedClient.getIdleTimeout()); assertEquals(oldClient.getHttpClient(), onlyBaseUrlChangedClient.getHttpClient()); } - try (Http2SolrClient idleTimeoutChangedClient = - new Http2SolrClient.Builder("baseSolrUrl") - .withHttpClient(oldClient) - .withIdleTimeout(3000, TimeUnit.MILLISECONDS) - .build()) { - assertFalse(oldClient.getIdleTimeout() == idleTimeoutChangedClient.getIdleTimeout()); - assertEquals(3000, idleTimeoutChangedClient.getIdleTimeout()); - } + + assertThrows( + "You cannot provide the HttpClient and also specify", // etc... + IllegalArgumentException.class, + () -> { + new Http2SolrClient.Builder("baseSolrUrl") + .withHttpClient(oldClient) + .withIdleTimeout(3000, TimeUnit.MILLISECONDS) + .build(); + }); } } From 20c0930486b218ceda4fc4ce48cd07db05506704 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 3 Jun 2025 09:38:38 -0400 Subject: [PATCH 02/14] Do the idle timeout at the request level. --- .../solr/client/solrj/io/SolrClientCache.java | 4 +-- .../client/solrj/impl/Http2SolrClient.java | 25 +++++++++++++------ 2 files changed, 20 insertions(+), 9 deletions(-) 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 b1a58c76852..fb7c2a55a70 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 @@ -208,8 +208,8 @@ private static Http2SolrClient.Builder newHttp2SolrClientBuilder( // cannot set idleTimeout or connection timeout // nocommit } else { - builder.withIdleTimeout(minSocketTimeout, TimeUnit.MILLISECONDS); - builder.withConnectionTimeout(minConnTimeout, TimeUnit.MILLISECONDS); + builder.withIdleTimeout(minSocketTimeout, TimeUnit.MILLISECONDS); + builder.withConnectionTimeout(minConnTimeout, TimeUnit.MILLISECONDS); } builder.withOptionalBasicAuthCredentials(basicAuthCredentials); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java index 95f4349360e..a8d5836145d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java @@ -111,6 +111,8 @@ public class Http2SolrClient extends HttpSolrClientBase { private final HttpClient httpClient; + private final long idleTimeoutMillis; + private List listenerFactory = new ArrayList<>(); protected AsyncTracker asyncTracker = new AsyncTracker(); @@ -128,7 +130,13 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { if (builder.httpClient != null) { if (builder.followRedirects != null || builder.connectionTimeoutMillis != null - || builder.idleTimeoutMillis != null) { + || builder.maxConnectionsPerHost != null + || builder.useHttp1_1 + != builder.httpClient.getTransport() instanceof HttpClientTransportOverHTTP + || builder.proxyHost != null + || builder.cookieStore != null + || builder.sslConfig != null + || builder.keyStoreReloadIntervalSecs != null) { throw new IllegalArgumentException( "You cannot provide the HttpClient and also specify options that are used to build a new client"); } @@ -147,8 +155,8 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { this.listenerFactory.addAll(builder.listenerFactory); } updateDefaultMimeTypeForParser(); - this.httpClient.setFollowRedirects(Boolean.TRUE.equals(builder.followRedirects)); + this.idleTimeoutMillis = builder.getIdleTimeoutMillis(); assert ObjectReleaseTracker.track(this); } @@ -256,8 +264,7 @@ private HttpClient createHttpClient(Builder builder) { asyncTracker.getMaxRequestsQueuedPerDestination()); httpClient.setUserAgentField(new HttpField(HttpHeader.USER_AGENT, USER_AGENT)); httpClient.setConnectTimeout(builder.getConnectionTimeoutMillis()); - httpClient.setIdleTimeout(builder.getIdleTimeoutMillis()); - // note: request timeout is set per request + // note: idle & request timeouts are set per request if (builder.cookieStore != null) { httpClient.setCookieStore(builder.cookieStore); @@ -334,7 +341,7 @@ public void setAuthenticationStore(AuthenticationStore authenticationStore) { /** (visible for testing) */ public long getIdleTimeout() { - return getHttpClient().getIdleTimeout(); + return idleTimeoutMillis; } public static class OutStream implements Closeable { @@ -522,7 +529,8 @@ public NamedList request(SolrRequest solrRequest, String collection) try { InputStreamResponseListener listener = new InputStreamReleaseTrackingResponseListener(); req = sendRequest(makeRequest(solrRequest, url, false), listener); - Response response = listener.get(requestTimeoutMillis, TimeUnit.MILLISECONDS); + // only waits for headers, so use the idle timeout + Response response = listener.get(idleTimeoutMillis, TimeUnit.MILLISECONDS); url = req.getURI().toString(); InputStream is = listener.getInputStream(); return processErrorsAndResponse(solrRequest, response, is, url); @@ -629,7 +637,7 @@ private void setBasicAuthHeader(SolrRequest solrRequest, Request req) { private void decorateRequest(Request req, SolrRequest solrRequest, boolean isAsync) { req.headers(headers -> headers.remove(HttpHeader.ACCEPT_ENCODING)); - + req.idleTimeout(idleTimeoutMillis, TimeUnit.MILLISECONDS); req.timeout(requestTimeoutMillis, TimeUnit.MILLISECONDS); if (solrRequest.getUserPrincipal() != null) { @@ -1084,6 +1092,9 @@ public Builder withHttpClient(Http2SolrClient http2SolrClient) { if (this.basicAuthAuthorizationStr == null) { this.basicAuthAuthorizationStr = http2SolrClient.basicAuthAuthorizationStr; } + if (this.idleTimeoutMillis == null) { + this.idleTimeoutMillis = http2SolrClient.idleTimeoutMillis; + } if (this.requestTimeoutMillis == null) { this.requestTimeoutMillis = http2SolrClient.requestTimeoutMillis; } From 1f212bf9a3f8f4bf8cf390782de9f5cd157fc7fc Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 3 Jun 2025 09:39:38 -0400 Subject: [PATCH 03/14] incorporate Luke's test --- .../solrj/impl/BasicHttpSolrClientTest.java | 20 +++++ .../solrj/impl/Http2SolrClientTest.java | 80 +++++++++++++++---- .../solrj/impl/HttpSolrClientTestBase.java | 5 ++ 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java index 2dd1466c882..8d02d256ed4 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.stream.IntStream; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -100,6 +101,24 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) } } + public static class SlowStreamServlet extends HttpServlet { + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + IntStream.range(0, 10) + .forEach( + i -> { + try { + Thread.sleep(500); + resp.getOutputStream().write(String.valueOf(i).getBytes()); + resp.getOutputStream().flush(); + } catch (IOException | InterruptedException e) { + throw new RuntimeException(e); + } + }); + } + } + public static class DebugServlet extends HttpServlet { public static void clear() { lastMethod = null; @@ -201,6 +220,7 @@ public static void beforeTest() throws Exception { .withServlet(new ServletHolder(RedirectServlet.class), "/redirect/*") .withServlet(new ServletHolder(SlowServlet.class), "/slow/*") .withServlet(new ServletHolder(DebugServlet.class), "/debug/*") + .withServlet(new ServletHolder(SlowStreamServlet.class), "/slowStream/*") .build(); createAndStartJetty(legacyExampleCollection1SolrHome(), jettyConfig); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java index 3ae16ca0704..06665edd846 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java @@ -17,10 +17,14 @@ package org.apache.solr.client.solrj.impl; +import static org.apache.solr.handler.admin.api.ReplicationAPIBase.FILE_STREAM; + import java.io.IOException; +import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.Collections; +import java.util.List; import java.util.concurrent.TimeUnit; import org.apache.solr.client.api.util.SolrVersion; import org.apache.solr.client.solrj.ResponseParser; @@ -35,6 +39,7 @@ import org.apache.solr.common.params.MapSolrParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.NamedList; import org.eclipse.jetty.client.WWWAuthenticationProtocolHandler; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.util.ssl.SslContextFactory; @@ -69,7 +74,7 @@ public void testTimeout() throws Exception { client.query(q, SolrRequest.METHOD.GET); fail("No exception thrown."); } catch (SolrServerException e) { - assertTrue(e.getMessage().contains("timeout") || e.getMessage().contains("Timeout")); + assertTrue(isTimeout(e)); } } @@ -100,7 +105,7 @@ public void testRequestTimeout() throws Exception { client.query(q, SolrRequest.METHOD.GET); fail("No exception thrown."); } catch (SolrServerException e) { - assertTrue(e.getMessage().contains("timeout") || e.getMessage().contains("Timeout")); + assertTrue(isTimeout(e)); } } @@ -651,28 +656,73 @@ public void testBuilder() { } @Test - public void testIdleTimeoutWithHttpClient() { + public void testIdleTimeoutWithHttpClient() throws Exception { + String url = getBaseUrl() + SLOW_STREAM_SERVLET_PATH; try (Http2SolrClient oldClient = - new Http2SolrClient.Builder("baseSolrUrl") - .withIdleTimeout(5000, TimeUnit.MILLISECONDS) + new Http2SolrClient.Builder(url) + .withRequestTimeout(Long.MAX_VALUE, TimeUnit.MILLISECONDS) + .withIdleTimeout(100, TimeUnit.MILLISECONDS) .build()) { + try (Http2SolrClient onlyBaseUrlChangedClient = - new Http2SolrClient.Builder("newBaseSolrUrl").withHttpClient(oldClient).build()) { + new Http2SolrClient.Builder(url).withHttpClient(oldClient).build()) { assertEquals(oldClient.getIdleTimeout(), onlyBaseUrlChangedClient.getIdleTimeout()); assertEquals(oldClient.getHttpClient(), onlyBaseUrlChangedClient.getHttpClient()); } - assertThrows( - "You cannot provide the HttpClient and also specify", // etc... - IllegalArgumentException.class, - () -> { - new Http2SolrClient.Builder("baseSolrUrl") - .withHttpClient(oldClient) - .withIdleTimeout(3000, TimeUnit.MILLISECONDS) - .build(); - }); + // too little time to succeed + QueryRequest req = new QueryRequest(); + req.setResponseParser(new InputStreamResponseParser(FILE_STREAM)); + assertExceptionThrownWithMessageContaining( + SolrServerException.class, List.of("Timeout"), () -> oldClient.request(req)); + + int newIdleTimeoutMs = 5 * 1000; // enough time to succeed + try (Http2SolrClient idleTimeoutChangedClient = + new Http2SolrClient.Builder(url) + .withHttpClient(oldClient) + .withIdleTimeout(newIdleTimeoutMs, TimeUnit.MILLISECONDS) + .build()) { + assertNotEquals(oldClient.getIdleTimeout(), idleTimeoutChangedClient.getIdleTimeout()); + assertEquals(newIdleTimeoutMs, idleTimeoutChangedClient.getIdleTimeout()); + NamedList response = idleTimeoutChangedClient.request(req); + try (InputStream is = (InputStream) response.get("stream")) { + assertEquals("0123456789", new String(is.readAllBytes())); + } + } } } + @Test + public void testRequestTimeoutWithHttpClient() throws Exception { + String url = getBaseUrl() + SLOW_STREAM_SERVLET_PATH; + try (Http2SolrClient oldClient = + new Http2SolrClient.Builder(url) + .withIdleTimeout(1000, TimeUnit.MILLISECONDS) + .withRequestTimeout(10, TimeUnit.SECONDS) + .build()) { + QueryRequest req = new QueryRequest(); + req.setResponseParser(new InputStreamResponseParser(FILE_STREAM)); + int newRequestTimeoutMs = 2000; + try (Http2SolrClient requestTimeoutChangedClient = + new Http2SolrClient.Builder(url) + .withHttpClient(oldClient) + .withRequestTimeout(newRequestTimeoutMs, TimeUnit.MILLISECONDS) + .build()) { + NamedList response = requestTimeoutChangedClient.request(req); + try (InputStream is = (InputStream) response.get("stream")) { + assertExceptionThrownWithMessageContaining( + IOException.class, List.of("Total timeout"), is::readAllBytes); + } + } + NamedList response = oldClient.request(req); + try (InputStream is = (InputStream) response.get("stream")) { + assertEquals("0123456789", new String(is.readAllBytes())); + } + } + } + + private static boolean isTimeout(SolrServerException e) { + return e.getMessage().contains("timeout") || e.getMessage().contains("Timeout"); + } /* Missed tests : - set cookies via interceptor - invariant params - compression */ } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java index f15521eb992..de8147e6983 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java @@ -63,6 +63,8 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase { protected static final String DEBUG_SERVLET_REGEX = DEBUG_SERVLET_PATH + "/*"; protected static final String REDIRECT_SERVLET_PATH = "/redirect"; protected static final String REDIRECT_SERVLET_REGEX = REDIRECT_SERVLET_PATH + "/*"; + protected static final String SLOW_STREAM_SERVLET_PATH = "/slowStream"; + protected static final String SLOW_STREAM_SERVLET_REGEX = SLOW_STREAM_SERVLET_PATH + "/*"; protected static final String COLLECTION_1 = "collection1"; // example chars that must be URI encoded - non-ASCII and curly quote protected static final String MUST_ENCODE = "\u1234\u007B"; @@ -77,6 +79,9 @@ public static void beforeTest() throws Exception { .withServlet( new ServletHolder(BasicHttpSolrClientTest.SlowServlet.class), SLOW_SERVLET_REGEX) .withServlet(new ServletHolder(DebugServlet.class), DEBUG_SERVLET_REGEX) + .withServlet( + new ServletHolder(BasicHttpSolrClientTest.SlowStreamServlet.class), + SLOW_STREAM_SERVLET_REGEX) .withSSLConfig(sslConfig.buildServerSSLConfig()) .build(); createAndStartJetty(legacyExampleCollection1SolrHome(), jettyConfig); From 73678f3df1962f41990278d5b55ed5a6b44d7ec5 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 3 Jun 2025 13:52:28 -0400 Subject: [PATCH 04/14] Don't set/override connection timeout --- solr/core/src/java/org/apache/solr/cloud/Overseer.java | 1 - solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java | 1 - solr/core/src/java/org/apache/solr/handler/IndexFetcher.java | 4 ---- .../org/apache/solr/client/solrj/io/SolrClientCache.java | 5 ++--- 4 files changed, 2 insertions(+), 9 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/Overseer.java b/solr/core/src/java/org/apache/solr/cloud/Overseer.java index 12b2cfa7206..91877fe790e 100644 --- a/solr/core/src/java/org/apache/solr/cloud/Overseer.java +++ b/solr/core/src/java/org/apache/solr/cloud/Overseer.java @@ -862,7 +862,6 @@ private void doCompatCheck(BiConsumer consumer) { new Http2SolrClient.Builder() .withHttpClient(getCoreContainer().getDefaultHttpSolrClient()) .withIdleTimeout(30000, TimeUnit.MILLISECONDS) - .withConnectionTimeout(15000, TimeUnit.MILLISECONDS) .build(); var client = new CloudHttp2SolrClient.Builder( diff --git a/solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java b/solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java index 2f9a3248b6b..a686743f6b5 100644 --- a/solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java +++ b/solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java @@ -360,7 +360,6 @@ private void requestRecovery( try (SolrClient client = new Http2SolrClient.Builder(baseUrl) .withHttpClient(solrClient) - .withConnectionTimeout(30000, TimeUnit.MILLISECONDS) .withIdleTimeout(120000, TimeUnit.MILLISECONDS) .build()) { client.request(recoverRequestCmd); diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 80c4ba6f1f9..eb5cf2f655f 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -179,8 +179,6 @@ public class IndexFetcher { private final Http2SolrClient solrClient; - private Integer connTimeout; - private Integer soTimeout; private boolean skipCommitOnLeaderVersionZero = true; @@ -258,7 +256,6 @@ private Http2SolrClient createSolrClient( .withHttpClient(updateShardHandler.getRecoveryOnlyHttpClient()) .withBasicAuthCredentials(httpBasicAuthUser, httpBasicAuthPassword) .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS) - .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS) .build(); } @@ -287,7 +284,6 @@ public IndexFetcher( String compress = (String) initArgs.get(COMPRESSION); useInternalCompression = ReplicationHandler.INTERNAL.equals(compress); useExternalCompression = ReplicationHandler.EXTERNAL.equals(compress); - connTimeout = getParameter(initArgs, HttpClientUtil.PROP_CONNECTION_TIMEOUT, 30000, null); soTimeout = getParameter(initArgs, HttpClientUtil.PROP_SO_TIMEOUT, 120000, null); String httpBasicAuthUser = (String) initArgs.get(HttpClientUtil.PROP_BASIC_AUTH_USER); 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 fb7c2a55a70..7ffcbc33e53 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 @@ -205,12 +205,11 @@ private static Http2SolrClient.Builder newHttp2SolrClientBuilder( .withDefaultCollection(URLUtil.extractCoreFromCoreUrl(url)); if (http2SolrClient != null) { builder.withHttpClient(http2SolrClient); - // cannot set idleTimeout or connection timeout - // nocommit + // cannot set connection timeout } else { - builder.withIdleTimeout(minSocketTimeout, TimeUnit.MILLISECONDS); builder.withConnectionTimeout(minConnTimeout, TimeUnit.MILLISECONDS); } + builder.withIdleTimeout(minSocketTimeout, TimeUnit.MILLISECONDS); builder.withOptionalBasicAuthCredentials(basicAuthCredentials); return builder; From 7cb825132d5437a377fbd2cbed8abeeb0866efd6 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 3 Jun 2025 23:11:31 -0400 Subject: [PATCH 05/14] test: fix charset --- .http | 2 ++ proxy.todo.txt | 14 ++++++++++++++ .../client/solrj/impl/BasicHttpSolrClientTest.java | 3 ++- .../client/solrj/impl/Http2SolrClientTest.java | 4 ++-- 4 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 .http create mode 100644 proxy.todo.txt diff --git a/.http b/.http new file mode 100644 index 00000000000..76dc5357200 --- /dev/null +++ b/.http @@ -0,0 +1,2 @@ +### +POST http://localhost:8983/solr/.system/select?q=*:* \ No newline at end of file diff --git a/proxy.todo.txt b/proxy.todo.txt new file mode 100644 index 00000000000..94dae158027 --- /dev/null +++ b/proxy.todo.txt @@ -0,0 +1,14 @@ + +* metrics tracking (separate from default metrics; different name) +* client instance (separate from default instance) +* tracing integration +* auth integration + +separate: +* Http2SolrClient shouldn't use the "idleTimeout" as a default to requestTimeout not existing for the request timeout. Just let it default to whatever Jetty does. + Q: what typically happens today; is requestTimeout almost always used, rendering this a minor matter? + +* Jetty Request.Listener registration -- want this done on the client. Simplifies Http2SolrClient + and then HttpClient could be shared to a Jetty ProxyServlet (doesn't know about Http2SolrClient), and + get our listeners that provide metrics, tracing, and auth. Makes what Solr has less special. + BUT: can we have per-request lifecycle? diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java index 3d267fea9b3..a90cc8c14ef 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java @@ -24,6 +24,7 @@ import java.io.InputStream; import java.lang.invoke.MethodHandles; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -110,7 +111,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO i -> { try { Thread.sleep(500); - resp.getOutputStream().write(String.valueOf(i).getBytes()); + resp.getOutputStream().write(String.valueOf(i).getBytes(StandardCharsets.UTF_8)); resp.getOutputStream().flush(); } catch (IOException | InterruptedException e) { throw new RuntimeException(e); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java index 06665edd846..484bb38e207 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java @@ -686,7 +686,7 @@ public void testIdleTimeoutWithHttpClient() throws Exception { assertEquals(newIdleTimeoutMs, idleTimeoutChangedClient.getIdleTimeout()); NamedList response = idleTimeoutChangedClient.request(req); try (InputStream is = (InputStream) response.get("stream")) { - assertEquals("0123456789", new String(is.readAllBytes())); + assertEquals("0123456789", new String(is.readAllBytes(), StandardCharsets.UTF_8)); } } } @@ -716,7 +716,7 @@ public void testRequestTimeoutWithHttpClient() throws Exception { } NamedList response = oldClient.request(req); try (InputStream is = (InputStream) response.get("stream")) { - assertEquals("0123456789", new String(is.readAllBytes())); + assertEquals("0123456789", new String(is.readAllBytes(), StandardCharsets.UTF_8)); } } } From 2e0e471dc6f662371eb0e2f00b7cd5afd61a3664 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 3 Jun 2025 23:13:19 -0400 Subject: [PATCH 06/14] Don't initialize stuff like sslConfig if an httpClient was provided. And move the builder check. --- .../client/solrj/impl/Http2SolrClient.java | 53 +++++++++---------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java index fc0543b0497..1c70a27b727 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java @@ -128,18 +128,6 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { super(serverBaseUrl, builder); if (builder.httpClient != null) { - if (builder.followRedirects != null - || builder.connectionTimeoutMillis != null - || builder.maxConnectionsPerHost != null - || builder.useHttp1_1 - != builder.httpClient.getTransport() instanceof HttpClientTransportOverHTTP - || builder.proxyHost != null - || builder.cookieStore != null - || builder.sslConfig != null - || builder.keyStoreReloadIntervalSecs != null) { - throw new IllegalArgumentException( - "You cannot provide the HttpClient and also specify options that are used to build a new client"); - } this.httpClient = builder.httpClient; if (this.executor == null) { this.executor = builder.executor; @@ -1029,22 +1017,31 @@ protected B build(Class type) { @Override public Http2SolrClient build() { - if (sslConfig == null) { - sslConfig = Http2SolrClient.defaultSSLConfig; - } - if (cookieStore == null) { - cookieStore = getDefaultCookieStore(); - } - - if (keyStoreReloadIntervalSecs != null - && keyStoreReloadIntervalSecs > 0 - && this.httpClient != null) { - log.warn("keyStoreReloadIntervalSecs can't be set when using external httpClient"); - keyStoreReloadIntervalSecs = null; - } else if (keyStoreReloadIntervalSecs == null - && this.httpClient == null - && Boolean.getBoolean("solr.keyStoreReload.enabled")) { - keyStoreReloadIntervalSecs = Long.getLong("solr.jetty.sslContext.reload.scanInterval", 30); + if (httpClient == null) { + // set defaults for building an httpClient... + if (sslConfig == null) { + sslConfig = Http2SolrClient.defaultSSLConfig; + } + if (cookieStore == null) { + cookieStore = getDefaultCookieStore(); + } + if (keyStoreReloadIntervalSecs == null + && Boolean.getBoolean("solr.keyStoreReload.enabled")) { + keyStoreReloadIntervalSecs = + Long.getLong("solr.jetty.sslContext.reload.scanInterval", 30); + } + } else { + if (followRedirects != null + || connectionTimeoutMillis != null + || maxConnectionsPerHost != null + || useHttp1_1 != httpClient.getTransport() instanceof HttpClientTransportOverHTTP + || proxyHost != null + || sslConfig != null + || cookieStore != null + || keyStoreReloadIntervalSecs != null) { + throw new IllegalArgumentException( + "You cannot provide the HttpClient and also specify options that are used to build a new client"); + } } Http2SolrClient client = new Http2SolrClient(baseSolrUrl, this); From d9934767058e870e931c6bba64658f0264999f20 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 3 Jun 2025 23:24:35 -0400 Subject: [PATCH 07/14] Avoid Long.MAX_VALUE for milliseconds to avoid overflow --- .../client/solrj/impl/HttpSolrClientBuilderBase.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java index cf630d02fbd..6e10a82daf2 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java @@ -25,6 +25,12 @@ public abstract class HttpSolrClientBuilderBase< B extends HttpSolrClientBuilderBase, C extends HttpSolrClientBase> { + /** + * About 24 days in milliseconds -- basically forever to wait for something. But not so large to + * cause overflow. + */ + protected static final long FOREVER_MILLIS = Integer.MAX_VALUE; + protected Long idleTimeoutMillis; protected Long connectionTimeoutMillis; protected Long requestTimeoutMillis; @@ -128,7 +134,7 @@ public B withIdleTimeout(long idleConnectionTimeout, TimeUnit unit) { public long getIdleTimeoutMillis() { return idleTimeoutMillis != null - ? (idleTimeoutMillis > 0 ? idleTimeoutMillis : Long.MAX_VALUE) + ? (idleTimeoutMillis > 0 ? idleTimeoutMillis : FOREVER_MILLIS) : HttpClientUtil.DEFAULT_SO_TIMEOUT; } @@ -141,7 +147,7 @@ public B withConnectionTimeout(long connectionTimeout, TimeUnit unit) { public long getConnectionTimeoutMillis() { return connectionTimeoutMillis != null - ? (connectionTimeoutMillis > 0 ? connectionTimeoutMillis : Long.MAX_VALUE) + ? (connectionTimeoutMillis > 0 ? connectionTimeoutMillis : FOREVER_MILLIS) : HttpClientUtil.DEFAULT_CONNECT_TIMEOUT; } @@ -154,7 +160,7 @@ public B withRequestTimeout(long requestTimeout, TimeUnit unit) { public long getRequestTimeoutMillis() { return requestTimeoutMillis != null && requestTimeoutMillis >= 0 - ? (requestTimeoutMillis > 0 ? requestTimeoutMillis : Long.MAX_VALUE) + ? (requestTimeoutMillis > 0 ? requestTimeoutMillis : FOREVER_MILLIS) : getIdleTimeoutMillis(); } From 253ee437b85694cd215597756f9a0c66e65a0f1d Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 3 Jun 2025 23:43:13 -0400 Subject: [PATCH 08/14] undo accidental commit of my notes --- .http | 2 -- proxy.todo.txt | 14 -------------- 2 files changed, 16 deletions(-) delete mode 100644 .http delete mode 100644 proxy.todo.txt diff --git a/.http b/.http deleted file mode 100644 index 76dc5357200..00000000000 --- a/.http +++ /dev/null @@ -1,2 +0,0 @@ -### -POST http://localhost:8983/solr/.system/select?q=*:* \ No newline at end of file diff --git a/proxy.todo.txt b/proxy.todo.txt deleted file mode 100644 index 94dae158027..00000000000 --- a/proxy.todo.txt +++ /dev/null @@ -1,14 +0,0 @@ - -* metrics tracking (separate from default metrics; different name) -* client instance (separate from default instance) -* tracing integration -* auth integration - -separate: -* Http2SolrClient shouldn't use the "idleTimeout" as a default to requestTimeout not existing for the request timeout. Just let it default to whatever Jetty does. - Q: what typically happens today; is requestTimeout almost always used, rendering this a minor matter? - -* Jetty Request.Listener registration -- want this done on the client. Simplifies Http2SolrClient - and then HttpClient could be shared to a Jetty ProxyServlet (doesn't know about Http2SolrClient), and - get our listeners that provide metrics, tracing, and auth. Makes what Solr has less special. - BUT: can we have per-request lifecycle? From beb432afba379ce079e4dd1351b26189ce850146 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Wed, 4 Jun 2025 01:17:01 -0400 Subject: [PATCH 09/14] revert CUSC --- .../client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java index 7da742a727b..58f59eb8eaf 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java @@ -255,8 +255,9 @@ void sendUpdateStream() throws Exception { responseListener = out.getResponseListener(); } + // just wait for the headers, so the idle timeout is sensible Response response = - responseListener.get(client.getRequestTimeoutMillis(), TimeUnit.MILLISECONDS); + responseListener.get(client.getIdleTimeout(), TimeUnit.MILLISECONDS); rspBody = responseListener.getInputStream(); int statusCode = response.getStatus(); From 454a667232ed5b37d0aadfd515edcc9aaf14b8d5 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 5 Jun 2025 18:25:07 -0400 Subject: [PATCH 10/14] SolrClientCache: keep a higher idle timeout from the passed in client --- .../java/org/apache/solr/client/solrj/io/SolrClientCache.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 7ffcbc33e53..7e0756a0fa8 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 @@ -209,7 +209,8 @@ private static Http2SolrClient.Builder newHttp2SolrClientBuilder( } else { builder.withConnectionTimeout(minConnTimeout, TimeUnit.MILLISECONDS); } - builder.withIdleTimeout(minSocketTimeout, TimeUnit.MILLISECONDS); + builder.withIdleTimeout( + Math.max(minSocketTimeout, builder.getIdleTimeoutMillis()), TimeUnit.MILLISECONDS); builder.withOptionalBasicAuthCredentials(basicAuthCredentials); return builder; From cd47a622f2f8458c0605a6c7b1a5f965b4112eec Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 5 Jun 2025 18:38:01 -0400 Subject: [PATCH 11/14] unused --- .../org/apache/solr/client/solrj/impl/HttpSolrClientBase.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java index 584ad98c8c6..e1056c20e01 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java @@ -388,8 +388,4 @@ public ResponseParser getParser() { public Set getUrlParamNames() { return urlParamNames; } - - long getRequestTimeoutMillis() { - return requestTimeoutMillis; - } } From 1bafa6374f69d44f15f5fdf20cfeaa792cbb7ef2 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sat, 14 Jun 2025 12:07:33 -0400 Subject: [PATCH 12/14] Simplify defaulting. Don't need "FOREVER" --- .../solrj/impl/HttpSolrClientBuilderBase.java | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java index 6e10a82daf2..60495a04d0e 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java @@ -25,11 +25,6 @@ public abstract class HttpSolrClientBuilderBase< B extends HttpSolrClientBuilderBase, C extends HttpSolrClientBase> { - /** - * About 24 days in milliseconds -- basically forever to wait for something. But not so large to - * cause overflow. - */ - protected static final long FOREVER_MILLIS = Integer.MAX_VALUE; protected Long idleTimeoutMillis; protected Long connectionTimeoutMillis; @@ -133,8 +128,8 @@ public B withIdleTimeout(long idleConnectionTimeout, TimeUnit unit) { } public long getIdleTimeoutMillis() { - return idleTimeoutMillis != null - ? (idleTimeoutMillis > 0 ? idleTimeoutMillis : FOREVER_MILLIS) + return idleTimeoutMillis != null && idleTimeoutMillis > 0 + ? idleTimeoutMillis : HttpClientUtil.DEFAULT_SO_TIMEOUT; } @@ -146,8 +141,8 @@ public B withConnectionTimeout(long connectionTimeout, TimeUnit unit) { } public long getConnectionTimeoutMillis() { - return connectionTimeoutMillis != null - ? (connectionTimeoutMillis > 0 ? connectionTimeoutMillis : FOREVER_MILLIS) + return connectionTimeoutMillis != null && connectionTimeoutMillis > 0 + ? connectionTimeoutMillis : HttpClientUtil.DEFAULT_CONNECT_TIMEOUT; } @@ -159,8 +154,8 @@ public B withRequestTimeout(long requestTimeout, TimeUnit unit) { } public long getRequestTimeoutMillis() { - return requestTimeoutMillis != null && requestTimeoutMillis >= 0 - ? (requestTimeoutMillis > 0 ? requestTimeoutMillis : FOREVER_MILLIS) + return requestTimeoutMillis != null && requestTimeoutMillis > 0 + ? requestTimeoutMillis : getIdleTimeoutMillis(); } From b8b8cce09232cf320b1316a5bfed0c0900f31868 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Mon, 16 Jun 2025 20:23:39 -0400 Subject: [PATCH 13/14] 0 doesn't mean infinite anymore. CHANGES & docs --- solr/CHANGES.txt | 7 +++++++ .../upgrade-notes/pages/major-changes-in-solr-9.adoc | 8 ++++++++ .../solr/client/solrj/impl/HttpSolrClientBuilderBase.java | 7 +++---- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 9457ebce1da..87665948bdc 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -250,6 +250,13 @@ Improvements * SOLR-16951: Add PKI Auth Caching for both generation of the PKI Auth Tokens and validation of received tokens. The default PKI Auth validity TTL has been increased from 5 seconds to 10 seconds. (Houston Putman) +* SOLR-17776: SolrJ: If Http2SolrClient.Builder.withHttpClient is used, (and it's used more in 9.8, shifted from deprecated Apache HttpClient + based clients), settings affecting HttpClient construction cannot be customized; an IllegalStateException will now be thrown if you try. + The connection timeout cannot be customized, but idle & request timeouts can be. Callers (various places in Solr) no longer customize the connection timeout. + (David Smiley, Luke Kot-Zaniewski) + +* SOLR-17776: The HttpJdkSolrClient wasn't setting the connection timeout as per the builder configuration. (David Smiley) + Optimizations --------------------- * SOLR-17578: Remove ZkController internal core supplier, for slightly faster reconnection after Zookeeper session loss. (Pierre Salagnac) diff --git a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc index f9aca44cac0..52fe51678d1 100644 --- a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc +++ b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc @@ -67,6 +67,14 @@ It is always strongly recommended that you fully reindex your documents after a In Solr 8, it was possible to add docValues to a schema without re-indexing via `UninvertDocValuesMergePolicy`, an advanced/expert utility. Due to changes in Lucene 9, that isn't possible any more. +== Solr 9.9 + +=== SolrJ + +If Http2SolrClient.Builder.withHttpClient is used, (and it's used more in 9.8, shifted from deprecated Apache HttpClient based clients), settings affecting HttpClient construction cannot be customized; an IllegalStateException will now be thrown if you try. +The connection timeout cannot be customized, but idle & request timeouts can be. +Callers (various places in Solr) no longer customize the connection timeout. + == Solr 9.8.1 === Configuration diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java index 60495a04d0e..453f296cffc 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java @@ -118,8 +118,7 @@ public B withMaxConnectionsPerHost(int max) { /** * The max time a connection can be idle (that is, without traffic of bytes in either direction). - * Sometimes called a "socket timeout". Zero means infinite. Note: not applicable to the JDK - * HttpClient. + * Sometimes called a "socket timeout". Note: not applicable to the JDK HttpClient. */ @SuppressWarnings("unchecked") public B withIdleTimeout(long idleConnectionTimeout, TimeUnit unit) { @@ -133,7 +132,7 @@ public long getIdleTimeoutMillis() { : HttpClientUtil.DEFAULT_SO_TIMEOUT; } - /** The max time a connection can take to connect to destinations. Zero means infinite. */ + /** The max time a connection can take to connect to destinations. */ @SuppressWarnings("unchecked") public B withConnectionTimeout(long connectionTimeout, TimeUnit unit) { this.connectionTimeoutMillis = TimeUnit.MILLISECONDS.convert(connectionTimeout, unit); @@ -146,7 +145,7 @@ public long getConnectionTimeoutMillis() { : HttpClientUtil.DEFAULT_CONNECT_TIMEOUT; } - /** Set a timeout for requests to receive a response. Zero means infinite. */ + /** Set a timeout for requests to receive a response. */ @SuppressWarnings("unchecked") public B withRequestTimeout(long requestTimeout, TimeUnit unit) { this.requestTimeoutMillis = TimeUnit.MILLISECONDS.convert(requestTimeout, unit); From f823d03a27b55526b2aef73872bd977d83a41ad9 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Wed, 18 Jun 2025 09:40:06 -0400 Subject: [PATCH 14/14] build() should not modify the builder state (a test reused the same builder for 2 clients). So make build() trivial and do defaulting/checks in the constructor. --- .../client/solrj/impl/Http2SolrClient.java | 194 +++++++++--------- 1 file changed, 100 insertions(+), 94 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java index 1c70a27b727..060d75290da 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java @@ -128,6 +128,20 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { super(serverBaseUrl, builder); if (builder.httpClient != null) { + // Validate that no conflicting options are provided when using an existing HttpClient + if (builder.followRedirects != null + || builder.connectionTimeoutMillis != null + || builder.maxConnectionsPerHost != null + || builder.useHttp1_1 + != builder.httpClient.getTransport() instanceof HttpClientTransportOverHTTP + || builder.proxyHost != null + || builder.sslConfig != null + || builder.cookieStore != null + || builder.keyStoreReloadIntervalSecs != null) { + throw new IllegalArgumentException( + "You cannot provide the HttpClient and also specify options that are used to build a new client"); + } + this.httpClient = builder.httpClient; if (this.executor == null) { this.executor = builder.executor; @@ -146,6 +160,17 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) { this.httpClient.setFollowRedirects(Boolean.TRUE.equals(builder.followRedirects)); this.idleTimeoutMillis = builder.getIdleTimeoutMillis(); + try { + applyHttpClientBuilderFactory(); + } catch (RuntimeException e) { + try { + this.close(); + } catch (Exception exceptionOnClose) { + e.addSuppressed(exceptionOnClose); + } + throw e; + } + assert ObjectReleaseTracker.track(this); } @@ -157,6 +182,29 @@ private void initAuthStoreFromExistingClient(HttpClient httpClient) { this.authenticationStore = (AuthenticationStoreHolder) httpClient.getAuthenticationStore(); } + private void applyHttpClientBuilderFactory() { + String factoryClassName = + System.getProperty(HttpClientUtil.SYS_PROP_HTTP_CLIENT_BUILDER_FACTORY); + if (factoryClassName != null) { + log.debug("Using Http Builder Factory: {}", factoryClassName); + HttpClientBuilderFactory factory; + try { + factory = + Class.forName(factoryClassName) + .asSubclass(HttpClientBuilderFactory.class) + .getDeclaredConstructor() + .newInstance(); + } catch (InstantiationException + | IllegalAccessException + | ClassNotFoundException + | InvocationTargetException + | NoSuchMethodException e) { + throw new RuntimeException("Unable to instantiate " + Http2SolrClient.class.getName(), e); + } + factory.setup(this); + } + } + @Deprecated(since = "9.7") public void addListenerFactory(HttpListenerFactory factory) { this.listenerFactory.add(factory); @@ -173,8 +221,6 @@ ProtocolHandlers getProtocolHandlers() { } private HttpClient createHttpClient(Builder builder) { - HttpClient httpClient; - executor = builder.executor; if (executor == null) { BlockingArrayQueue queue = new BlockingArrayQueue<>(256, 256); @@ -186,21 +232,24 @@ private HttpClient createHttpClient(Builder builder) { shutdownExecutor = false; } - SslContextFactory.Client sslContextFactory; - if (builder.sslConfig == null) { - sslContextFactory = getDefaultSslContextFactory(); - } else { - sslContextFactory = builder.sslConfig.createClientContextFactory(); - } + SSLConfig sslConfig = + builder.sslConfig != null ? builder.sslConfig : Http2SolrClient.defaultSSLConfig; + SslContextFactory.Client sslContextFactory = + (sslConfig == null) + ? getDefaultSslContextFactory() + : sslConfig.createClientContextFactory(); + Long keyStoreReloadIntervalSecs = builder.keyStoreReloadIntervalSecs; + if (keyStoreReloadIntervalSecs == null && Boolean.getBoolean("solr.keyStoreReload.enabled")) { + keyStoreReloadIntervalSecs = Long.getLong("solr.jetty.sslContext.reload.scanInterval", 30); + } if (sslContextFactory != null && sslContextFactory.getKeyStoreResource() != null - && builder.keyStoreReloadIntervalSecs != null - && builder.keyStoreReloadIntervalSecs > 0) { + && keyStoreReloadIntervalSecs != null + && keyStoreReloadIntervalSecs > 0) { scanner = new KeyStoreScanner(sslContextFactory); try { - scanner.setScanInterval( - (int) Math.min(builder.keyStoreReloadIntervalSecs, Integer.MAX_VALUE)); + scanner.setScanInterval((int) Math.min(keyStoreReloadIntervalSecs, Integer.MAX_VALUE)); scanner.start(); if (log.isDebugEnabled()) { log.debug("Key Store Scanner started"); @@ -222,6 +271,7 @@ private HttpClient createHttpClient(Builder builder) { clientConnector.setSslContextFactory(sslContextFactory); clientConnector.setSelectors(2); + HttpClient httpClient; HttpClientTransport transport; if (builder.useHttp1_1) { if (log.isDebugEnabled()) { @@ -254,8 +304,9 @@ private HttpClient createHttpClient(Builder builder) { httpClient.setConnectTimeout(builder.getConnectionTimeoutMillis()); // note: idle & request timeouts are set per request - if (builder.cookieStore != null) { - httpClient.setHttpCookieStore(builder.cookieStore); + var cookieStore = builder.getCookieStore(); + if (cookieStore != null) { + httpClient.setHttpCookieStore(cookieStore); } this.authenticationStore = new AuthenticationStoreHolder(); @@ -283,15 +334,14 @@ private void setupProxy(Builder builder, HttpClient httpClient) { if (builder.proxyIsSocks4) { proxy = new Socks4Proxy(address, builder.proxyIsSecure); } else { - final Protocol protocol; - if (builder.useHttp1_1) { - protocol = HttpClientTransportOverHTTP.HTTP11; - } else { - // see HttpClientTransportOverHTTP2#newOrigin - String protocolName = builder.proxyIsSecure ? "h2" : "h2c"; - protocol = new Protocol(List.of(protocolName), false); - } - proxy = new HttpProxy(address, builder.proxyIsSecure, protocol); + // Move protocol initialization closer to where it's used + proxy = + new HttpProxy( + address, + builder.proxyIsSecure, + builder.useHttp1_1 + ? HttpClientTransportOverHTTP.HTTP11 + : new Protocol(List.of(builder.proxyIsSecure ? "h2" : "h2c"), false)); } httpClient.getProxyConfiguration().addProxy(proxy); } @@ -593,13 +643,18 @@ private NamedList processErrorsAndResponse( ResponseParser parser = solrRequest.getResponseParser() == null ? this.parser : solrRequest.getResponseParser(); String contentType = response.getHeaders().get(HttpHeader.CONTENT_TYPE); + + // Move variable initializations closer to where they are used + String responseMethod = response.getRequest() == null ? "" : response.getRequest().getMethod(); + + // Initialize mimeType and encoding only when needed String mimeType = null; String encoding = null; if (contentType != null) { mimeType = MimeTypes.getContentTypeWithoutCharset(contentType); encoding = MimeTypes.getCharsetFromContentType(contentType); } - String responseMethod = response.getRequest() == null ? "" : response.getRequest().getMethod(); + return processErrorsAndResponse( response.getStatus(), response.getReason(), @@ -692,22 +747,26 @@ private MakeRequestReturnValue makeRequest( if (SolrRequest.METHOD.POST == solrRequest.getMethod() || SolrRequest.METHOD.PUT == solrRequest.getMethod()) { - RequestWriter.ContentWriter contentWriter = requestWriter.getContentWriter(solrRequest); - Collection streams = - contentWriter == null ? requestWriter.getContentStreams(solrRequest) : null; - - boolean isMultipart = isMultipart(streams); - + // Move method initialization closer to where it's used HttpMethod method = SolrRequest.METHOD.POST == solrRequest.getMethod() ? HttpMethod.POST : HttpMethod.PUT; + RequestWriter.ContentWriter contentWriter = requestWriter.getContentWriter(solrRequest); + if (contentWriter != null) { var content = new OutputStreamRequestContent(contentWriter.getContentType()); var r = httpClient.newRequest(url + wparams.toQueryString()).method(method).body(content); decorateRequest(r, solrRequest, isAsync); return new MakeRequestReturnValue(r, contentWriter, content); + } + + // Only get streams if contentWriter is null + Collection streams = requestWriter.getContentStreams(solrRequest); + + // Move isMultipart initialization closer to where it's used + boolean isMultipart = isMultipart(streams); - } else if (streams == null || isMultipart) { + if (streams == null || isMultipart) { // send server list and request list as query string params ModifiableSolrParams queryParams = calculateQueryParams(this.urlParamNames, wparams); queryParams.add(calculateQueryParams(solrRequest.getQueryParams(), wparams)); @@ -765,14 +824,18 @@ private Request fillContentStream( } if (streams != null) { for (ContentStream contentStream : streams) { + // Move contentType initialization closer to where it's used String contentType = contentStream.getContentType(); if (contentType == null) { contentType = "multipart/form-data"; // default } + + // Move name initialization closer to where it's used String name = contentStream.getName(); if (name == null) { name = ""; } + HttpFields.Mutable fields = HttpFields.build(1); fields.add(HttpHeader.CONTENT_TYPE, contentType); content.addPart( @@ -787,6 +850,7 @@ private Request fillContentStream( } } else { // application/x-www-form-urlencoded + // Move queryString initialization into the else block where it's used String queryString = wparams.toQueryString(); // remove the leading "?" if there is any queryString = queryString.startsWith("?") ? queryString.substring(1) : queryString; @@ -1000,7 +1064,10 @@ public Http2SolrClient.Builder requestTimeout(int requestTimeout) { return this; } - private HttpCookieStore getDefaultCookieStore() { + private HttpCookieStore getCookieStore() { + if (cookieStore == null) { + return cookieStore; + } if (Boolean.getBoolean("solr.http.disableCookies")) { return new HttpCookieStore.Empty(); } @@ -1017,68 +1084,7 @@ protected B build(Class type) { @Override public Http2SolrClient build() { - if (httpClient == null) { - // set defaults for building an httpClient... - if (sslConfig == null) { - sslConfig = Http2SolrClient.defaultSSLConfig; - } - if (cookieStore == null) { - cookieStore = getDefaultCookieStore(); - } - if (keyStoreReloadIntervalSecs == null - && Boolean.getBoolean("solr.keyStoreReload.enabled")) { - keyStoreReloadIntervalSecs = - Long.getLong("solr.jetty.sslContext.reload.scanInterval", 30); - } - } else { - if (followRedirects != null - || connectionTimeoutMillis != null - || maxConnectionsPerHost != null - || useHttp1_1 != httpClient.getTransport() instanceof HttpClientTransportOverHTTP - || proxyHost != null - || sslConfig != null - || cookieStore != null - || keyStoreReloadIntervalSecs != null) { - throw new IllegalArgumentException( - "You cannot provide the HttpClient and also specify options that are used to build a new client"); - } - } - - Http2SolrClient client = new Http2SolrClient(baseSolrUrl, this); - try { - httpClientBuilderSetup(client); - } catch (RuntimeException e) { - try { - client.close(); - } catch (Exception exceptionOnClose) { - e.addSuppressed(exceptionOnClose); - } - throw e; - } - return client; - } - - private void httpClientBuilderSetup(Http2SolrClient client) { - String factoryClassName = - System.getProperty(HttpClientUtil.SYS_PROP_HTTP_CLIENT_BUILDER_FACTORY); - if (factoryClassName != null) { - log.debug("Using Http Builder Factory: {}", factoryClassName); - HttpClientBuilderFactory factory; - try { - factory = - Class.forName(factoryClassName) - .asSubclass(HttpClientBuilderFactory.class) - .getDeclaredConstructor() - .newInstance(); - } catch (InstantiationException - | IllegalAccessException - | ClassNotFoundException - | InvocationTargetException - | NoSuchMethodException e) { - throw new RuntimeException("Unable to instantiate " + Http2SolrClient.class.getName(), e); - } - factory.setup(client); - } + return new Http2SolrClient(baseSolrUrl, this); } /**