From 5261cd3cc75e97398291a3ba71c066813224ed14 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Wed, 4 Dec 2024 14:10:05 -0600 Subject: [PATCH 01/23] Initial development --- .../org/apache/solr/cloud/ZkController.java | 11 +- .../handler/component/HttpShardHandler.java | 2 +- .../component/HttpShardHandlerFactory.java | 13 +- .../solrj/impl/CloudHttp2SolrClient.java | 44 +--- .../client/solrj/impl/LBHttp2SolrClient.java | 78 ++++-- .../solr/client/solrj/impl/LBSolrClient.java | 13 +- .../impl/CloudHttp2SolrClientBuilderTest.java | 34 +-- .../LBHttp2SolrClientIntegrationTest.java | 27 +- .../solrj/impl/LBHttp2SolrClientTest.java | 232 ++++++++++-------- 9 files changed, 220 insertions(+), 234 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index 76f9618d4b1..3dc5453ca2e 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -198,9 +198,6 @@ public String toString() { public final ZkStateReader zkStateReader; private SolrCloudManager cloudManager; - // only for internal usage - private Http2SolrClient http2SolrClient; - private CloudHttp2SolrClient cloudSolrClient; private final String zkServerAddress; // example: 127.0.0.1:54062/solr @@ -776,7 +773,6 @@ public void close() { sysPropsCacher.close(); customThreadPool.execute(() -> IOUtils.closeQuietly(cloudManager)); customThreadPool.execute(() -> IOUtils.closeQuietly(cloudSolrClient)); - customThreadPool.execute(() -> IOUtils.closeQuietly(http2SolrClient)); try { try { @@ -872,15 +868,14 @@ public SolrCloudManager getSolrCloudManager() { if (cloudManager != null) { return cloudManager; } - http2SolrClient = + var httpClientBuilder = new Http2SolrClient.Builder() .withHttpClient(cc.getDefaultHttpSolrClient()) .withIdleTimeout(30000, TimeUnit.MILLISECONDS) - .withConnectionTimeout(15000, TimeUnit.MILLISECONDS) - .build(); + .withConnectionTimeout(15000, TimeUnit.MILLISECONDS); cloudSolrClient = new CloudHttp2SolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader)) - .withHttpClient(http2SolrClient) + .withInternalClientBuilder(httpClientBuilder) .build(); cloudManager = new SolrClientCloudManager(cloudSolrClient, cc.getObjectCache()); cloudManager.getClusterStateProvider().connect(); diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java index 7592eed86fc..320a5fe70d7 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java @@ -114,7 +114,7 @@ public class HttpShardHandler extends ShardHandler { protected AtomicInteger pending; private final Map> shardToURLs; - protected LBHttp2SolrClient lbClient; + protected LBHttp2SolrClient lbClient; public HttpShardHandler(HttpShardHandlerFactory httpShardHandlerFactory) { this.httpShardHandlerFactory = httpShardHandlerFactory; diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java index ac7dc0cf8e0..655cb65d77c 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java +++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java @@ -84,7 +84,7 @@ public class HttpShardHandlerFactory extends ShardHandlerFactory protected volatile Http2SolrClient defaultClient; protected InstrumentedHttpListenerFactory httpListenerFactory; - protected LBHttp2SolrClient loadbalancer; + protected LBHttp2SolrClient loadbalancer; int corePoolSize = 0; int maximumPoolSize = Integer.MAX_VALUE; @@ -305,16 +305,15 @@ public void init(PluginInfo info) { sb); int soTimeout = getParameter(args, HttpClientUtil.PROP_SO_TIMEOUT, HttpClientUtil.DEFAULT_SO_TIMEOUT, sb); - - this.defaultClient = - new Http2SolrClient.Builder() + var defaultClientBuilder = new Http2SolrClient.Builder() .withConnectionTimeout(connectionTimeout, TimeUnit.MILLISECONDS) .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS) .withExecutor(commExecutor) - .withMaxConnectionsPerHost(maxConnectionsPerHost) - .build(); + .withMaxConnectionsPerHost(maxConnectionsPerHost); + this.defaultClient = defaultClientBuilder.build(); + this.defaultClient.addListenerFactory(this.httpListenerFactory); - this.loadbalancer = new LBHttp2SolrClient.Builder(defaultClient).build(); + this.loadbalancer = new LBHttp2SolrClient.Builder(defaultClientBuilder).build(); initReplicaListTransformers(getParameter(args, "replicaRouting", null, sb)); 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 ade1ebe433f..7d2e2517b00 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 @@ -40,9 +40,8 @@ public class CloudHttp2SolrClient extends CloudSolrClient { private final ClusterStateProvider stateProvider; - private final LBHttp2SolrClient lbClient; + private final LBHttp2SolrClient lbClient; private final Http2SolrClient myClient; - private final boolean clientIsInternal; /** * Create a new client object that connects to Zookeeper and is always aware of the SolrCloud @@ -54,8 +53,8 @@ public class CloudHttp2SolrClient extends CloudSolrClient { */ protected CloudHttp2SolrClient(Builder builder) { super(builder.shardLeadersOnly, builder.parallelUpdates, builder.directUpdatesToLeadersOnly); - this.clientIsInternal = builder.httpClient == null; - this.myClient = createOrGetHttpClientFromBuilder(builder); + var httpClientBuilder = createOrGetHttpClientBuilder(builder); + this.myClient = httpClientBuilder.build(); this.stateProvider = createClusterStateProvider(builder); this.retryExpiryTimeNano = builder.retryExpiryTimeNano; this.defaultCollection = builder.defaultCollection; @@ -73,16 +72,14 @@ protected CloudHttp2SolrClient(Builder builder) { // locks. this.locks = objectList(builder.parallelCacheRefreshesLocks); - this.lbClient = new LBHttp2SolrClient.Builder(myClient).build(); + this.lbClient = new LBHttp2SolrClient.Builder<>(httpClientBuilder).build(); } - private Http2SolrClient createOrGetHttpClientFromBuilder(Builder builder) { - if (builder.httpClient != null) { - return builder.httpClient; - } else if (builder.internalClientBuilder != null) { - return builder.internalClientBuilder.build(); + private Http2SolrClient.Builder createOrGetHttpClientBuilder(Builder builder) { + if (builder.internalClientBuilder != null) { + return builder.internalClientBuilder; } else { - return new Http2SolrClient.Builder().build(); + return new Http2SolrClient.Builder(); } } @@ -129,7 +126,7 @@ private ClusterStateProvider createHttp2ClusterStateProvider( private void closeMyClientIfNeeded() { try { - if (clientIsInternal && myClient != null) { + if (myClient != null) { myClient.close(); } } catch (Exception e) { @@ -148,7 +145,7 @@ public void close() throws IOException { } @Override - public LBHttp2SolrClient getLbClient() { + public LBHttp2SolrClient getLbClient() { return lbClient; } @@ -171,7 +168,6 @@ public static class Builder { protected Collection zkHosts = new ArrayList<>(); protected List solrUrls = new ArrayList<>(); protected String zkChroot; - protected Http2SolrClient httpClient; protected boolean shardLeadersOnly = true; protected boolean directUpdatesToLeadersOnly = false; protected boolean parallelUpdates = true; @@ -404,22 +400,6 @@ public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) { return this; } - /** - * Set the internal http client. - * - *

Note: closing the httpClient instance is at the responsibility of the caller. - * - * @param httpClient http client - * @return this - */ - public Builder withHttpClient(Http2SolrClient httpClient) { - if (this.internalClientBuilder != null) { - throw new IllegalStateException( - "The builder can't accept an httpClient AND an internalClientBuilder, only one of those can be provided"); - } - this.httpClient = httpClient; - return this; - } /** * If provided, the CloudHttp2SolrClient will build it's internal Http2SolrClient using this @@ -430,10 +410,6 @@ public Builder withHttpClient(Http2SolrClient httpClient) { * @return this */ public Builder withInternalClientBuilder(Http2SolrClient.Builder 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"); - } this.internalClientBuilder = internalClientBuilder; return this; } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java index 2c926a26261..78bb72ddddb 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java @@ -16,25 +16,32 @@ */ package org.apache.solr.client.solrj.impl; -import static org.apache.solr.common.params.CommonParams.ADMIN_PATHS; +import org.apache.solr.client.solrj.ResponseParser; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.request.IsUpdateRequest; +import org.apache.solr.client.solrj.request.RequestWriter; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.util.IOUtils; +import org.apache.solr.common.util.NamedList; +import org.slf4j.MDC; import java.io.IOException; import java.net.ConnectException; import java.net.SocketException; import java.net.SocketTimeoutException; import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; -import org.apache.solr.client.solrj.ResponseParser; -import org.apache.solr.client.solrj.SolrClient; -import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.request.IsUpdateRequest; -import org.apache.solr.client.solrj.request.RequestWriter; -import org.apache.solr.common.SolrException; -import org.apache.solr.common.util.NamedList; -import org.slf4j.MDC; +import java.util.stream.Collectors; + +import static org.apache.solr.common.params.CommonParams.ADMIN_PATHS; /** * This "LoadBalanced Http Solr Client" is a load balancing wrapper around a Http Solr Client. This @@ -94,35 +101,55 @@ * * @since solr 8.0 */ -public class LBHttp2SolrClient extends LBSolrClient { +public class LBHttp2SolrClient> extends LBSolrClient { - protected final C solrClient; + private final Map urlToClient; + private final Set urlParamNames; @SuppressWarnings("unchecked") private LBHttp2SolrClient(Builder builder) { super(Arrays.asList(builder.solrEndpoints)); - this.solrClient = (C) builder.solrClient; + + String tmpBaseSolrUrl = builder.solrClientBuilder.baseSolrUrl; + Map buildUrlToClient = new HashMap<>(); + for(LBSolrClient.Endpoint endpoint : builder.solrEndpoints) { + builder.solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl(); + buildUrlToClient.put(endpoint.toString(), builder.solrClientBuilder.build()); + } + builder.solrClientBuilder.baseSolrUrl = tmpBaseSolrUrl; + this.urlToClient = Collections.unmodifiableMap(buildUrlToClient); + this.aliveCheckIntervalMillis = builder.aliveCheckIntervalMillis; this.defaultCollection = builder.defaultCollection; + this.urlParamNames = Collections.unmodifiableSet(builder.solrClientBuilder.urlParamNames); + } @Override - protected SolrClient getClient(Endpoint endpoint) { - return solrClient; + protected HttpSolrClientBase getClient(Endpoint endpoint) { + return urlToClient.get(endpoint.toString()); } @Override public ResponseParser getParser() { - return solrClient.getParser(); + return urlToClient.isEmpty() ? null : urlToClient.values().iterator().next().getParser(); } @Override public RequestWriter getRequestWriter() { - return solrClient.getRequestWriter(); + return urlToClient.isEmpty() ? null : urlToClient.values().iterator().next().getRequestWriter(); } public Set getUrlParamNames() { - return solrClient.getUrlParamNames(); + return urlParamNames; + } + + @Override + public void close() { + super.close(); + for(HttpSolrClientBase client : urlToClient.values()) { + IOUtils.closeQuietly(client); + } } /** @@ -290,16 +317,17 @@ private void onFailedRequest( } } - public static class Builder { + public static class Builder> { + + private final B solrClientBuilder; - private final C solrClient; private final LBSolrClient.Endpoint[] solrEndpoints; private long aliveCheckIntervalMillis = TimeUnit.MILLISECONDS.convert(60, TimeUnit.SECONDS); // 1 minute between checks protected String defaultCollection; - public Builder(C solrClient, Endpoint... endpoints) { - this.solrClient = solrClient; + public Builder(B solrClientBuilder, Endpoint... endpoints) { + this.solrClientBuilder = solrClientBuilder; this.solrEndpoints = endpoints; } @@ -309,7 +337,7 @@ public Builder(C solrClient, Endpoint... endpoints) { * * @param aliveCheckInterval how often to ping for aliveness */ - public Builder setAliveCheckInterval(int aliveCheckInterval, TimeUnit unit) { + public Builder setAliveCheckInterval(int aliveCheckInterval, TimeUnit unit) { if (aliveCheckInterval <= 0) { throw new IllegalArgumentException( "Alive check interval must be " + "positive, specified value = " + aliveCheckInterval); @@ -319,13 +347,13 @@ public Builder setAliveCheckInterval(int aliveCheckInterval, TimeUnit unit) { } /** Sets a default for core or collection based requests. */ - public Builder withDefaultCollection(String defaultCoreOrCollection) { + public Builder withDefaultCollection(String defaultCoreOrCollection) { this.defaultCollection = defaultCoreOrCollection; return this; } - public LBHttp2SolrClient build() { - return new LBHttp2SolrClient(this); + public LBHttp2SolrClient build() { + return new LBHttp2SolrClient(this); } } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java index 64201b03c13..b1476585c5d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java @@ -500,21 +500,10 @@ private NamedList doRequest(Endpoint endpoint, SolrRequest solrReques return doRequest(solrClient, endpoint.getBaseUrl(), endpoint.getCore(), solrRequest); } - // TODO SOLR-17541 should remove the need for the special-casing below; remove as a part of that - // ticket. + //TODO remove this method NOCOMMIT private NamedList doRequest( SolrClient solrClient, String baseUrl, String collection, SolrRequest solrRequest) throws SolrServerException, IOException { - // Some implementations of LBSolrClient.getClient(...) return a Http2SolrClient that may not be - // pointed at the desired URL (or any URL for that matter). We special case that here to ensure - // the appropriate URL is provided. - if (solrClient instanceof Http2SolrClient httpSolrClient) { - return httpSolrClient.requestWithBaseUrl(baseUrl, (c) -> c.request(solrRequest, collection)); - } else if (solrClient instanceof HttpJdkSolrClient) { - return ((HttpJdkSolrClient) solrClient).requestWithBaseUrl(baseUrl, solrRequest, collection); - } - - // Assume provided client already uses 'baseUrl' return solrClient.request(solrRequest, collection); } 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 01f57970e99..45ebc7a0312 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 @@ -128,7 +128,6 @@ public void testExternalClientAndInternalBuilderTogether() { () -> new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) - .withHttpClient(mock(Http2SolrClient.class)) .withInternalClientBuilder(mock(Http2SolrClient.Builder.class)) .build()); expectThrows( @@ -137,7 +136,6 @@ public void testExternalClientAndInternalBuilderTogether() { new CloudHttp2SolrClient.Builder( Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT)) .withInternalClientBuilder(mock(Http2SolrClient.Builder.class)) - .withHttpClient(mock(Http2SolrClient.class)) .build()); } @@ -160,20 +158,6 @@ public void testProvideInternalBuilder() throws IOException { verify(http2Client, times(1)).close(); } - @Test - public void testProvideExternalClient() throws IOException { - Http2SolrClient http2Client = mock(Http2SolrClient.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 testDefaultCollectionPassedFromBuilderToClient() throws IOException { try (CloudHttp2SolrClient createdClient = @@ -196,29 +180,21 @@ public void testDefaultCollectionPassedFromBuilderToClient() throws IOException public void testHttpClientPreservedInHttp2ClusterStateProvider() throws IOException { List solrUrls = List.of(cluster.getJettySolrRunner(0).getBaseUrl().toString()); - // No httpClient - No internalClientBuilder - testHttpClientConsistency(solrUrls, null, null); - - // httpClient - No internalClientBuilder - try (Http2SolrClient httpClient = new Http2SolrClient.Builder().build()) { - testHttpClientConsistency(solrUrls, httpClient, null); - } + // without internalClientBuilder + testHttpClientConsistency(solrUrls, null); - // No httpClient - internalClientBuilder + // with internalClientBuilder Http2SolrClient.Builder internalClientBuilder = new Http2SolrClient.Builder(); - testHttpClientConsistency(solrUrls, null, internalClientBuilder); + testHttpClientConsistency(solrUrls, internalClientBuilder); } private void testHttpClientConsistency( List solrUrls, - Http2SolrClient httpClient, Http2SolrClient.Builder internalClientBuilder) throws IOException { CloudHttp2SolrClient.Builder clientBuilder = new CloudHttp2SolrClient.Builder(solrUrls); - if (httpClient != null) { - clientBuilder.withHttpClient(httpClient); - } else if (internalClientBuilder != null) { + if (internalClientBuilder != null) { clientBuilder.withInternalClientBuilder(internalClientBuilder); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java index 7653731b511..69746e03f5c 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java @@ -118,30 +118,28 @@ public void tearDown() throws Exception { private LBClientHolder client(LBSolrClient.Endpoint... baseSolrEndpoints) { if (random().nextBoolean()) { - var delegateClient = + var delegateClientBuilder = new Http2SolrClient.Builder() .withConnectionTimeout(1000, TimeUnit.MILLISECONDS) - .withIdleTimeout(2000, TimeUnit.MILLISECONDS) - .build(); + .withIdleTimeout(2000, TimeUnit.MILLISECONDS); var lbClient = - new LBHttp2SolrClient.Builder<>(delegateClient, baseSolrEndpoints) + new LBHttp2SolrClient.Builder<>(delegateClientBuilder, baseSolrEndpoints) .withDefaultCollection(solr[0].getDefaultCollection()) .setAliveCheckInterval(500, TimeUnit.MILLISECONDS) .build(); - return new LBClientHolder(lbClient, delegateClient); + return new LBClientHolder(lbClient, delegateClientBuilder); } else { - var delegateClient = + var delegateClientBuilder = new HttpJdkSolrClient.Builder() .withConnectionTimeout(1000, TimeUnit.MILLISECONDS) .withIdleTimeout(2000, TimeUnit.MILLISECONDS) - .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT) - .build(); + .withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT); var lbClient = - new LBHttp2SolrClient.Builder<>(delegateClient, baseSolrEndpoints) + new LBHttp2SolrClient.Builder<>(delegateClientBuilder, baseSolrEndpoints) .withDefaultCollection(solr[0].getDefaultCollection()) .setAliveCheckInterval(500, TimeUnit.MILLISECONDS) .build(); - return new LBClientHolder(lbClient, delegateClient); + return new LBClientHolder(lbClient, delegateClientBuilder); } } @@ -318,9 +316,9 @@ public void startJetty() throws Exception { private static class LBClientHolder implements AutoCloseable { final LBHttp2SolrClient lbClient; - final HttpSolrClientBase delegate; + final HttpSolrClientBuilderBase delegate; - LBClientHolder(LBHttp2SolrClient lbClient, HttpSolrClientBase delegate) { + LBClientHolder(LBHttp2SolrClient lbClient, HttpSolrClientBuilderBase delegate) { this.lbClient = lbClient; this.delegate = delegate; } @@ -328,11 +326,6 @@ private static class LBClientHolder implements AutoCloseable { @Override public void close() { lbClient.close(); - try { - delegate.close(); - } catch (IOException ioe) { - throw new UncheckedIOException(ioe); - } } } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java index 9d2019309b0..3ee01106d29 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java @@ -18,6 +18,8 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -51,11 +53,9 @@ public void testLBHttp2SolrClientWithTheseParamNamesInTheUrl() { Set urlParamNames = new HashSet<>(2); urlParamNames.add("param1"); - try (Http2SolrClient http2SolrClient = - new Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames).build(); - LBHttp2SolrClient testClient = - new LBHttp2SolrClient.Builder<>(http2SolrClient, new LBSolrClient.Endpoint(url)) - .build()) { + var httpClientBuilder = new Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames); + var endpoint = new LBSolrClient.Endpoint(url); + try (var testClient = new LBHttp2SolrClient.Builder(httpClientBuilder, endpoint).build()) { assertArrayEquals( "Wrong urlParamNames found in lb client.", @@ -64,7 +64,7 @@ public void testLBHttp2SolrClientWithTheseParamNamesInTheUrl() { assertArrayEquals( "Wrong urlParamNames found in base client.", urlParamNames.toArray(), - http2SolrClient.getUrlParamNames().toArray()); + testClient.getClient(endpoint).getUrlParamNames().toArray()); } } @@ -74,12 +74,10 @@ public void testSynchronous() throws Exception { LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two"); List endpointList = List.of(ep1, ep2); - Http2SolrClient.Builder b = - new Http2SolrClient.Builder("http://base.url").withConnectionTimeout(10, TimeUnit.SECONDS); - ; - try (MockHttpSolrClient client = new MockHttpSolrClient("http://base.url", b); - LBHttp2SolrClient testClient = - new LBHttp2SolrClient.Builder<>(client, ep1, ep2).build()) { + var httpClientBuilder = new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS); + + try (LBHttp2SolrClient testClient = + new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) { String lastEndpoint = null; for (int i = 0; i < 10; i++) { @@ -103,15 +101,13 @@ public void testSynchronousWithFalures() throws Exception { LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two"); List endpointList = List.of(ep1, ep2); - Http2SolrClient.Builder b = - new Http2SolrClient.Builder("http://base.url").withConnectionTimeout(10, TimeUnit.SECONDS); - ; - try (MockHttpSolrClient client = new MockHttpSolrClient("http://base.url", b); - LBHttp2SolrClient testClient = - new LBHttp2SolrClient.Builder<>(client, ep1, ep2).build()) { + var httpClientBuilder = new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS); + + try (LBHttp2SolrClient testClient = + new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) { - client.basePathToFail = ep1.getBaseUrl(); - String basePathToSucceed = ep2.getBaseUrl(); + setEndpointToFail(testClient, ep1); + setEndpointToSucceed(testClient, ep2); String qValue = "First time"; for (int i = 0; i < 5; i++) { @@ -121,13 +117,13 @@ public void testSynchronousWithFalures() throws Exception { LBSolrClient.Rsp response = testClient.request(req); assertEquals( "The healthy node 'endpoint two' should have served the request: " + i, - basePathToSucceed, + ep2.getBaseUrl(), response.server); checkSynchonousResponseContent(response, qValue); } - client.basePathToFail = ep2.getBaseUrl(); - basePathToSucceed = ep1.getBaseUrl(); + setEndpointToFail(testClient, ep2); + setEndpointToSucceed(testClient, ep1); qValue = "Second time"; for (int i = 0; i < 5; i++) { @@ -137,21 +133,13 @@ public void testSynchronousWithFalures() throws Exception { LBSolrClient.Rsp response = testClient.request(req); assertEquals( "The healthy node 'endpoint one' should have served the request: " + i, - basePathToSucceed, + ep1.getBaseUrl(), response.server); checkSynchonousResponseContent(response, qValue); } } } - private void checkSynchonousResponseContent(LBSolrClient.Rsp response, String qValue) { - assertEquals("There should be one element in the respnse.", 1, response.getResponse().size()); - assertEquals( - "The response key 'response' should echo the query.", - qValue, - response.getResponse().get("response")); - } - @Test public void testAsyncWithFailures() { @@ -162,28 +150,33 @@ public void testAsyncWithFailures() { LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two"); List endpointList = List.of(ep1, ep2); - Http2SolrClient.Builder b = - new Http2SolrClient.Builder("http://base.url").withConnectionTimeout(10, TimeUnit.SECONDS); - ; - try (MockHttpSolrClient client = new MockHttpSolrClient("http://base.url", b); - LBHttp2SolrClient testClient = - new LBHttp2SolrClient.Builder<>(client, ep1, ep2).build()) { + var httpClientBuilder = new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS); + + try (LBHttp2SolrClient testClient = + new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) { for (int j = 0; j < 2; j++) { // first time Endpoint One will return error code 500. // second time Endpoint One will be healthy - String basePathToSucceed; + LBSolrClient.Endpoint endpointToSucceed; + LBSolrClient.Endpoint endpointToFail; if (j == 0) { - client.basePathToFail = ep1.getBaseUrl(); - basePathToSucceed = ep2.getBaseUrl(); + setEndpointToFail(testClient, ep1); + setEndpointToSucceed(testClient, ep2); + endpointToSucceed = ep2; + endpointToFail = ep1; } else { - client.basePathToFail = ep2.getBaseUrl(); - basePathToSucceed = ep1.getBaseUrl(); + setEndpointToFail(testClient, ep2); + setEndpointToSucceed(testClient, ep1); + endpointToSucceed = ep1; + endpointToFail = ep2; } + List successEndpointLastBasePaths = basePathsForEndpoint(testClient, endpointToSucceed); + List failEndpointLastBasePaths = basePathsForEndpoint(testClient, endpointToFail); for (int i = 0; i < 10; i++) { - // i: we'll try 10 times to see if it behaves the same every time. + // i: we'll try 10 times. It should behave the same with iter 2-10. . QueryRequest queryRequest = new QueryRequest(new MapSolrParams(Map.of("q", "" + i))); LBSolrClient.Req req = new LBSolrClient.Req(queryRequest, endpointList); @@ -196,42 +189,40 @@ public void testAsyncWithFailures() { } catch (TimeoutException | ExecutionException e) { fail(iterMessage + " Response ended in failure: " + e); } + if (i == 0) { - // When j=0, "endpoint one" fails. - // The first time around (i) it tries the first, then the second. - // - // With j=0 and i>0, it only tries "endpoint two". - // - // When j=1 and i=0, "endpoint two" starts failing. - // So it tries both it and "endpoint one" + // When i=0, it must try both endpoints to find success: // - // With j=1 and i>0, it only tries "endpoint one". - assertEquals(iterMessage, 2, client.lastBasePaths.size()); - - String failedBasePath = client.lastBasePaths.remove(0); - assertEquals(iterMessage, client.basePathToFail, failedBasePath); + // with j=0, endpoint one is tried first because it + // is first one the list, but it fails. + // with j=1, endpoint two is tried first because + // it is the only known healthy node, but + // now it is failing. + assertEquals(iterMessage, 1, successEndpointLastBasePaths.size()); + assertEquals(iterMessage, 1, failEndpointLastBasePaths.size()); } else { - // The first endpoint does not give the exception, it doesn't retry. - assertEquals(iterMessage, 1, client.lastBasePaths.size()); + // With i>0, + // With j=0 and i>0, it only tries "endpoint two". + // With j=1 and i>0, it only tries "endpoint one". + assertEquals(iterMessage, 1, successEndpointLastBasePaths); + assertTrue(iterMessage, failEndpointLastBasePaths.isEmpty()); } - String successBasePath = client.lastBasePaths.remove(0); - assertEquals(iterMessage, basePathToSucceed, successBasePath); + successEndpointLastBasePaths.clear(); + failEndpointLastBasePaths.clear(); } } } } - @Test public void testAsync() { LBSolrClient.Endpoint ep1 = new LBSolrClient.Endpoint("http://endpoint.one"); LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two"); List endpointList = List.of(ep1, ep2); - Http2SolrClient.Builder b = - new Http2SolrClient.Builder("http://base.url").withConnectionTimeout(10, TimeUnit.SECONDS); - try (MockHttpSolrClient client = new MockHttpSolrClient("http://base.url", b); - LBHttp2SolrClient testClient = - new LBHttp2SolrClient.Builder<>(client, ep1, ep2).build()) { + var httpClientBuilder = new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS); + + try (LBHttp2SolrClient testClient = + new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) { int limit = 10; // For simplicity use an even limit List> responses = new ArrayList<>(); @@ -243,23 +234,17 @@ public void testAsync() { } QueryRequest[] queryRequests = new QueryRequest[limit]; - int numEndpointOne = 0; - int numEndpointTwo = 0; + List> lastSolrRequests = lastSolrRequests(testClient, ep1, ep2); + assertEquals(limit, lastSolrRequests.size()); + for (int i = 0; i < limit; i++) { - SolrRequest lastSolrReq = client.lastSolrRequests.get(i); + SolrRequest lastSolrReq = lastSolrRequests.get(i); assertTrue(lastSolrReq instanceof QueryRequest); QueryRequest lastQueryReq = (QueryRequest) lastSolrReq; int index = Integer.parseInt(lastQueryReq.getParams().get("q")); assertNull("Found same request twice: " + index, queryRequests[index]); queryRequests[index] = lastQueryReq; - final String lastBasePath = client.lastBasePaths.get(i); - if (lastBasePath.equals(ep1.toString())) { - numEndpointOne++; - } else if (lastBasePath.equals(ep2.toString())) { - numEndpointTwo++; - } - LBSolrClient.Rsp lastRsp = null; try { lastRsp = responses.get(index).get(); @@ -278,15 +263,50 @@ public void testAsync() { // It is the user's responsibility to shuffle the endpoints when using // async. LB Http Solr Client will always try the passed-in endpoints // in order. In this case, endpoint 1 gets all the requests! - assertEquals(limit, numEndpointOne); - assertEquals(0, numEndpointTwo); + List ep1BasePaths = basePathsForEndpoint(testClient, ep1); + List ep2BasePaths = basePathsForEndpoint(testClient, ep2); + assertEquals(limit, basePathsForEndpoint(testClient, ep1).size()); + assertEquals(0, basePathsForEndpoint(testClient, ep2).size()); + } + } + + private void checkSynchonousResponseContent(LBSolrClient.Rsp response, String qValue) { + assertEquals("There should be one element in the response.", 1, response.getResponse().size()); + assertEquals( + "The response key 'response' should echo the query.", + qValue, + response.getResponse().get("response")); + } + + private void setEndpointToFail(LBHttp2SolrClient testClient, LBSolrClient.Endpoint ep) { + ((MockHttpSolrClient) testClient.getClient(ep)).allRequestsShallFail = true; + } + + private void setEndpointToSucceed(LBHttp2SolrClient testClient, LBSolrClient.Endpoint ep) { + ((MockHttpSolrClient) testClient.getClient(ep)).allRequestsShallFail = false; + } - assertEquals(limit, client.lastSolrRequests.size()); - assertEquals(limit, client.lastCollections.size()); + private List basePathsForEndpoint(LBHttp2SolrClient testClient, LBSolrClient.Endpoint ep) { + return ((MockHttpSolrClient) testClient.getClient(ep)).lastBasePaths; + } + + private List> lastSolrRequests(LBHttp2SolrClient testClient, LBSolrClient.Endpoint... endpoints) { + return Arrays.stream(endpoints) + .map(testClient::getClient) + .map(MockHttpSolrClient.class::cast) + .flatMap(c -> c.lastSolrRequests.stream()) + .toList(); + } + + public static class MockHttpSolrClientBuilder extends HttpSolrClientBuilderBase { + + @Override + public MockHttpSolrClient build() { + return new MockHttpSolrClient(baseSolrUrl, this); } } - public static class MockHttpSolrClient extends Http2SolrClient { + public static class MockHttpSolrClient extends HttpSolrClientBase { public List> lastSolrRequests = new ArrayList<>(); @@ -294,15 +314,13 @@ public static class MockHttpSolrClient extends Http2SolrClient { public List lastCollections = new ArrayList<>(); - public String basePathToFail = null; + public boolean allRequestsShallFail; public String tmpBaseUrl = null; - protected MockHttpSolrClient(String serverBaseUrl, Builder builder) { + public boolean closeCalled; - // TODO: Consider creating an interface for Http*SolrClient - // so mocks can Implement, not Extend, and not actually need to - // build an (unused) client + protected MockHttpSolrClient(String serverBaseUrl, MockHttpSolrClientBuilder builder) { super(serverBaseUrl, builder); } @@ -312,25 +330,12 @@ public NamedList request(final SolrRequest request, String collection lastSolrRequests.add(request); lastBasePaths.add(tmpBaseUrl); lastCollections.add(collection); - if (tmpBaseUrl.equals(basePathToFail)) { + if (allRequestsShallFail) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "We should retry this."); } return generateResponse(request); } - @Override - public R requestWithBaseUrl( - String baseUrl, SolrClientFunction clientFunction) - throws SolrServerException, IOException { - // This use of 'tmpBaseUrl' is NOT thread safe, but that's fine for our purposes here. - try { - tmpBaseUrl = baseUrl; - return clientFunction.apply(this); - } finally { - tmpBaseUrl = null; - } - } - @Override public CompletableFuture> requestAsync( final SolrRequest solrRequest, String collection) { @@ -338,7 +343,7 @@ public CompletableFuture> requestAsync( lastSolrRequests.add(solrRequest); lastBasePaths.add(tmpBaseUrl); lastCollections.add(collection); - if (tmpBaseUrl != null && tmpBaseUrl.equals(basePathToFail)) { + if (allRequestsShallFail) { cf.completeExceptionally( new SolrException(SolrException.ErrorCode.SERVER_ERROR, "We should retry this.")); } else { @@ -351,5 +356,30 @@ private NamedList generateResponse(SolrRequest solrRequest) { String id = solrRequest.getParams().get("q"); return new NamedList<>(Collections.singletonMap("response", id)); } + + @Override + public void close() throws IOException { + closeCalled = true; + } + + @Override + protected boolean isFollowRedirects() { + return false; + } + + @Override + protected boolean processorAcceptsMimeType(Collection processorSupportedContentTypes, String mimeType) { + return false; + } + + @Override + protected String allProcessorSupportedContentTypesCommaDelimited(Collection processorSupportedContentTypes) { + return null; + } + + @Override + protected void updateDefaultMimeTypeForParser() { + //no-op + } } } From bc8c8887d343b723dbb2294799a235d96023fd5f Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Thu, 5 Dec 2024 15:24:46 -0600 Subject: [PATCH 02/23] fix bugs --- .../client/solrj/impl/LBHttp2SolrClient.java | 34 +++++++++---------- .../solrj/impl/LBHttp2SolrClientTest.java | 2 +- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java index 78bb72ddddb..e0e0306bd64 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java @@ -121,8 +121,12 @@ private LBHttp2SolrClient(Builder builder) { this.aliveCheckIntervalMillis = builder.aliveCheckIntervalMillis; this.defaultCollection = builder.defaultCollection; - this.urlParamNames = Collections.unmodifiableSet(builder.solrClientBuilder.urlParamNames); + if(builder.solrClientBuilder.urlParamNames == null) { + this.urlParamNames = Collections.emptySet(); + } else { + this.urlParamNames = Collections.unmodifiableSet(builder.solrClientBuilder.urlParamNames); + } } @Override @@ -237,23 +241,17 @@ private CompletableFuture> doAsyncRequest( RetryListener listener) { String baseUrl = endpoint.toString(); rsp.server = baseUrl; - final var client = (Http2SolrClient) getClient(endpoint); - try { - CompletableFuture> future = - client.requestWithBaseUrl(baseUrl, (c) -> c.requestAsync(req.getRequest())); - future.whenComplete( - (result, throwable) -> { - if (!future.isCompletedExceptionally()) { - onSuccessfulRequest(result, endpoint, rsp, isZombie, listener); - } else if (!future.isCancelled()) { - onFailedRequest(throwable, endpoint, isNonRetryable, isZombie, listener); - } - }); - return future; - } catch (SolrServerException | IOException e) { - // Unreachable, since 'requestWithBaseUrl' above is running the request asynchronously - throw new RuntimeException(e); - } + final var client = getClient(endpoint); + CompletableFuture> future = client.requestAsync(req.getRequest()); + future.whenComplete( + (result, throwable) -> { + if (!future.isCompletedExceptionally()) { + onSuccessfulRequest(result, endpoint, rsp, isZombie, listener); + } else if (!future.isCancelled()) { + onFailedRequest(throwable, endpoint, isNonRetryable, isZombie, listener); + } + }); + return future; } private void onSuccessfulRequest( diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java index 3ee01106d29..5d7c911dfad 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java @@ -204,7 +204,7 @@ public void testAsyncWithFailures() { // With i>0, // With j=0 and i>0, it only tries "endpoint two". // With j=1 and i>0, it only tries "endpoint one". - assertEquals(iterMessage, 1, successEndpointLastBasePaths); + assertEquals(iterMessage, 1, successEndpointLastBasePaths.size()); assertTrue(iterMessage, failEndpointLastBasePaths.isEmpty()); } successEndpointLastBasePaths.clear(); From a2491695ec8c4f1439e49dab513aa8441163e7ca Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Thu, 5 Dec 2024 15:51:34 -0600 Subject: [PATCH 03/23] javadoc modifications --- .../client/solrj/impl/LBHttp2SolrClient.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java index e0e0306bd64..f6a0f2a1880 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java @@ -44,7 +44,7 @@ import static org.apache.solr.common.params.CommonParams.ADMIN_PATHS; /** - * This "LoadBalanced Http Solr Client" is a load balancing wrapper around a Http Solr Client. This + * This "LoadBalanced Http Solr Client" is a load balancer for multiple Http Solr Clients. This * is useful when you have multiple Solr endpoints and requests need to be Load Balanced among them. * *

Do NOT use this class for indexing in leader/follower scenarios since documents must be @@ -63,7 +63,7 @@ *

* * 
- * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClient,
+ * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClientBuilder,
  *         new LBSolrClient.Endpoint("http://host1:8080/solr"), new LBSolrClient.Endpoint("http://host2:8080/solr"))
  *     .build();
  *
@@ -76,7 +76,7 @@ *
* * 
- * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClient,
+ * SolrClient client = new LBHttp2SolrClient.Builder(http2SolrClientBuilder,
  *         new LBSolrClient.Endpoint("http://host1:8080/solr", "coreA"),
  *         new LBSolrClient.Endpoint("http://host2:8080/solr", "coreB"))
  *     .build();
@@ -324,6 +324,16 @@ public static class Builder> {
         TimeUnit.MILLISECONDS.convert(60, TimeUnit.SECONDS); // 1 minute between checks
     protected String defaultCollection;
 
+    /**
+     * 
Use this Builder to configure an LBHttp2SolrClient.  The passed-in Solr Client Builder will be used to
+     * generate an internal client per specified Endpoint.
+     *
+     * 
Implementation Note: During construction LBHttp2SolrClient will temporarily modify
+     * the Builder's {@link HttpSolrClientBuilderBase#baseSolrUrl}.
+     *
+     * @param solrClientBuilder A Builder like {@link Http2SolrClient.Builder} used to generate the internal clients
+     * @param endpoints the various Solr Endpoints to load balance
+     */
     public Builder(B solrClientBuilder, Endpoint... endpoints) {
       this.solrClientBuilder = solrClientBuilder;
       this.solrEndpoints = endpoints;

From 1f8a30736067e95d77eee5fd23ae8fb785d0da33 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Thu, 5 Dec 2024 16:00:47 -0600
Subject: [PATCH 04/23] tidy / remove nocommit

---
 .../component/HttpShardHandlerFactory.java    |  6 +-
 .../solrj/impl/CloudHttp2SolrClient.java      |  1 -
 .../client/solrj/impl/LBHttp2SolrClient.java  | 47 ++++++-------
 .../solr/client/solrj/impl/LBSolrClient.java  | 21 ++----
 .../impl/CloudHttp2SolrClientBuilderTest.java |  4 +-
 .../LBHttp2SolrClientIntegrationTest.java     |  5 +-
 .../solrj/impl/LBHttp2SolrClientTest.java     | 69 +++++++++++--------
 7 files changed, 75 insertions(+), 78 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
index 655cb65d77c..6978717ccdf 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
@@ -305,7 +305,8 @@ public void init(PluginInfo info) {
             sb);
     int soTimeout =
         getParameter(args, HttpClientUtil.PROP_SO_TIMEOUT, HttpClientUtil.DEFAULT_SO_TIMEOUT, sb);
-    var defaultClientBuilder = new Http2SolrClient.Builder()
+    var defaultClientBuilder =
+        new Http2SolrClient.Builder()
             .withConnectionTimeout(connectionTimeout, TimeUnit.MILLISECONDS)
             .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS)
             .withExecutor(commExecutor)
@@ -313,7 +314,8 @@ public void init(PluginInfo info) {
     this.defaultClient = defaultClientBuilder.build();
 
     this.defaultClient.addListenerFactory(this.httpListenerFactory);
-    this.loadbalancer = new LBHttp2SolrClient.Builder(defaultClientBuilder).build();
+    this.loadbalancer =
+        new LBHttp2SolrClient.Builder(defaultClientBuilder).build();
 
     initReplicaListTransformers(getParameter(args, "replicaRouting", null, sb));
 
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 7d2e2517b00..ef80a292b9a 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
@@ -400,7 +400,6 @@ public Builder withCollectionCacheTtl(long timeToLive, TimeUnit unit) {
       return this;
     }
 
-
     /**
      * 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
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index f6a0f2a1880..272ee19661f 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -16,15 +16,7 @@
  */
 package org.apache.solr.client.solrj.impl;
 
-import org.apache.solr.client.solrj.ResponseParser;
-import org.apache.solr.client.solrj.SolrClient;
-import org.apache.solr.client.solrj.SolrServerException;
-import org.apache.solr.client.solrj.request.IsUpdateRequest;
-import org.apache.solr.client.solrj.request.RequestWriter;
-import org.apache.solr.common.SolrException;
-import org.apache.solr.common.util.IOUtils;
-import org.apache.solr.common.util.NamedList;
-import org.slf4j.MDC;
+import static org.apache.solr.common.params.CommonParams.ADMIN_PATHS;
 
 import java.io.IOException;
 import java.net.ConnectException;
@@ -33,19 +25,23 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.stream.Collectors;
-
-import static org.apache.solr.common.params.CommonParams.ADMIN_PATHS;
+import org.apache.solr.client.solrj.ResponseParser;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.request.IsUpdateRequest;
+import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.NamedList;
+import org.slf4j.MDC;
 
 /**
- * This "LoadBalanced Http Solr Client" is a load balancer for multiple Http Solr Clients. This
- * is useful when you have multiple Solr endpoints and requests need to be Load Balanced among them.
+ * This "LoadBalanced Http Solr Client" is a load balancer for multiple Http Solr Clients. This is
+ * useful when you have multiple Solr endpoints and requests need to be Load Balanced among them.
  *
  * 
Do NOT use this class for indexing in leader/follower scenarios since documents must be
  * sent to the correct leader; no inter-node routing is done.
@@ -101,7 +97,7 @@
  *
  * @since solr 8.0
  */
-public class LBHttp2SolrClient> extends LBSolrClient {
+public class LBHttp2SolrClient> extends LBSolrClient {
 
   private final Map urlToClient;
   private final Set urlParamNames;
@@ -112,7 +108,7 @@ private LBHttp2SolrClient(Builder builder) {
 
     String tmpBaseSolrUrl = builder.solrClientBuilder.baseSolrUrl;
     Map buildUrlToClient = new HashMap<>();
-    for(LBSolrClient.Endpoint endpoint : builder.solrEndpoints) {
+    for (LBSolrClient.Endpoint endpoint : builder.solrEndpoints) {
       builder.solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl();
       buildUrlToClient.put(endpoint.toString(), builder.solrClientBuilder.build());
     }
@@ -122,7 +118,7 @@ private LBHttp2SolrClient(Builder builder) {
     this.aliveCheckIntervalMillis = builder.aliveCheckIntervalMillis;
     this.defaultCollection = builder.defaultCollection;
 
-    if(builder.solrClientBuilder.urlParamNames == null) {
+    if (builder.solrClientBuilder.urlParamNames == null) {
       this.urlParamNames = Collections.emptySet();
     } else {
       this.urlParamNames = Collections.unmodifiableSet(builder.solrClientBuilder.urlParamNames);
@@ -151,7 +147,7 @@ public Set getUrlParamNames() {
   @Override
   public void close() {
     super.close();
-    for(HttpSolrClientBase client : urlToClient.values()) {
+    for (HttpSolrClientBase client : urlToClient.values()) {
       IOUtils.closeQuietly(client);
     }
   }
@@ -315,7 +311,7 @@ private void onFailedRequest(
     }
   }
 
-  public static class Builder> {
+  public static class Builder> {
 
     private final B solrClientBuilder;
 
@@ -325,13 +321,14 @@ public static class Builder> {
     protected String defaultCollection;
 
     /**
-     * 
Use this Builder to configure an LBHttp2SolrClient.  The passed-in Solr Client Builder will be used to
-     * generate an internal client per specified Endpoint.
+     * Use this Builder to configure an LBHttp2SolrClient. The passed-in Solr Client Builder will be
+     * used to generate an internal client per specified Endpoint.
      *
-     * 
Implementation Note: During construction LBHttp2SolrClient will temporarily modify
-     * the Builder's {@link HttpSolrClientBuilderBase#baseSolrUrl}.
+     * 
Implementation Note: During construction LBHttp2SolrClient will temporarily modify the
+     * Builder's {@link HttpSolrClientBuilderBase#baseSolrUrl}.
      *
-     * @param solrClientBuilder A Builder like {@link Http2SolrClient.Builder} used to generate the internal clients
+     * @param solrClientBuilder A Builder like {@link Http2SolrClient.Builder} used to generate the
+     *     internal clients
      * @param endpoints the various Solr Endpoints to load balance
      */
     public Builder(B solrClientBuilder, Endpoint... endpoints) {
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
index b1476585c5d..31c75662b48 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
@@ -497,14 +497,7 @@ private static boolean isTimeExceeded(long timeAllowedNano, long timeOutTime) {
   private NamedList doRequest(Endpoint endpoint, SolrRequest solrRequest)
       throws SolrServerException, IOException {
     final var solrClient = getClient(endpoint);
-    return doRequest(solrClient, endpoint.getBaseUrl(), endpoint.getCore(), solrRequest);
-  }
-
-  //TODO remove this method NOCOMMIT
-  private NamedList doRequest(
-      SolrClient solrClient, String baseUrl, String collection, SolrRequest solrRequest)
-      throws SolrServerException, IOException {
-    return solrClient.request(solrRequest, collection);
+    return solrClient.request(solrRequest, endpoint.getCore());
   }
 
   protected Exception doRequest(
@@ -614,12 +607,7 @@ private void checkAZombieServer(EndpointWrapper zombieServer) {
       // First the one on the endpoint, then the default collection
       final String effectiveCollection =
           Objects.requireNonNullElse(zombieEndpoint.getCore(), getDefaultCollection());
-      final var responseRaw =
-          doRequest(
-              getClient(zombieEndpoint),
-              zombieEndpoint.getBaseUrl(),
-              effectiveCollection,
-              queryRequest);
+      final var responseRaw = getClient(zombieEndpoint).request(queryRequest, effectiveCollection);
       QueryResponse resp = new QueryResponse();
       resp.setResponse(responseRaw);
 
@@ -723,7 +711,7 @@ public NamedList request(
         // Choose the endpoint's core/collection over any specified by the user
         final var effectiveCollection =
             endpoint.getCore() == null ? collection : endpoint.getCore();
-        return doRequest(getClient(endpoint), endpoint.getBaseUrl(), effectiveCollection, request);
+        return getClient(endpoint).request(request, effectiveCollection);
       } catch (SolrException e) {
         // Server is alive but the request was malformed or invalid
         throw e;
@@ -764,8 +752,7 @@ public NamedList request(
         ++numServersTried;
         final String effectiveCollection =
             endpoint.getCore() == null ? collection : endpoint.getCore();
-        NamedList rsp =
-            doRequest(getClient(endpoint), endpoint.getBaseUrl(), effectiveCollection, request);
+        NamedList rsp = getClient(endpoint).request(request, effectiveCollection);
         // remove from zombie list *before* adding to the alive list to avoid a race that could lose
         // a server
         zombieServers.remove(endpoint.getUrl());
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 45ebc7a0312..e7bce730a26 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
@@ -189,9 +189,7 @@ public void testHttpClientPreservedInHttp2ClusterStateProvider() throws IOExcept
   }
 
   private void testHttpClientConsistency(
-      List solrUrls,
-      Http2SolrClient.Builder internalClientBuilder)
-      throws IOException {
+      List solrUrls, Http2SolrClient.Builder internalClientBuilder) throws IOException {
     CloudHttp2SolrClient.Builder clientBuilder = new CloudHttp2SolrClient.Builder(solrUrls);
 
     if (internalClientBuilder != null) {
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java
index 69746e03f5c..2a63892d302 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java
@@ -18,7 +18,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.lang.invoke.MethodHandles;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -316,9 +315,9 @@ public void startJetty() throws Exception {
   private static class LBClientHolder implements AutoCloseable {
 
     final LBHttp2SolrClient lbClient;
-    final HttpSolrClientBuilderBase delegate;
+    final HttpSolrClientBuilderBase delegate;
 
-    LBClientHolder(LBHttp2SolrClient lbClient, HttpSolrClientBuilderBase delegate) {
+    LBClientHolder(LBHttp2SolrClient lbClient, HttpSolrClientBuilderBase delegate) {
       this.lbClient = lbClient;
       this.delegate = delegate;
     }
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
index 5d7c911dfad..ca3ce7ce4bf 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
@@ -30,7 +30,6 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import org.apache.solr.SolrTestCase;
-import org.apache.solr.client.solrj.SolrClientFunction;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.request.QueryRequest;
@@ -53,9 +52,12 @@ public void testLBHttp2SolrClientWithTheseParamNamesInTheUrl() {
     Set urlParamNames = new HashSet<>(2);
     urlParamNames.add("param1");
 
-    var httpClientBuilder = new Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames);
+    var httpClientBuilder =
+        new Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames);
     var endpoint = new LBSolrClient.Endpoint(url);
-    try (var testClient = new LBHttp2SolrClient.Builder(httpClientBuilder, endpoint).build()) {
+    try (var testClient =
+        new LBHttp2SolrClient.Builder(httpClientBuilder, endpoint)
+            .build()) {
 
       assertArrayEquals(
           "Wrong urlParamNames found in lb client.",
@@ -74,10 +76,11 @@ public void testSynchronous() throws Exception {
     LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
     List endpointList = List.of(ep1, ep2);
 
-    var httpClientBuilder = new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
+    var httpClientBuilder =
+        new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
 
     try (LBHttp2SolrClient testClient =
-            new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
+        new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
 
       String lastEndpoint = null;
       for (int i = 0; i < 10; i++) {
@@ -101,10 +104,11 @@ public void testSynchronousWithFalures() throws Exception {
     LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
     List endpointList = List.of(ep1, ep2);
 
-    var httpClientBuilder = new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
+    var httpClientBuilder =
+        new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
 
     try (LBHttp2SolrClient testClient =
-            new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
+        new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
 
       setEndpointToFail(testClient, ep1);
       setEndpointToSucceed(testClient, ep2);
@@ -150,10 +154,11 @@ public void testAsyncWithFailures() {
     LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
     List endpointList = List.of(ep1, ep2);
 
-    var httpClientBuilder = new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
+    var httpClientBuilder =
+        new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
 
     try (LBHttp2SolrClient testClient =
-            new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
+        new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
 
       for (int j = 0; j < 2; j++) {
         // first time Endpoint One will return error code 500.
@@ -172,7 +177,8 @@ public void testAsyncWithFailures() {
           endpointToSucceed = ep1;
           endpointToFail = ep2;
         }
-        List successEndpointLastBasePaths = basePathsForEndpoint(testClient, endpointToSucceed);
+        List successEndpointLastBasePaths =
+            basePathsForEndpoint(testClient, endpointToSucceed);
         List failEndpointLastBasePaths = basePathsForEndpoint(testClient, endpointToFail);
 
         for (int i = 0; i < 10; i++) {
@@ -213,16 +219,18 @@ public void testAsyncWithFailures() {
       }
     }
   }
+
   @Test
   public void testAsync() {
     LBSolrClient.Endpoint ep1 = new LBSolrClient.Endpoint("http://endpoint.one");
     LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
     List endpointList = List.of(ep1, ep2);
 
-    var httpClientBuilder = new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
+    var httpClientBuilder =
+        new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);
 
     try (LBHttp2SolrClient testClient =
-                 new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
+        new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
 
       int limit = 10; // For simplicity use an even limit
       List> responses = new ArrayList<>();
@@ -273,32 +281,37 @@ public void testAsync() {
   private void checkSynchonousResponseContent(LBSolrClient.Rsp response, String qValue) {
     assertEquals("There should be one element in the response.", 1, response.getResponse().size());
     assertEquals(
-            "The response key 'response' should echo the query.",
-            qValue,
-            response.getResponse().get("response"));
+        "The response key 'response' should echo the query.",
+        qValue,
+        response.getResponse().get("response"));
   }
 
-  private void setEndpointToFail(LBHttp2SolrClient testClient, LBSolrClient.Endpoint ep) {
+  private void setEndpointToFail(
+      LBHttp2SolrClient testClient, LBSolrClient.Endpoint ep) {
     ((MockHttpSolrClient) testClient.getClient(ep)).allRequestsShallFail = true;
   }
 
-  private void setEndpointToSucceed(LBHttp2SolrClient testClient, LBSolrClient.Endpoint ep) {
+  private void setEndpointToSucceed(
+      LBHttp2SolrClient testClient, LBSolrClient.Endpoint ep) {
     ((MockHttpSolrClient) testClient.getClient(ep)).allRequestsShallFail = false;
   }
 
-  private List basePathsForEndpoint(LBHttp2SolrClient testClient, LBSolrClient.Endpoint ep) {
+  private List basePathsForEndpoint(
+      LBHttp2SolrClient testClient, LBSolrClient.Endpoint ep) {
     return ((MockHttpSolrClient) testClient.getClient(ep)).lastBasePaths;
   }
 
-  private List> lastSolrRequests(LBHttp2SolrClient testClient, LBSolrClient.Endpoint... endpoints) {
+  private List> lastSolrRequests(
+      LBHttp2SolrClient testClient, LBSolrClient.Endpoint... endpoints) {
     return Arrays.stream(endpoints)
-            .map(testClient::getClient)
-            .map(MockHttpSolrClient.class::cast)
-            .flatMap(c -> c.lastSolrRequests.stream())
-            .toList();
+        .map(testClient::getClient)
+        .map(MockHttpSolrClient.class::cast)
+        .flatMap(c -> c.lastSolrRequests.stream())
+        .toList();
   }
 
-  public static class MockHttpSolrClientBuilder extends HttpSolrClientBuilderBase {
+  public static class MockHttpSolrClientBuilder
+      extends HttpSolrClientBuilderBase {
 
     @Override
     public MockHttpSolrClient build() {
@@ -368,18 +381,20 @@ protected boolean isFollowRedirects() {
     }
 
     @Override
-    protected boolean processorAcceptsMimeType(Collection processorSupportedContentTypes, String mimeType) {
+    protected boolean processorAcceptsMimeType(
+        Collection processorSupportedContentTypes, String mimeType) {
       return false;
     }
 
     @Override
-    protected String allProcessorSupportedContentTypesCommaDelimited(Collection processorSupportedContentTypes) {
+    protected String allProcessorSupportedContentTypesCommaDelimited(
+        Collection processorSupportedContentTypes) {
       return null;
     }
 
     @Override
     protected void updateDefaultMimeTypeForParser() {
-      //no-op
+      // no-op
     }
   }
 }

From 90bf9e2c781d9f3998f0ad1ca6c20d68a6caeadd Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Fri, 6 Dec 2024 11:20:10 -0600
Subject: [PATCH 05/23] remove obsolete test

---
 .../impl/CloudHttp2SolrClientBuilderTest.java  | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java
index e7bce730a26..b06e7f4a367 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
@@ -121,24 +121,6 @@ public void testIsDirectUpdatesToLeadersOnlyDefault() throws IOException {
     }
   }
 
-  @Test
-  public void testExternalClientAndInternalBuilderTogether() {
-    expectThrows(
-        IllegalStateException.class,
-        () ->
-            new CloudHttp2SolrClient.Builder(
-                    Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT))
-                .withInternalClientBuilder(mock(Http2SolrClient.Builder.class))
-                .build());
-    expectThrows(
-        IllegalStateException.class,
-        () ->
-            new CloudHttp2SolrClient.Builder(
-                    Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT))
-                .withInternalClientBuilder(mock(Http2SolrClient.Builder.class))
-                .build());
-  }
-
   @Test
   public void testProvideInternalBuilder() throws IOException {
     Http2SolrClient http2Client = mock(Http2SolrClient.class);

From 51676f5ab6764f6c7356153398d469fd1b7d64e6 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Fri, 6 Dec 2024 16:23:12 -0600
Subject: [PATCH 06/23] Fix bug:  (1) Only generate a client per base url, not
 base url+core. (2) Generate new clients whenever a previously-unseen base url
 is encountered.

---
 .../client/solrj/impl/LBHttp2SolrClient.java  | 40 +++++++++++++------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index 272ee19661f..b96782e8e98 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -28,6 +28,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import org.apache.solr.client.solrj.ResponseParser;
@@ -102,18 +103,17 @@ public class LBHttp2SolrClient> extend
   private final Map urlToClient;
   private final Set urlParamNames;
 
+  private final Builder builder;
+
   @SuppressWarnings("unchecked")
   private LBHttp2SolrClient(Builder builder) {
     super(Arrays.asList(builder.solrEndpoints));
+    this.builder = builder;
 
-    String tmpBaseSolrUrl = builder.solrClientBuilder.baseSolrUrl;
-    Map buildUrlToClient = new HashMap<>();
+    this.urlToClient = new ConcurrentHashMap<>();
     for (LBSolrClient.Endpoint endpoint : builder.solrEndpoints) {
-      builder.solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl();
-      buildUrlToClient.put(endpoint.toString(), builder.solrClientBuilder.build());
+      buildClient(endpoint);
     }
-    builder.solrClientBuilder.baseSolrUrl = tmpBaseSolrUrl;
-    this.urlToClient = Collections.unmodifiableMap(buildUrlToClient);
 
     this.aliveCheckIntervalMillis = builder.aliveCheckIntervalMillis;
     this.defaultCollection = builder.defaultCollection;
@@ -125,9 +125,25 @@ private LBHttp2SolrClient(Builder builder) {
     }
   }
 
+  private synchronized HttpSolrClientBase buildClient(Endpoint endpoint) {
+    var client = urlToClient.get(endpoint.toString());
+    if(client == null) {
+      String tmpBaseSolrUrl = builder.solrClientBuilder.baseSolrUrl;
+      builder.solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl();
+      client = builder.solrClientBuilder.build();
+      urlToClient.put(endpoint.getBaseUrl(), client);
+      builder.solrClientBuilder.baseSolrUrl = tmpBaseSolrUrl;
+    }
+    return client;
+  }
+
   @Override
   protected HttpSolrClientBase getClient(Endpoint endpoint) {
-    return urlToClient.get(endpoint.toString());
+    var client =  urlToClient.get(endpoint.getBaseUrl());
+    if(client == null) {
+      return buildClient(endpoint);
+    }
+    return client;
   }
 
   @Override
@@ -238,7 +254,7 @@ private CompletableFuture> doAsyncRequest(
     String baseUrl = endpoint.toString();
     rsp.server = baseUrl;
     final var client = getClient(endpoint);
-    CompletableFuture> future = client.requestAsync(req.getRequest());
+    CompletableFuture> future = client.requestAsync(req.getRequest(), endpoint.getCore());
     future.whenComplete(
         (result, throwable) -> {
           if (!future.isCompletedExceptionally()) {
@@ -322,14 +338,14 @@ public static class Builder> {
 
     /**
      * Use this Builder to configure an LBHttp2SolrClient. The passed-in Solr Client Builder will be
-     * used to generate an internal client per specified Endpoint.
+     * used to generate an internal client per Endpoint.
      *
-     * 
Implementation Note: During construction LBHttp2SolrClient will temporarily modify the
-     * Builder's {@link HttpSolrClientBuilderBase#baseSolrUrl}.
+     * 
Implementation Note: LBHttp2SolrClient will temporarily modify the passed-in Builder's
+     * {@link HttpSolrClientBuilderBase#baseSolrUrl} whenever it needs to generate a new Http Solr Client.
      *
      * @param solrClientBuilder A Builder like {@link Http2SolrClient.Builder} used to generate the
      *     internal clients
-     * @param endpoints the various Solr Endpoints to load balance
+     * @param endpoints the initial Solr Endpoints to load balance
      */
     public Builder(B solrClientBuilder, Endpoint... endpoints) {
       this.solrClientBuilder = solrClientBuilder;

From db76064057cd44ee092a3d9a6a4fd6b82145d23e Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Fri, 6 Dec 2024 16:26:03 -0600
Subject: [PATCH 07/23] tidy

---
 .../solr/client/solrj/impl/LBHttp2SolrClient.java   | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index b96782e8e98..e8c76c1a4e6 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -24,7 +24,6 @@
 import java.net.SocketTimeoutException;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
@@ -127,7 +126,7 @@ private LBHttp2SolrClient(Builder builder) {
 
   private synchronized HttpSolrClientBase buildClient(Endpoint endpoint) {
     var client = urlToClient.get(endpoint.toString());
-    if(client == null) {
+    if (client == null) {
       String tmpBaseSolrUrl = builder.solrClientBuilder.baseSolrUrl;
       builder.solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl();
       client = builder.solrClientBuilder.build();
@@ -139,8 +138,8 @@ private synchronized HttpSolrClientBase buildClient(Endpoint endpoint) {
 
   @Override
   protected HttpSolrClientBase getClient(Endpoint endpoint) {
-    var client =  urlToClient.get(endpoint.getBaseUrl());
-    if(client == null) {
+    var client = urlToClient.get(endpoint.getBaseUrl());
+    if (client == null) {
       return buildClient(endpoint);
     }
     return client;
@@ -254,7 +253,8 @@ private CompletableFuture> doAsyncRequest(
     String baseUrl = endpoint.toString();
     rsp.server = baseUrl;
     final var client = getClient(endpoint);
-    CompletableFuture> future = client.requestAsync(req.getRequest(), endpoint.getCore());
+    CompletableFuture> future =
+        client.requestAsync(req.getRequest(), endpoint.getCore());
     future.whenComplete(
         (result, throwable) -> {
           if (!future.isCompletedExceptionally()) {
@@ -341,7 +341,8 @@ public static class Builder> {
      * used to generate an internal client per Endpoint.
      *
      * 
Implementation Note: LBHttp2SolrClient will temporarily modify the passed-in Builder's
-     * {@link HttpSolrClientBuilderBase#baseSolrUrl} whenever it needs to generate a new Http Solr Client.
+     * {@link HttpSolrClientBuilderBase#baseSolrUrl} whenever it needs to generate a new Http Solr
+     * Client.
      *
      * @param solrClientBuilder A Builder like {@link Http2SolrClient.Builder} used to generate the
      *     internal clients

From 605bdc7831de2eab3c3b5f4c15e9df59777cad83 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Fri, 6 Dec 2024 17:16:05 -0600
Subject: [PATCH 08/23] Only hold the internal builder

---
 .../solr/client/solrj/impl/LBHttp2SolrClient.java    | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index e8c76c1a4e6..97ef57319bc 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -102,12 +102,12 @@ public class LBHttp2SolrClient> extend
   private final Map urlToClient;
   private final Set urlParamNames;
 
-  private final Builder builder;
+  private final HttpSolrClientBuilderBase solrClientBuilder;
 
   @SuppressWarnings("unchecked")
   private LBHttp2SolrClient(Builder builder) {
     super(Arrays.asList(builder.solrEndpoints));
-    this.builder = builder;
+    this.solrClientBuilder = builder.solrClientBuilder;
 
     this.urlToClient = new ConcurrentHashMap<>();
     for (LBSolrClient.Endpoint endpoint : builder.solrEndpoints) {
@@ -127,11 +127,11 @@ private LBHttp2SolrClient(Builder builder) {
   private synchronized HttpSolrClientBase buildClient(Endpoint endpoint) {
     var client = urlToClient.get(endpoint.toString());
     if (client == null) {
-      String tmpBaseSolrUrl = builder.solrClientBuilder.baseSolrUrl;
-      builder.solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl();
-      client = builder.solrClientBuilder.build();
+      String tmpBaseSolrUrl = solrClientBuilder.baseSolrUrl;
+      solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl();
+      client = solrClientBuilder.build();
       urlToClient.put(endpoint.getBaseUrl(), client);
-      builder.solrClientBuilder.baseSolrUrl = tmpBaseSolrUrl;
+      solrClientBuilder.baseSolrUrl = tmpBaseSolrUrl;
     }
     return client;
   }

From 508af0d0da3eebd37fd8f179a68ebc3ee735a6dd Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Fri, 6 Dec 2024 17:40:00 -0600
Subject: [PATCH 09/23] remove override base url from jdk client

---
 .../client/solrj/impl/HttpJdkSolrClient.java  | 23 ++++---------------
 .../solrj/impl/HttpJdkSolrClientTest.java     | 20 ----------------
 2 files changed, 5 insertions(+), 38 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
index 1127b3fd1a1..bbcd98d2736 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
@@ -138,7 +138,7 @@ protected HttpJdkSolrClient(String serverBaseUrl, HttpJdkSolrClient.Builder buil
   public CompletableFuture> requestAsync(
       final SolrRequest solrRequest, String collection) {
     try {
-      PreparedRequest pReq = prepareRequest(solrRequest, collection, null);
+      PreparedRequest pReq = prepareRequest(solrRequest, collection);
       return httpClient
           .sendAsync(pReq.reqb.build(), HttpResponse.BodyHandlers.ofInputStream())
           .thenApply(
@@ -157,10 +157,9 @@ public CompletableFuture> requestAsync(
     }
   }
 
-  protected NamedList requestWithBaseUrl(
-      String baseUrl, SolrRequest solrRequest, String collection)
+  public NamedList request(SolrRequest solrRequest, String collection)
       throws SolrServerException, IOException {
-    PreparedRequest pReq = prepareRequest(solrRequest, collection, baseUrl);
+    PreparedRequest pReq = prepareRequest(solrRequest, collection);
     HttpResponse response = null;
     try {
       response = httpClient.send(pReq.reqb.build(), HttpResponse.BodyHandlers.ofInputStream());
@@ -192,25 +191,13 @@ protected NamedList requestWithBaseUrl(
     }
   }
 
-  @Override
-  public NamedList request(SolrRequest solrRequest, String collection)
-      throws SolrServerException, IOException {
-    return requestWithBaseUrl(null, solrRequest, collection);
-  }
-
-  private PreparedRequest prepareRequest(
-      SolrRequest solrRequest, String collection, String overrideBaseUrl)
+  private PreparedRequest prepareRequest(SolrRequest solrRequest, String collection)
       throws SolrServerException, IOException {
     checkClosed();
     if (ClientUtils.shouldApplyDefaultCollection(collection, solrRequest)) {
       collection = defaultCollection;
     }
-    String url;
-    if (overrideBaseUrl != null) {
-      url = ClientUtils.buildRequestUrl(solrRequest, overrideBaseUrl, collection);
-    } else {
-      url = getRequestUrl(solrRequest, collection);
-    }
+    String url = getRequestUrl(solrRequest, collection);
     ResponseParser parserToUse = responseParser(solrRequest);
     ModifiableSolrParams queryParams = initializeSolrParams(solrRequest, parserToUse);
     var reqb = HttpRequest.newBuilder();
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 b3980ad44bc..1dbfc0e998b 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
@@ -34,7 +34,6 @@
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
-import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.client.solrj.request.RequestWriter;
 import org.apache.solr.client.solrj.response.SolrPingResponse;
 import org.apache.solr.common.params.CommonParams;
@@ -154,25 +153,6 @@ protected void testQuerySetup(SolrRequest.METHOD method, ResponseParser rp) thro
     }
   }
 
-  @Test
-  public void testRequestWithBaseUrl() throws Exception {
-    DebugServlet.clear();
-    DebugServlet.addResponseHeader("Content-Type", "application/octet-stream");
-    DebugServlet.responseBodyByQueryFragment.put("", javabinResponse());
-    String someOtherUrl = getBaseUrl() + "/some/other/base/url";
-    String intendedUrl = getBaseUrl() + DEBUG_SERVLET_PATH;
-    SolrQuery q = new SolrQuery("foo");
-    q.setParam("a", MUST_ENCODE);
-
-    HttpJdkSolrClient.Builder b =
-        builder(someOtherUrl).withResponseParser(new BinaryResponseParser());
-    try (HttpJdkSolrClient client = b.build()) {
-      client.requestWithBaseUrl(intendedUrl, new QueryRequest(q, SolrRequest.METHOD.GET), null);
-      assertEquals(
-          client.getParser().getVersion(), DebugServlet.parameters.get(CommonParams.VERSION)[0]);
-    }
-  }
-
   @Test
   public void testGetById() throws Exception {
     DebugServlet.clear();

From 39177b4e4081d6e6551c0cc28194be5261250b42 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Mon, 9 Dec 2024 10:19:08 -0600
Subject: [PATCH 10/23] fix test

---
 .../src/test/org/apache/solr/cloud/OverseerTest.java     | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
index bad8d58d021..e611902898f 100644
--- a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
@@ -1945,17 +1945,16 @@ public Void answer(InvocationOnMock invocation) {
   }
 
   private SolrCloudManager getCloudDataProvider(ZkStateReader zkStateReader) {
-    var httpSolrClient =
+    var httpSolrClientBuilder =
         new Http2SolrClient.Builder()
             .withIdleTimeout(30000, TimeUnit.MILLISECONDS)
-            .withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
-            .build();
+            .withConnectionTimeout(15000, TimeUnit.MILLISECONDS);
     var cloudSolrClient =
         new CloudHttp2SolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader))
-            .withHttpClient(httpSolrClient)
+            .withInternalClientBuilder(httpSolrClientBuilder)
             .build();
     solrClients.add(cloudSolrClient);
-    solrClients.add(httpSolrClient);
+    solrClients.add(httpSolrClientBuilder.build());
     SolrClientCloudManager sccm = new SolrClientCloudManager(cloudSolrClient, null);
     sccm.getClusterStateProvider().connect();
     return sccm;

From 19577dc3828f571637eae6fba9c4afa2fc958875 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Mon, 9 Dec 2024 10:51:19 -0600
Subject: [PATCH 11/23] add missing @Override

---
 .../org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java     | 1 +
 1 file changed, 1 insertion(+)

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 bbcd98d2736..c26d13606b6 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
@@ -157,6 +157,7 @@ public CompletableFuture> requestAsync(
     }
   }
 
+  @Override
   public NamedList request(SolrRequest solrRequest, String collection)
       throws SolrServerException, IOException {
     PreparedRequest pReq = prepareRequest(solrRequest, collection);

From ca59536ebcb61c5853142b84043a9e0c0c93ace7 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Tue, 10 Dec 2024 15:21:08 -0600
Subject: [PATCH 12/23] implement code review suggestions

---
 .../client/solrj/impl/LBHttp2SolrClient.java  | 26 ++++++++-----------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index 97ef57319bc..cd8eac088ad 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -120,25 +120,23 @@ private LBHttp2SolrClient(Builder builder) {
     if (builder.solrClientBuilder.urlParamNames == null) {
       this.urlParamNames = Collections.emptySet();
     } else {
-      this.urlParamNames = Collections.unmodifiableSet(builder.solrClientBuilder.urlParamNames);
+      this.urlParamNames = Set.copyOf(builder.solrClientBuilder.urlParamNames);
     }
   }
 
-  private synchronized HttpSolrClientBase buildClient(Endpoint endpoint) {
-    var client = urlToClient.get(endpoint.toString());
-    if (client == null) {
-      String tmpBaseSolrUrl = solrClientBuilder.baseSolrUrl;
-      solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl();
-      client = solrClientBuilder.build();
-      urlToClient.put(endpoint.getBaseUrl(), client);
-      solrClientBuilder.baseSolrUrl = tmpBaseSolrUrl;
-    }
+  private HttpSolrClientBase buildClient(Endpoint endpoint) {
+    String tmpBaseSolrUrl = solrClientBuilder.baseSolrUrl;
+    solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl();
+    var client = solrClientBuilder.build();
+    urlToClient.put(endpoint.getBaseUrl(), client);
+    solrClientBuilder.baseSolrUrl = tmpBaseSolrUrl;
     return client;
   }
 
   @Override
-  protected HttpSolrClientBase getClient(Endpoint endpoint) {
-    var client = urlToClient.get(endpoint.getBaseUrl());
+  protected HttpSolrClientBase getClient(final Endpoint endpoint) {
+    var client =
+        urlToClient.computeIfAbsent(endpoint.getBaseUrl(), s -> this.buildClient(endpoint));
     if (client == null) {
       return buildClient(endpoint);
     }
@@ -162,9 +160,7 @@ public Set getUrlParamNames() {
   @Override
   public void close() {
     super.close();
-    for (HttpSolrClientBase client : urlToClient.values()) {
-      IOUtils.closeQuietly(client);
-    }
+    urlToClient.values().forEach(IOUtils::closeQuietly);
   }
 
   /**

From 67c8b8f85646ce3ce9e8b6eb210eff805c6173f2 Mon Sep 17 00:00:00 2001
From: David Smiley 
Date: Thu, 12 Dec 2024 01:20:45 -0500
Subject: [PATCH 13/23] fix getClient/buildClient

---
 .../client/solrj/impl/LBHttp2SolrClient.java  | 33 +++++++------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index cd8eac088ad..623f0742984 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -102,18 +102,13 @@ public class LBHttp2SolrClient> extend
   private final Map urlToClient;
   private final Set urlParamNames;
 
+  // must synchronize on this when building
   private final HttpSolrClientBuilderBase solrClientBuilder;
 
-  @SuppressWarnings("unchecked")
   private LBHttp2SolrClient(Builder builder) {
     super(Arrays.asList(builder.solrEndpoints));
     this.solrClientBuilder = builder.solrClientBuilder;
 
-    this.urlToClient = new ConcurrentHashMap<>();
-    for (LBSolrClient.Endpoint endpoint : builder.solrEndpoints) {
-      buildClient(endpoint);
-    }
-
     this.aliveCheckIntervalMillis = builder.aliveCheckIntervalMillis;
     this.defaultCollection = builder.defaultCollection;
 
@@ -122,25 +117,21 @@ private LBHttp2SolrClient(Builder builder) {
     } else {
       this.urlParamNames = Set.copyOf(builder.solrClientBuilder.urlParamNames);
     }
-  }
 
-  private HttpSolrClientBase buildClient(Endpoint endpoint) {
-    String tmpBaseSolrUrl = solrClientBuilder.baseSolrUrl;
-    solrClientBuilder.baseSolrUrl = endpoint.getBaseUrl();
-    var client = solrClientBuilder.build();
-    urlToClient.put(endpoint.getBaseUrl(), client);
-    solrClientBuilder.baseSolrUrl = tmpBaseSolrUrl;
-    return client;
+    this.urlToClient = new ConcurrentHashMap<>();
+    for (LBSolrClient.Endpoint endpoint : builder.solrEndpoints) {
+      getClient(endpoint);
+    }
   }
 
   @Override
   protected HttpSolrClientBase getClient(final Endpoint endpoint) {
-    var client =
-        urlToClient.computeIfAbsent(endpoint.getBaseUrl(), s -> this.buildClient(endpoint));
-    if (client == null) {
-      return buildClient(endpoint);
-    }
-    return client;
+    return urlToClient.computeIfAbsent(endpoint.getBaseUrl(), url -> {
+      synchronized (solrClientBuilder) {
+        solrClientBuilder.baseSolrUrl = url;
+        return solrClientBuilder.build();
+      }
+    });
   }
 
   @Override
@@ -159,8 +150,8 @@ public Set getUrlParamNames() {
 
   @Override
   public void close() {
-    super.close();
     urlToClient.values().forEach(IOUtils::closeQuietly);
+    super.close();
   }
 
   /**

From 7b9086285827c888b8a9421d79d1b878e0e04651 Mon Sep 17 00:00:00 2001
From: David Smiley 
Date: Thu, 12 Dec 2024 02:07:56 -0500
Subject: [PATCH 14/23] more

---
 .../handler/component/HttpShardHandlerFactory.java |  7 +++----
 .../src/test/org/apache/solr/cli/PostToolTest.java |  1 +
 .../solr/client/solrj/impl/LBHttp2SolrClient.java  | 14 ++++++++------
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
index 6978717ccdf..e431825267b 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
@@ -310,12 +310,11 @@ public void init(PluginInfo info) {
             .withConnectionTimeout(connectionTimeout, TimeUnit.MILLISECONDS)
             .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS)
             .withExecutor(commExecutor)
-            .withMaxConnectionsPerHost(maxConnectionsPerHost);
+            .withMaxConnectionsPerHost(maxConnectionsPerHost)
+            .withListenerFactory(List.of(this.httpListenerFactory));
     this.defaultClient = defaultClientBuilder.build();
 
-    this.defaultClient.addListenerFactory(this.httpListenerFactory);
-    this.loadbalancer =
-        new LBHttp2SolrClient.Builder(defaultClientBuilder).build();
+    this.loadbalancer = new LBHttp2SolrClient.Builder<>(defaultClientBuilder).build();
 
     initReplicaListTransformers(getParameter(args, "replicaRouting", null, sb));
 
diff --git a/solr/core/src/test/org/apache/solr/cli/PostToolTest.java b/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
index 425c9f495dd..f77929700aa 100644
--- a/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
+++ b/solr/core/src/test/org/apache/solr/cli/PostToolTest.java
@@ -76,6 +76,7 @@ public void testBasicRun() throws Exception {
 
     withBasicAuth(CollectionAdminRequest.createCollection(collection, "conf1", 1, 1, 0, 0))
         .processAndWait(cluster.getSolrClient(), 10);
+    waitForState("creating", collection, activeClusterShape(1, 1));
 
     File jsonDoc = File.createTempFile("temp", ".json");
 
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index 623f0742984..b64ead67652 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -126,12 +126,14 @@ private LBHttp2SolrClient(Builder builder) {
 
   @Override
   protected HttpSolrClientBase getClient(final Endpoint endpoint) {
-    return urlToClient.computeIfAbsent(endpoint.getBaseUrl(), url -> {
-      synchronized (solrClientBuilder) {
-        solrClientBuilder.baseSolrUrl = url;
-        return solrClientBuilder.build();
-      }
-    });
+    return urlToClient.computeIfAbsent(
+        endpoint.getBaseUrl(),
+        url -> {
+          synchronized (solrClientBuilder) {
+            solrClientBuilder.baseSolrUrl = url;
+            return solrClientBuilder.build();
+          }
+        });
   }
 
   @Override

From 1539d97ca1fdca235bcb9717853055909de25470 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Fri, 13 Dec 2024 17:44:18 -0600
Subject: [PATCH 15/23] PKI Authentication Plugin to add the listener factory
 to subsequently-built clients

---
 .../apache/solr/core/HttpSolrClientProvider.java |  6 ++++--
 .../component/HttpShardHandlerFactory.java       | 11 +++++++----
 .../solr/security/HttpClientBuilderPlugin.java   |  4 ++++
 .../solr/security/PKIAuthenticationPlugin.java   | 16 +++++++++++++++-
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java b/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
index 2bf25a896f6..b10eae05f24 100644
--- a/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
+++ b/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
@@ -37,13 +37,15 @@ final class HttpSolrClientProvider implements AutoCloseable {
 
   private final Http2SolrClient httpSolrClient;
 
+  private final Http2SolrClient.Builder httpClientBuilder;
+
   private final InstrumentedHttpListenerFactory trackHttpSolrMetrics;
 
   HttpSolrClientProvider(UpdateShardHandlerConfig cfg, SolrMetricsContext parentContext) {
     trackHttpSolrMetrics = new InstrumentedHttpListenerFactory(getNameStrategy(cfg));
     initializeMetrics(parentContext);
 
-    Http2SolrClient.Builder httpClientBuilder =
+    this.httpClientBuilder =
         new Http2SolrClient.Builder().withListenerFactory(List.of(trackHttpSolrMetrics));
 
     if (cfg != null) {
@@ -76,7 +78,7 @@ Http2SolrClient getSolrClient() {
   }
 
   void setSecurityBuilder(HttpClientBuilderPlugin builder) {
-    builder.setup(httpSolrClient);
+    builder.setup(httpClientBuilder, httpSolrClient);
   }
 
   @Override
diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
index e431825267b..f2b6d69e1df 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
@@ -83,6 +83,7 @@ public class HttpShardHandlerFactory extends ShardHandlerFactory
   protected ExecutorService commExecutor;
 
   protected volatile Http2SolrClient defaultClient;
+  protected Http2SolrClient.Builder httpClientBuilder;
   protected InstrumentedHttpListenerFactory httpListenerFactory;
   protected LBHttp2SolrClient loadbalancer;
 
@@ -305,16 +306,16 @@ public void init(PluginInfo info) {
             sb);
     int soTimeout =
         getParameter(args, HttpClientUtil.PROP_SO_TIMEOUT, HttpClientUtil.DEFAULT_SO_TIMEOUT, sb);
-    var defaultClientBuilder =
+    this.httpClientBuilder =
         new Http2SolrClient.Builder()
             .withConnectionTimeout(connectionTimeout, TimeUnit.MILLISECONDS)
             .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS)
             .withExecutor(commExecutor)
             .withMaxConnectionsPerHost(maxConnectionsPerHost)
             .withListenerFactory(List.of(this.httpListenerFactory));
-    this.defaultClient = defaultClientBuilder.build();
+    this.defaultClient = httpClientBuilder.build();
 
-    this.loadbalancer = new LBHttp2SolrClient.Builder<>(defaultClientBuilder).build();
+    this.loadbalancer = new LBHttp2SolrClient.Builder<>(httpClientBuilder).build();
 
     initReplicaListTransformers(getParameter(args, "replicaRouting", null, sb));
 
@@ -323,8 +324,10 @@ public void init(PluginInfo info) {
 
   @Override
   public void setSecurityBuilder(HttpClientBuilderPlugin clientBuilderPlugin) {
+    System.out.println("IN SET SEC BUILDER");
+    new Exception().printStackTrace(System.out);
     if (clientBuilderPlugin != null) {
-      clientBuilderPlugin.setup(defaultClient);
+      clientBuilderPlugin.setup(httpClientBuilder, defaultClient);
     }
   }
 
diff --git a/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java b/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
index b43d5f22190..dfd414e3b36 100644
--- a/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/HttpClientBuilderPlugin.java
@@ -34,4 +34,8 @@ public interface HttpClientBuilderPlugin {
   public SolrHttpClientBuilder getHttpClientBuilder(SolrHttpClientBuilder builder);
 
   public default void setup(Http2SolrClient client) {}
+
+  public default void setup(Http2SolrClient.Builder httpClientBuilder, Http2SolrClient client) {
+    setup(client);
+  }
 }
diff --git a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
index b1f6e6b6eed..a577fe1cbad 100644
--- a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
@@ -374,8 +374,17 @@ PublicKey fetchPublicKeyFromRemote(String nodename) {
     }
   }
 
+  @Override
+  public void setup(Http2SolrClient.Builder httpClientBuilder, Http2SolrClient client) {
+    setup(client, httpClientBuilder);
+  }
+
   @Override
   public void setup(Http2SolrClient client) {
+    setup(client, null);
+  }
+
+  private void setup(Http2SolrClient client, Http2SolrClient.Builder builder) {
     final HttpListenerFactory.RequestResponseListener listener =
         new HttpListenerFactory.RequestResponseListener() {
           private static final String CACHED_REQUEST_USER_KEY = "cachedRequestUser";
@@ -431,7 +440,12 @@ private Optional getUserFromJettyRequest(Request request) {
                 (String) request.getAttributes().get(CACHED_REQUEST_USER_KEY));
           }
         };
-    client.addListenerFactory(() -> listener);
+    if(client != null) {
+      client.addListenerFactory(() -> listener);
+    }
+    if(builder != null) {
+      builder.withListenerFactory(List.of(() -> listener));
+    }
   }
 
   @Override

From 8f172790d11ebe4dc1a795d8171e08e02dcc1ab0 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Fri, 13 Dec 2024 17:51:02 -0600
Subject: [PATCH 16/23] @lucene.experimental annotations

---
 .../org/apache/solr/client/solrj/SolrClientFunction.java     | 5 ++++-
 .../org/apache/solr/client/solrj/impl/Http2SolrClient.java   | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java
index 0adb49471dc..38ddfea9842 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java
@@ -18,7 +18,10 @@
 
 import java.io.IOException;
 
-/** A lambda intended for invoking SolrClient operations */
+/** A lambda intended for invoking SolrClient operations
+ *
+ * @lucene.experimental
+ * */
 @FunctionalInterface
 public interface SolrClientFunction {
   R apply(C c) throws IOException, SolrServerException;
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 fb2eb1a123f..cb5a55f9f1e 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
@@ -571,6 +571,8 @@ public final  R requestWithBaseUrl(
    * @param clientFunction a Function that consumes a Http2SolrClient and returns an arbitrary value
    * @return the value returned after invoking 'clientFunction'
    * @param  the type returned by the provided function (and by this method)
+   *
+   * @lucene.experimental
    */
   public  R requestWithBaseUrl(
       String baseUrl, SolrClientFunction clientFunction)

From effedb9b3855c54c30edfe2ef9c0bc66c8d901da Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Fri, 13 Dec 2024 17:52:04 -0600
Subject: [PATCH 17/23] tidy

---
 .../org/apache/solr/security/PKIAuthenticationPlugin.java    | 4 ++--
 .../org/apache/solr/client/solrj/SolrClientFunction.java     | 5 +++--
 .../org/apache/solr/client/solrj/impl/Http2SolrClient.java   | 1 -
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
index a577fe1cbad..f956a367ccf 100644
--- a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
@@ -440,10 +440,10 @@ private Optional getUserFromJettyRequest(Request request) {
                 (String) request.getAttributes().get(CACHED_REQUEST_USER_KEY));
           }
         };
-    if(client != null) {
+    if (client != null) {
       client.addListenerFactory(() -> listener);
     }
-    if(builder != null) {
+    if (builder != null) {
       builder.withListenerFactory(List.of(() -> listener));
     }
   }
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java
index 38ddfea9842..68246001a40 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClientFunction.java
@@ -18,10 +18,11 @@
 
 import java.io.IOException;
 
-/** A lambda intended for invoking SolrClient operations
+/**
+ * A lambda intended for invoking SolrClient operations
  *
  * @lucene.experimental
- * */
+ */
 @FunctionalInterface
 public interface SolrClientFunction {
   R apply(C c) throws IOException, SolrServerException;
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 cb5a55f9f1e..b2dd4c94566 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
@@ -571,7 +571,6 @@ public final  R requestWithBaseUrl(
    * @param clientFunction a Function that consumes a Http2SolrClient and returns an arbitrary value
    * @return the value returned after invoking 'clientFunction'
    * @param  the type returned by the provided function (and by this method)
-   *
    * @lucene.experimental
    */
   public  R requestWithBaseUrl(

From 29a34456d46fbe510db1fba8420c9401211cef5b Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Fri, 13 Dec 2024 17:58:29 -0600
Subject: [PATCH 18/23] remove system.out.println

---
 .../apache/solr/handler/component/HttpShardHandlerFactory.java  | 2 --
 1 file changed, 2 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
index f2b6d69e1df..29b4c953fdb 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
@@ -324,8 +324,6 @@ public void init(PluginInfo info) {
 
   @Override
   public void setSecurityBuilder(HttpClientBuilderPlugin clientBuilderPlugin) {
-    System.out.println("IN SET SEC BUILDER");
-    new Exception().printStackTrace(System.out);
     if (clientBuilderPlugin != null) {
       clientBuilderPlugin.setup(httpClientBuilder, defaultClient);
     }

From 50470cd310f6f0a6ccc640ac4407711b4088f624 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Mon, 16 Dec 2024 08:38:21 -0600
Subject: [PATCH 19/23] httpClientBuilder > httpSolrClientBuilder

---
 .../handler/component/HttpShardHandlerFactory.java     | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
index 29b4c953fdb..14e79e6866c 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
@@ -83,7 +83,7 @@ public class HttpShardHandlerFactory extends ShardHandlerFactory
   protected ExecutorService commExecutor;
 
   protected volatile Http2SolrClient defaultClient;
-  protected Http2SolrClient.Builder httpClientBuilder;
+  protected Http2SolrClient.Builder httpSolrClientBuilder;
   protected InstrumentedHttpListenerFactory httpListenerFactory;
   protected LBHttp2SolrClient loadbalancer;
 
@@ -306,16 +306,16 @@ public void init(PluginInfo info) {
             sb);
     int soTimeout =
         getParameter(args, HttpClientUtil.PROP_SO_TIMEOUT, HttpClientUtil.DEFAULT_SO_TIMEOUT, sb);
-    this.httpClientBuilder =
+    this.httpSolrClientBuilder =
         new Http2SolrClient.Builder()
             .withConnectionTimeout(connectionTimeout, TimeUnit.MILLISECONDS)
             .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS)
             .withExecutor(commExecutor)
             .withMaxConnectionsPerHost(maxConnectionsPerHost)
             .withListenerFactory(List.of(this.httpListenerFactory));
-    this.defaultClient = httpClientBuilder.build();
+    this.defaultClient = httpSolrClientBuilder.build();
 
-    this.loadbalancer = new LBHttp2SolrClient.Builder<>(httpClientBuilder).build();
+    this.loadbalancer = new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder).build();
 
     initReplicaListTransformers(getParameter(args, "replicaRouting", null, sb));
 
@@ -325,7 +325,7 @@ public void init(PluginInfo info) {
   @Override
   public void setSecurityBuilder(HttpClientBuilderPlugin clientBuilderPlugin) {
     if (clientBuilderPlugin != null) {
-      clientBuilderPlugin.setup(httpClientBuilder, defaultClient);
+      clientBuilderPlugin.setup(httpSolrClientBuilder, defaultClient);
     }
   }
 

From f423bd25778f0e8cefbef8838810a5679f594267 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Mon, 16 Dec 2024 09:08:13 -0600
Subject: [PATCH 20/23] API change to Http2SolrClient.Builder: - remove
 'withListenerFactory', replace with 'withListenerFactories' - add
 'addListenerFactory'

---
 .../solr/core/HttpSolrClientProvider.java     |  2 +-
 .../component/HttpShardHandlerFactory.java    |  2 +-
 .../security/PKIAuthenticationPlugin.java     |  2 +-
 .../client/solrj/impl/Http2SolrClient.java    | 34 ++++++++++++++-----
 4 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java b/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
index b10eae05f24..c209940dbbd 100644
--- a/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
+++ b/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
@@ -46,7 +46,7 @@ final class HttpSolrClientProvider implements AutoCloseable {
     initializeMetrics(parentContext);
 
     this.httpClientBuilder =
-        new Http2SolrClient.Builder().withListenerFactory(List.of(trackHttpSolrMetrics));
+        new Http2SolrClient.Builder().addListenerFactory(trackHttpSolrMetrics);
 
     if (cfg != null) {
       httpClientBuilder
diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
index 14e79e6866c..1437dee63ea 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
@@ -312,7 +312,7 @@ public void init(PluginInfo info) {
             .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS)
             .withExecutor(commExecutor)
             .withMaxConnectionsPerHost(maxConnectionsPerHost)
-            .withListenerFactory(List.of(this.httpListenerFactory));
+            .addListenerFactory(this.httpListenerFactory);
     this.defaultClient = httpSolrClientBuilder.build();
 
     this.loadbalancer = new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder).build();
diff --git a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
index f956a367ccf..6a129a137e9 100644
--- a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java
@@ -444,7 +444,7 @@ private Optional getUserFromJettyRequest(Request request) {
       client.addListenerFactory(() -> listener);
     }
     if (builder != null) {
-      builder.withListenerFactory(List.of(() -> listener));
+      builder.addListenerFactory(() -> listener);
     }
   }
 
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 b2dd4c94566..9dcd00936e2 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
@@ -140,8 +140,8 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) {
       this.httpClient = createHttpClient(builder);
       this.closeClient = true;
     }
-    if (builder.listenerFactory != null) {
-      this.listenerFactory.addAll(builder.listenerFactory);
+    if (builder.listenerFactories != null) {
+      this.listenerFactory.addAll(builder.listenerFactories);
     }
     updateDefaultMimeTypeForParser();
 
@@ -907,7 +907,7 @@ public static class Builder
 
     protected Long keyStoreReloadIntervalSecs;
 
-    private List listenerFactory;
+    private List listenerFactories = new ArrayList<>(0);
 
     public Builder() {
       super();
@@ -933,8 +933,26 @@ public Builder(String baseSolrUrl) {
       this.baseSolrUrl = baseSolrUrl;
     }
 
-    public Http2SolrClient.Builder withListenerFactory(List listenerFactory) {
-      this.listenerFactory = listenerFactory;
+    /**
+     * specify a listener factory, which will be appened to any existing values.
+     *
+     * @param listenerFactory a HttpListenerFactory
+     * @return This Builder
+     */
+    public Http2SolrClient.Builder addListenerFactory(HttpListenerFactory listenerFactory) {
+      this.listenerFactories.add(listenerFactory);
+      return this;
+    }
+
+    /**
+     * Specify listener factories, which will replace any existing values.
+     *
+     * @param listenerFactories a list of HttpListenerFactory instances
+     * @return This Builder
+     */
+    public Http2SolrClient.Builder withListenerFactories(List listenerFactories) {
+      this.listenerFactories.clear();
+      this.listenerFactories.addAll(listenerFactories);
       return this;
     }
 
@@ -1110,9 +1128,9 @@ public Builder withHttpClient(Http2SolrClient http2SolrClient) {
       if (this.urlParamNames == null) {
         this.urlParamNames = http2SolrClient.urlParamNames;
       }
-      if (this.listenerFactory == null) {
-        this.listenerFactory = new ArrayList();
-        http2SolrClient.listenerFactory.forEach(this.listenerFactory::add);
+      if (this.listenerFactories.isEmpty()) {
+        this.listenerFactories.clear();
+        http2SolrClient.listenerFactory.forEach(this.listenerFactories::add);
       }
       if (this.executor == null) {
         this.executor = http2SolrClient.executor;

From 6176a9929cf4d20b3ff64971659d6c2d06a17cac Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Mon, 16 Dec 2024 09:12:47 -0600
Subject: [PATCH 21/23] Remove unnecessary null check

---
 .../org/apache/solr/client/solrj/impl/Http2SolrClient.java    | 4 +---
 1 file changed, 1 insertion(+), 3 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 9dcd00936e2..bcfb4aa0ff8 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
@@ -140,9 +140,7 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) {
       this.httpClient = createHttpClient(builder);
       this.closeClient = true;
     }
-    if (builder.listenerFactories != null) {
-      this.listenerFactory.addAll(builder.listenerFactories);
-    }
+    this.listenerFactory.addAll(builder.listenerFactories);
     updateDefaultMimeTypeForParser();
 
     this.httpClient.setFollowRedirects(Boolean.TRUE.equals(builder.followRedirects));

From 783f81527932ae75d04d8e917fba4271ef58ff67 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Mon, 16 Dec 2024 09:14:06 -0600
Subject: [PATCH 22/23] tidy

---
 .../src/java/org/apache/solr/core/HttpSolrClientProvider.java | 4 +---
 .../org/apache/solr/client/solrj/impl/Http2SolrClient.java    | 3 ++-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java b/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
index c209940dbbd..347607ea9f9 100644
--- a/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
+++ b/solr/core/src/java/org/apache/solr/core/HttpSolrClientProvider.java
@@ -16,7 +16,6 @@
  */
 package org.apache.solr.core;
 
-import java.util.List;
 import java.util.concurrent.TimeUnit;
 import org.apache.solr.client.solrj.impl.Http2SolrClient;
 import org.apache.solr.common.util.IOUtils;
@@ -45,8 +44,7 @@ final class HttpSolrClientProvider implements AutoCloseable {
     trackHttpSolrMetrics = new InstrumentedHttpListenerFactory(getNameStrategy(cfg));
     initializeMetrics(parentContext);
 
-    this.httpClientBuilder =
-        new Http2SolrClient.Builder().addListenerFactory(trackHttpSolrMetrics);
+    this.httpClientBuilder = new Http2SolrClient.Builder().addListenerFactory(trackHttpSolrMetrics);
 
     if (cfg != null) {
       httpClientBuilder
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 bcfb4aa0ff8..e93af72c26e 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
@@ -948,7 +948,8 @@ public Http2SolrClient.Builder addListenerFactory(HttpListenerFactory listenerFa
      * @param listenerFactories a list of HttpListenerFactory instances
      * @return This Builder
      */
-    public Http2SolrClient.Builder withListenerFactories(List listenerFactories) {
+    public Http2SolrClient.Builder withListenerFactories(
+        List listenerFactories) {
       this.listenerFactories.clear();
       this.listenerFactories.addAll(listenerFactories);
       return this;

From e8c998d1bbc7f03cd046c2cbfb9dd9bab16521e9 Mon Sep 17 00:00:00 2001
From: jdyer1 
Date: Mon, 16 Dec 2024 09:35:48 -0600
Subject: [PATCH 23/23] small change to doc comment

---
 .../org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index b64ead67652..4c0d46b13db 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -329,9 +329,8 @@ public static class Builder> {
      * Use this Builder to configure an LBHttp2SolrClient. The passed-in Solr Client Builder will be
      * used to generate an internal client per Endpoint.
      *
-     * 
Implementation Note: LBHttp2SolrClient will temporarily modify the passed-in Builder's
-     * {@link HttpSolrClientBuilderBase#baseSolrUrl} whenever it needs to generate a new Http Solr
-     * Client.
+     * 
Implementation Note: LBHttp2SolrClient will modify the passed-in Builder's {@link
+     * HttpSolrClientBuilderBase#baseSolrUrl} whenever it needs to generate a new Http Solr Client.
      *
      * @param solrClientBuilder A Builder like {@link Http2SolrClient.Builder} used to generate the
      *     internal clients