Take out the possibility of having a solr.properties file

Details

Type: Improvement

Status:Resolved

Priority: Blocker

Resolution:
Fixed

Affects Version/s:4.3, 6.0

Fix Version/s:
None

Component/s:
None

Labels:

None

Description

We seem to have re-thought whether deprecating Solr.xml is The Right Thing To Do or not, the consensus seems to be that we should keep solr.xml, not be able to specify solr.properties but add an attribute to the <cores> tag in solr.xml, tentatively called autoDiscover=true|false (assume false for 4.x, true for 5.0?)

Activity

I actually think the current format is not right - almost everything is an attribute in <cores> tag. That seems less than ideal...I wonder if we could just design the solr.xml from the ground up - as if it was a properties file. I have not donated any sweat to this yet, so just an idea, but I think we can do better than the current solr.xml - <cores>.

Mark Miller
added a comment - 19/Mar/13 16:45 I actually think the current format is not right - almost everything is an attribute in <cores> tag. That seems less than ideal...I wonder if we could just design the solr.xml from the ground up - as if it was a properties file. I have not donated any sweat to this yet, so just an idea, but I think we can do better than the current solr.xml - <cores>.

That reminds me that I did just that in SOLR-3613 - refactored solr.xml into a better structure for the cloud params. But it was never completed due to arguments about not wanting to namespace cloud java-options for the 4.0.0 release due to back-compat with beta.

I just uploaded my work to that patch, it won't apply to today's trunk, but have a look at the proposed solr.xml format. The patch tries to prefix all params such as zkHost with a solr. prefix, but still keep the short form as a valid alias. But then I got somewhat lost trying to also support webapp-name. prefixing automatically...

Jan Høydahl
added a comment - 19/Mar/13 22:25 That reminds me that I did just that in SOLR-3613 - refactored solr.xml into a better structure for the cloud params. But it was never completed due to arguments about not wanting to namespace cloud java-options for the 4.0.0 release due to back-compat with beta.
I just uploaded my work to that patch, it won't apply to today's trunk, but have a look at the proposed solr.xml format. The patch tries to prefix all params such as zkHost with a solr. prefix, but still keep the short form as a valid alias. But then I got somewhat lost trying to also support webapp-name. prefixing automatically...

I had a long plane ride so I cut solr.properties out in the attached patch. All tests pass, although I haven't yet really gone over it yet, consider this preliminary.

Known tasks not yet done:
1> add tests for persisting the new property.
2> there are a couple of JIRAs (SOLR-4631 and SOLR-4632) that I have yet to incorporate.

There is a new attribute for <cores>, autoDiscoverCores=["true"|"false"], defaults presently to "false". If you'd like a different name, speak now...

I left the interface definition in place, currently only ConfigSolrXml implements it (renamed from ConfigSolrXmlBackCompat). My theory is that since it's there already, leaving it in has the advantage of allowing easier implementation of a pluggable configuration provider of some kind should the desire arise. One possibility (although I know nothing about whether it's desirable) would be provide all this info from ZK directly, maybe based on a sysprop?

I also renamed several files, removed the supporting config fies for solr.properties etc., so be aware that some things have moved around...

I plan to commit this in the next couple of days to both trunk and 4x on the theory that the faster I take solr.properties out of code anyone can get to, the less they'll be inconvenienced by flip-flopping.

Changing this didn't require any changes to the tricky bits around coordinating core load/unload/reload code, so it's not a radical change. Not to say I necessarily did it right, but it's not nearly as scary as the rest of the changes for SOLR-4196.

Erick Erickson
added a comment - 24/Mar/13 18:10 I had a long plane ride so I cut solr.properties out in the attached patch. All tests pass, although I haven't yet really gone over it yet, consider this preliminary.
Known tasks not yet done:
1> add tests for persisting the new property.
2> there are a couple of JIRAs ( SOLR-4631 and SOLR-4632 ) that I have yet to incorporate.
There is a new attribute for <cores>, autoDiscoverCores= ["true"|"false"] , defaults presently to "false". If you'd like a different name, speak now...
I left the interface definition in place, currently only ConfigSolrXml implements it (renamed from ConfigSolrXmlBackCompat). My theory is that since it's there already, leaving it in has the advantage of allowing easier implementation of a pluggable configuration provider of some kind should the desire arise. One possibility (although I know nothing about whether it's desirable) would be provide all this info from ZK directly, maybe based on a sysprop?
I also renamed several files, removed the supporting config fies for solr.properties etc., so be aware that some things have moved around...
I plan to commit this in the next couple of days to both trunk and 4x on the theory that the faster I take solr.properties out of code anyone can get to, the less they'll be inconvenienced by flip-flopping.
Changing this didn't require any changes to the tricky bits around coordinating core load/unload/reload code, so it's not a radical change. Not to say I necessarily did it right, but it's not nearly as scary as the rest of the changes for SOLR-4196 .
All comments welcome....

I'm not a fan of this - I don't believe false should be an option in the future, so I don't like adding this option. Rather, we should just determine if it's an old solr.xml or a new one (add a version, change the format, change the file name, something - I'll have to dig in first to know what I would vote for).

Given that, I also still think we should change the solr.xml format to make more sense. I'll be pitching in on this soon (I've said that for a while, but I keep getting caught up in other things - I will be doing this before 4.3 though.)

Mark Miller
added a comment - 28/Mar/13 16:54 autoDiscoverCores= ["true"|"false"]
I'm not a fan of this - I don't believe false should be an option in the future, so I don't like adding this option. Rather, we should just determine if it's an old solr.xml or a new one (add a version, change the format, change the file name, something - I'll have to dig in first to know what I would vote for).
Given that, I also still think we should change the solr.xml format to make more sense. I'll be pitching in on this soon (I've said that for a while, but I keep getting caught up in other things - I will be doing this before 4.3 though.)

The more the merrier. I've done most of the legwork in SOLR-4615 (removed the whole solr.properties thing, un-deprecated solr.xml, fixed up the tests, etc) so you'll probably want to either wait until I check that in (Real Soon Now) or work off that patch/JIRA.

Erick Erickson
added a comment - 28/Mar/13 20:47 The more the merrier. I've done most of the legwork in SOLR-4615 (removed the whole solr.properties thing, un-deprecated solr.xml, fixed up the tests, etc) so you'll probably want to either wait until I check that in (Real Soon Now) or work off that patch/JIRA.

I messed up a little and didn't get the trunk patch (committed then tried to cut the final patch). So this patch is for 4x after merging from trunk.

trunk r: 1463316

I'm going to make sure this works with 4x, check it in and close this JIRA. Before I do that I'll open another blocker for straightening out solr.xml (however we're going to do that). At least this way people won't get be going down the solr.properties path and pulling the attribute out of <cores> shouldn't be hard.

Erick Erickson
added a comment - 01/Apr/13 22:17 I messed up a little and didn't get the trunk patch (committed then tried to cut the final patch). So this patch is for 4x after merging from trunk.
trunk r: 1463316
I'm going to make sure this works with 4x, check it in and close this JIRA. Before I do that I'll open another blocker for straightening out solr.xml (however we're going to do that). At least this way people won't get be going down the solr.properties path and pulling the attribute out of <cores> shouldn't be hard.