SOLR-16959: Make CoresLocator class configurable #1891
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
pvcnt
force-pushed
the
cores-locator-class
branch
from
2d32cd4 to
fe87566
Compare
murblanc
reviewed
Sep 4, 2023
There was a problem hiding this comment.
Thanks for doing this Vincent. Looks good to me.
Full disclaimer: I work with Vincent in the same company and that change, in addition to aligning CoresLocator with configuration flexibility available for other classes will also eventually simplify the integration of new types of cores that have no local presence (index Blob/S3 storage Salesforce plans to eventually contribute, see email).
solr/solr-ref-guide/modules/configuration-guide/pages/configuring-solr-xml.adoc
Outdated
Show resolved
Hide resolved
added 2 commits
September 4, 2023 17:28
dsmiley
changed the title
dsmiley
approved these changes
Sep 5, 2023
pvcnt
force-pushed
the
cores-locator-class
branch
from
63c6625 to
83455b4
Compare
added 2 commits
September 7, 2023 16:16
|
Can you please add a CHANGES.txt note in the 9.4 section, probably in the "Improvements" section? A brief sentence would do like "Make the internal CoresLocator implementation configurable in solr.xml" |
|
@dsmiley That's done. |
# Conflicts: # solr/CHANGES.txt
dsmiley
pushed a commit
that referenced
this pull request
Sep 27, 2023
Co-authored-by: Vincent Primault <[email protected]> (cherry picked from commit 708ab9b)
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-16959
Description
I would like to be able to hook into the process of discovering and persisting core descriptors. This allows to work with core descriptors not stored locally, or replicated elsewhere.
Solution
For this purpose, I propose to make the CoresLocator instance configurable, pretty much like other services (e.g., ConfigSetService, some handlers) are already configurable. Attached PR creates a new "coresLocator" parameter in solr.xml in order to achieve this. Because it is created outside of the CoreContainer (it is needed to load a CoreContainer), constructor of a custom CoresLocator receives a NodeConfig object (and not a CoreContainer) that it can use to lookup other parameters, if required.
Tests
I added new unit tests to validate the correct instantiation of a CoresLocator.
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.