SOLR-18188: Remove org.apache.solr.client.solrj.apache; update tests to use Jetty equivalents by dsmiley · Pull Request #4451 · apache/solr

dsmiley · 2026-05-19T16:05:11Z

https://issues.apache.org/jira/browse/SOLR-18188

This branch/PR originated on the HttpSolrClient refactor / re-addition, whcih is related. That aspect has since merged; brought this branch up to date. I'll want to take chunks of work here and commit separately (other PRs) so we don't lose context on why some tests or test infra changed in non-obvious ways.

As of this writing: tests pass, even with SSL. "nightly" tests have failures.

Not going to be a good use of time to review this massive thing so I suggest not reviewing except for the things I call out in a self-review.

WILL NOT COMPILE Not widely updating anything yet; just touch this source file.

WILL NOT COMPILE Enhanced our clients to use it.

# Conflicts: # solr/core/src/test/org/apache/solr/cloud/HttpPartitionTest.java # solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java # solr/core/src/test/org/apache/solr/cloud/LeaderVoteWaitTimeoutTest.java # solr/core/src/test/org/apache/solr/cloud/RecoveryZkTestWithAuth.java # solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java # solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java # solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java # solr/core/src/test/org/apache/solr/filestore/TestDistribFileStore.java # solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java # solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerFailureTest.java # solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerTest.java # solr/core/src/test/org/apache/solr/handler/component/DistributedQueryElevationComponentTest.java # solr/core/src/test/org/apache/solr/pkg/TestPackages.java # solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java # solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBase.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleBinaryTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleXMLTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrExceptionTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrSchemalessExampleTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/TestSolrJErrorHandling.java # solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleJettyTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/response/InputStreamResponseParserTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/response/TestSuggesterResponse.java # solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java # solr/test-framework/src/java/org/apache/solr/client/solrj/apache/ConcurrentUpdateSolrClient.java # solr/test-framework/src/java/org/apache/solr/client/solrj/apache/HttpApacheSolrClient.java # solr/test-framework/src/java/org/apache/solr/client/solrj/apache/SolrClientBuilder.java # solr/test-framework/src/java/org/apache/solr/cloud/AbstractBasicDistributedZkTestBase.java # solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java # solr/test-framework/src/java/org/apache/solr/cloud/AbstractRecoveryZkTestBase.java # solr/test-framework/src/java/org/apache/solr/cloud/AbstractSyncSliceTestBase.java # solr/test-framework/src/java/org/apache/solr/cloud/AbstractUnloadDistributedZkTestBase.java # solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java # solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java # solr/test-framework/src/test/org/apache/solr/client/solrj/apache/HttpSolrClientBuilderTest.java

Rename HttpSolrClientBuilderBase.java to HttpSolrClient.BuilderBase (inner class)

# Conflicts: # solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-tika.adoc

…ient # Conflicts: # solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerBackup.java # solr/solrj/src/test/org/apache/solr/client/solrj/TestBatchUpdate.java # solr/solrj/src/test/org/apache/solr/client/solrj/TestSolrJErrorHandling.java # solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleJettyTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientBadInputTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java # solr/solrj/src/test/org/apache/solr/client/solrj/response/InputStreamResponseParserTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/response/TestSuggesterResponse.java # solr/test-framework/src/java/org/apache/solr/SolrJettyTestBase.java # solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java

# Conflicts: # solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java # solr/core/src/test/org/apache/solr/filestore/TestDistribFileStore.java # solr/core/src/test/org/apache/solr/handler/admin/api/ClusterPropsAPITest.java # solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java # solr/core/src/test/org/apache/solr/pkg/TestPackages.java # solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java # solr/core/src/test/org/apache/solr/servlet/CacheHeaderTestBase.java # solr/core/src/test/org/apache/solr/servlet/SecurityHeadersTest.java # solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java # solr/solrj-streaming/gradle.lockfile # solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleBinaryTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleCborTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleXMLTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrExceptionTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/SolrSchemalessExampleTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/TestLBHttpSolrClient.java # solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleJettyTest.java # solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java # solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java # solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java # solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java # solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java # solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java # solr/test-framework/src/test/org/apache/solr/client/solrj/apache/ConnectionReuseTest.java # solr/test-framework/src/test/org/apache/solr/client/solrj/apache/HttpSolrClientBuilderTest.java # solr/test-framework/src/test/org/apache/solr/client/solrj/apache/HttpSolrClientConPoolTest.java # solr/test-framework/src/test/org/apache/solr/client/solrj/apache/LBHttpSolrClientTest.java

# Conflicts: # changelog/unreleased/SOLR-17968-HttpSolrClient.yml # solr/solrj/src/java/org/apache/solr/client/solrj/impl/CollectionScopedSolrClient.java # solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java # solr/solrj/src/test/org/apache/solr/client/solrj/impl/ClusterStateProviderTest.java # solr/test-framework/src/java/org/apache/solr/client/solrj/apache/HttpApacheSolrClient.java

The race condition was that chaosMonkey.expireSession() starts a background coreZkRegister thread, but waitForRecoveriesToFinish() returned immediately because the node hadn't re-joined live_nodes yet (non-live replicas in DOWN state are ignored). The fix increases blockUntilConnected from 50ms to 30s and replaces waitForRecoveriesToFinish with a waitForState that explicitly waits for the expired node to be back in live_nodes and all live replicas to be ACTIVE. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

And add other lockfiles

dsmiley · 2026-05-20T01:47:50Z

-
-/** Apache HttpClient based {@link org.apache.solr.client.solrj.SolrClient} implementations */
-@Deprecated
-package org.apache.solr.client.solrj.apache;


Removing this whole package :-) bye bye HttpSolrClient, CloudLegacySolrClient, etc.

dsmiley · 2026-05-20T02:01:06Z

-   * A variant of {@link org.apache.solr.client.solrj.impl.CloudSolrClient.Builder} that will
-   * randomize some internal settings.
-   */
-  public static class RandomizingCloudHttp2SolrClientBuilder extends CloudSolrClient.Builder {


basically I renamed the http2 one to RandomizingCloudSolrClientBuilder

dsmiley · 2026-05-20T02:01:36Z


  protected volatile JettySolrRunner controlJetty;
-  protected final List<SolrClient> clients = Collections.synchronizedList(new ArrayList<>());
+  protected final List<HttpSolrClient> clients = Collections.synchronizedList(new ArrayList<>());


using this more specifc type allows getBaseURL

dsmiley · 2026-05-20T02:03:44Z

-              .setCreateNodeSet(controlJetty.getNodeName())
-              .process(client)
-              .getStatus());
+      RetryUtil.retryOnException(


I don't love the retry but it's needed sometimes

and undo change to HttpSolrClient.Builder.withHttpClient inheriting the Url (questionable)

… It fires off an async shard split (processAsync("2000", ...)), asserts it's running, then the test ends — without waiting for the split to complete. The try(SolrClient) block closes the client, and then the test class tears down the cluster while the split is still running in the background. That split creates ZkShardTerms objects (via SplitOp.execute → ZkController.getShardTerms) that never get properly closed because the cluster shuts down mid-operation. The fix mirrors what the other test methods (testParallelCollectionAPICalls, testTaskExclusivity, testDeduplicationOfSubmittedTasks) already do — they all call getRequestStateAfterCompletion before exiting. testLongAndShortRunningParallelApiCalls is the only one that skips this step.

dsmiley

This is nearly mergeable.
I actually reviewed all these tests again.... albeit skimmed.
Some tests create new clients, switched to Jetty, but most tests now re-use existing managed (Jetty) SolrClients. That might help with overall test efficiency; it can only help.
In a number of cases, clients were created with specific timeout settings. Strictly speaking, replicating that would mean not using existing managed clients (created with whatever timeouts). I'm skeptical all these specific timeouts matter; I'd rather the managed ones use some recommended defaults for tests, and then only by exception (e.g. finding certain tests fail) do we go out of our way to customize it.

dsmiley · 2026-06-06T17:17:46Z

    // avoid bad connection races due to shutdown
-    final var apacheHttpClient = ((CloudLegacySolrClient) cluster.getSolrClient()).getHttpClient();
-    apacheHttpClient.getConnectionManager().closeExpiredConnections();
-    apacheHttpClient.getConnectionManager().closeIdleConnections(1, TimeUnit.MILLISECONDS);
+    // var apacheHttpClient = ((CloudLegacySolrClient) cluster.getSolrClient()).getHttpClient();
+    // apacheHttpClient.getConnectionManager().closeExpiredConnections();
+    // apacheHttpClient.getConnectionManager().closeIdleConnections(1, TimeUnit.MILLISECONDS);


leaving commented code here since if this becomes flaky, I want an investigator to see how this used to be.

dsmiley · 2026-06-06T17:23:28Z

Changes here were to ensure that the "old nodes" run with Http1 -- as I assume that is what's meant by "old nodes". Not sure why this test was failing, causing me to do this.
Ideally this would have been in the misc-test-fixes PR.

dsmiley · 2026-06-06T17:31:54Z

-  protected final List<SolrClient> coreClients = Collections.synchronizedList(new ArrayList<>());
+  protected final List<HttpSolrClient> coreClients =
+      Collections.synchronizedList(new ArrayList<>());


This has wide ripple effects on other tests... but positve because finally we can see this is an HttpSolrClient and thus stop casting, e.g. just to get the base URL

dsmiley · 2026-06-06T17:34:48Z

Substantial change... some is simply removing test support specific to Apache HttpClient test infra. But the more interesting part is that this infra was using utilities in Apache HttpClient's libraries (see those import statements) that are now gone so we must do manually what it was doing. So some increase in code for that.

@markrmiller way way back when, you brought this into existence, so perhaps you may want to review it. I confess I let Claude do the work here and I barely spot-checked it.

dsmiley · 2026-06-06T17:37:53Z

ideally this was have been done in the "misc-test-fixes" pr, since merged. ah well.

dsmiley · 2026-06-06T17:40:44Z

      indexr("id", docId + 1, t1, "slip this doc in");

-      waitForRecoveriesToFinish(false);
+      // Wait for the session-expired node to fully re-register its replica as ACTIVE.


one of the slightly more interesting changes in this massive PR. Probably should have been in the misc-test-fixes PR.

dsmiley · 2026-06-06T17:46:52Z

This is blocked on a minor PR for enabling our clients to send the query type header, used for rate limiting. solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java fails on this branch now since the test switched away from our legacy client that used that header. Other than that, this is mergeable. I plan to merge this Tuesday. It's truly ready for any review if anyone wants to take a look here. Given the size of this PR, I recommend simply spot-checking a few things, and certainly please look where I self-reviewed with comments.

…HttpClient

dsmiley and others added 29 commits October 31, 2025 22:28

SOLR-17968: Rename HttpSolrClient to HttpApacheSolrClient

755c6d1

WILL NOT COMPILE Not widely updating anything yet; just touch this source file.

SOLR-17968: HttpSolrClient returns in org.apache.solr.client.solrj

f35170a

WILL NOT COMPILE Enhanced our clients to use it.

Reference HttpApacheSolrClient where appropriate.

7552398

ref guide sample code

1295041

deprecations

93ae2a4

Don't override getBaseURL

829e69c

Change some HttpSolrClient references to HttpApacheSolrClient

22b070a

changelog

561a6d6

HttpApacheSolrClient no longer implements HttpClient

2a435c0

Rename HttpSolrClientBase to HttpSolrClient.

096a5e5

Rename HttpSolrClientBuilderBase.java to HttpSolrClient.BuilderBase (inner class)

Add HttpSolrClient.builder(baseUrl) static method

3705b87

Merge branch 'main' into SOLR-17968-HttpSolrClient

1e2c987

# Conflicts: # solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-tika.adoc

fix from merge

43589c2

solrj-streaming need not depend on jetty

6b3f53e

Tests...

71472f5

withHttpClient takes baseSolrUrl

00f488a

Tests, switch

d7c767a

Ignore TestRequestRateLimiter for now

aba0816

Stop using HttpClientUtil

68cf3c1

SSLTestConfig no longer depends on Apache httpcomponents

c342722

New build tests.ssl option to force SSL when supported

ba8acfa

Remove package org.apache.solr.client.solrj.apache

47943d6

Drop dependencies

1c46d68

changelog

b14e3ff

github-actions Bot added the documentation Improvements or additions to documentation label May 19, 2026

github-actions Bot added cat:index cat:cli cat:api cat:schema module:clustering cat:security module:langid module:scripting labels May 19, 2026

dsmiley added 3 commits May 19, 2026 21:40

revert httpclient/httpcore LICENSE & NOTICE

b76222f

restore solr/ui/gradle.lockfile

529b0a3

And add other lockfiles

changelog cleanup

1af5b60

github-actions Bot removed the admin-ui label May 20, 2026

dsmiley commented May 20, 2026

View reviewed changes

dsmiley added 3 commits May 24, 2026 01:37

Add SolrJettyTestRule.newSolrClientBuilder

3325b06

and undo change to HttpSolrClient.Builder.withHttpClient inheriting the Url (questionable)

Merge branch 'main' into SOLR-18188-testsStopUsingApacheHttpClient

e711e53

dsmiley mentioned this pull request Jun 3, 2026

SOLR-18188: Misc test fixes #4496

Merged

dsmiley added 2 commits June 5, 2026 23:58

Merge branch 'main' into SOLR-18188-testsStopUsingApacheHttpClient

410669c

solrj-streaming/build.gradle: testImplementation solrj-jetty

b2c387d

dsmiley commented Jun 6, 2026

View reviewed changes

revert a few things

76760c5

github-actions Bot removed the jetty-server label Jun 6, 2026

dsmiley marked this pull request as ready for review June 6, 2026 17:42

Merge branch 'main' into fork/dsmiley/SOLR-18188-testsStopUsingApache…

f63539b

…HttpClient

dsmiley mentioned this pull request Jun 9, 2026

SOLR-18243: Refactor solrj-streaming to reuse shared CloudHttp2SolrClient engine #4510

Open

@AwaitsFix TestRequestRateLimiter

6455fd2

github-actions Bot added the jetty-server label Jun 10, 2026

dsmiley merged commit 31405c5 into apache:main Jun 10, 2026
6 of 7 checks passed

dsmiley deleted the SOLR-18188-testsStopUsingApacheHttpClient branch June 10, 2026 04:34

Conversation

dsmiley commented May 19, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsmiley commented Jun 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant