thelabdude opened a new pull request #2067: URL: https://github.com/apache/lucene-solr/pull/2067
# Description For collections with many shards (or aliases with many collections and some shards), `CloudSolrStream` will end up creating a new `HttpSolrClient` for every `SolrStream` it opens because the cache key is the full core URL, such as: `http://127.0.0.1:63460/solr/collection4_shard4_replica_n6/` In addition, `CloudSolrStream#getSlices` was calling `clusterState.getCollectionsMap()` which pre-emptively loads all `LazyCollectionRef` from ZK unnecessarily. This could cause an issue with clusters with many collections and slow down the streaming expression execution. # Solution In this PR, I've introduced a new ctor in `SolrStream` to pass the `core` which is then used to determine the URL of the node hosting the core to be queried when the stream is opened. This leads to reusing the same `HttpSolrClient` for the same node because the cache key is now `http://127.0.0.1:63460/solr/`. I chose this new ctor approach because `CloudSolrStream` is not the only consumer of SolrStream and it knows how the the list of URLs where constructed from cluster state, so it can safely make the decision about passing the core and reusing clients. We may also be able to see if `distrib=false` in the params passed to the ctor but then we'd still have to parse out the core name, which `CloudSolrStream` can do with confidence but I'm not so sure from the other paths in the code. When the request is sent to the remote core, we need to add the core name back to the path (since we stripped it from the baseUrl used by the HttpSolrClient). This happens in `SolrStream#constructParser`. This method is public and takes a SolrClient (even though SolrStream already has an HttpSolrClient created in the open method); hence, I've had to be a little more paranoid around checking the type of SolrClient `server instanceof HttpSolrClient` when determining if the core needs to be added to the path. Perhaps we can change this method's signature in Solr 9? # Tests Test still under construction ... I have a manual test that creates 10 collections with 100 shards and with this fix, a simple query streaming expression execution time is cut by at least half (500 ms vs. >1000) just for the `openStreams` operation. # Checklist Please review the following and check all that apply: - [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability. - [ ] I have created a Jira issue and added the issue ID to my pull request title. - [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended) - [ ] I have developed this patch against the `master` branch. - [ ] I have run `./gradlew check`. - [ ] I have added tests for my changes. - [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only). ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
