SOLR-17066: Switch "LB" clients away from core URLs #2283
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This acts out the first step of the 3 below that I've been considering to make the LB changes a little more reasonably. I _haven't_ actually switched any LB usage over to using base URLs or anything either. Just trying to match the existing behavior and usage, with this 'Endpoint' abstraction jammed in. 1. Create 'Endpoint' class (in LBSolrCLient, or is this generally more useful?) 2. Change LBSolrClient.ServerWrapper to be an "EndpointWrapper" 3. Change internal LBSolrClient logic to use endpoint instead of Server
The whole idea of a **DEFAULT** collection doesn't make as much sense
for LB, since round-robining between various repicas is kindof the whole
reason LB exists.
What's the most coherent interface that I could come up with:
1. Switch all LB builders and ctors to using "Endpoints"
2. Document in Javadocs that all "endpoints" for a client should be of
the same type (e.g. they all have a core, or none have a core)
- maybe enforce this in ctors?
3. Document in Javadocs that 'defaultCollection' is only used when
"base" endpoints are provided
Committing this in interim state so that I can share across computers.
The LB client builders have been switched over to using 'Endpoint', and
a lot of the old methods have been removed.
Still TODO:
- class level javadoc rework for the LB clients
- audit use of Endpoint.toString()
- look into Zombie Server alive-checking (causing
TestLBHttp2SolrClient test failure I think)
- The Big One: Switch LBSolrClient.Req over to using Endpoint
Still need to change Req usage to actually provide endpoints
|
Question Is this change worth the added complexity? I'm excited to move away from core URLs, but want to make sure the net effect is still positive given some of the complexity this pulls in. Curious what others think. If we do go this route, this change should target |
github-actions
bot
added
the
documentation
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/SOLR-17066
Description
Bootstrapping Solr clients with core URLs has several downsides: it prevents the created client from making core-agnostic requests, it prevents the client from making both v1 and v2 requests, etc. For all these reasons, SOLR-17066 has been moving our client implementations away from using these "core URLs" and towards using a "default collection" instead.
Solr's "Load balancing" client is an interesting case though. A common (maybe even the primary?) use-case of the "LB" client is to round-robin requests across a set of replicas. Each replica in a set has a unique core name, and distinguishing between them is important to ensure uniform load distribution. So users can't rely on the "default collection" feature to meet their needs.
Any attempt to move the LBClients away from "core URLs" must also retain a way for them to specify a distinct set of replicas/cores.
Solution
This PR solves this problem by creating a new LBClient-specific abstraction called an "endpoint". Endpoints are thin wrappers around a Solr base URL and optional core name, and users now provide these "Endpoint" objects when creating their LBClient, instead of passing raw String URLs.
Using a strongly typed object instead of raw String URLs allows core-based clients to still make core-agnostic requests as needed, while still supporting the "target a set of distinct replicas" use case.
Tests
Changes to
TestLBHttpSolrClientandTestLBHttp2SolrClient. New unit tests for theEndpointclass. Existing tests continue to pass.Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.