solr icon indicating copy to clipboard operation
solr copied to clipboard

SOLR-16503: Replace default USH apache client with Http2SolrClient

Open iamsanjay opened this issue 1 year ago • 1 comments

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

Replaced the default apache Http client provided by UpdateShardHandler with the Http2SolrClient provided by CoreContainer#getDefaultHttpSolrClient which will be introduced in #2689. As we are still deciding on what is the best way moving forward to set the url. Here, for now, almost everywhere a new Http2SolrClient is being re-created.

try (var solrClient =
          new Http2SolrClient.Builder(baseUrl)
              .withHttpClient(coreContainer.getDefaultHttpSolrClient())
              .build()) {
    // code goes here
}

Of course, If we decided to go with #2714 then setting up url and closing the instance would be replaced with a new abstraction.

var updateRsp = client.requestWithBaseUrl(url, (c) -> req.process(c))

Checklist

Please review the following and check all that apply:

  • [x] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • [x] I have created a Jira issue and added the issue ID to my pull request title.
  • [x] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • [x] I have developed this patch against the main branch.
  • [x] I have run ./gradlew check.
  • [ ] I have added tests for my changes.
  • [ ] I have added documentation for the Reference Guide

iamsanjay avatar Oct 04 '24 13:10 iamsanjay

SplitShardWithNodeRoleTest.testSolrClusterWithNodeRoleWithPull failed! Even in the past this one failed. If I look at the test, It seem simple, However the complexity that split operation involves is rather too much. This is how it runs.

  1. Create a collection with one shard containing one NRT and one PULL replica.
  2. waitForState to achieve above configuration
  3. Index 10 documents
  4. Commit collection
  5. Perform split operation
  6. waitForState for 2 active sub-shards (about 45 seconds, and this is where it failed.)

I am not sure If increasing Timeout will help us. But we can try it! In the logs, one can see that just before the assertion failed, the IndexFetcher was running. May be the PULL replicas were downloading the index, and the time finished and the sub-shard never really recovered.

Question: Do both NRT and PULL replicas need to be active for sub-shards to be active?

Note :- I found a JIRA ticked related to this one https://issues.apache.org/jira/browse/SOLR-16753.

iamsanjay avatar Oct 05 '24 05:10 iamsanjay

@dsmiley I think it doesn't even matter If we pass only zkAddress, or the combination of both zkHost and zkChroot.

void testZkClientClusterStateProviderCtor() throws IOException {
        try (
        var stateProvider = new ZkClientClusterStateProvider(Collections.singletonList("localhost:2181"), "/solr");
        var stateProvider1 = new ZkClientClusterStateProvider(Collections.singletonList("localhost:2181/solr"), null);
        var stateProvider2 = new ZkClientClusterStateProvider("localhost:2181/solr");){
            assertEquals(stateProvider.getZkHost(), stateProvider2.getZkHost());
            assertEquals(stateProvider.getZkHost(), stateProvider1.getZkHost());
        }
    }

iamsanjay avatar Nov 08 '24 04:11 iamsanjay

I plan to merge this PR by the end of the week if there are no further comments.

iamsanjay avatar Nov 19 '24 17:11 iamsanjay

@iamsanjay is anything stopping this from getting merged? Tests still pass? Need a CHANGES.txt saying something like: "Most remaining usages of Apache HttpClient in Solr switched to Jetty HttpClient (HTTP 2)."

dsmiley avatar Jan 08 '25 03:01 dsmiley

Thank you for this wonderful effort.. Getting through this migration gets us much closer to Solr 10!

epugh avatar Jan 13 '25 13:01 epugh