Details
-
Improvement
-
Status: Closed
-
Major
-
Resolution: Fixed
-
None
-
None
Description
Given that we now handle multiple cores via the solr.xml and the discussion around <indexDefaults> and <mainIndex> at http://www.lucidimagination.com/search/p:solr?q=mainIndex+vs.+indexDefaults
We should deprecate old <indexDefaults> and <mainIndex> sections and only use a new <indexConfig> section.
3.6: Deprecation warning if old section used
4.0: If LuceneMatchVersion before LUCENE_40 then warn (so old configs will work), else fail fast
Attachments
Attachments
- SOLR-1052-3x.patch
- 11 kB
- Jan Høydahl
- SOLR-1052-3x.patch
- 15 kB
- Jan Høydahl
- SOLR-1052-3x.patch
- 34 kB
- Jan Høydahl
- SOLR-1052-3x.patch
- 35 kB
- Jan Høydahl
- SOLR-1052-3x-fix-tests.patch
- 6 kB
- Jan Høydahl
- SOLR-1052.patch
- 132 kB
- Jan Høydahl
Issue Links
- is related to
-
SOLR-7815 Remove LuceneQueryOptimizer
- Closed
- relates to
-
SOLR-10572 (from 7.0.0 onwards) remove three no-longer-supported warnings in SolrIndexConfig
- Resolved
-
SOLR-3093 Decide destiny of LuceneQueryOptimizer (solrconfig.xml: <boolTofilterOptimizer>)
- Closed
-
SOLR-14267 complete <query><HashDocSet> solrconfig.xml removal
- Closed
Activity
Suggestion:
In 3.6, deprecate indexDefaults, removing from example solrconfig, announcing its death in CHANGES.TXT
In 4.0, remove every trace of indexDefaults
Why not deprecate in both code and configurations in 3.6? That way it doesn't just disappear from users in 3.6.
Attempt on 3.6 deprecation. I "merged" the configs into <mainIndex>, added missing comments and commented out some settings which were identical to the defaults anyway. I also changed the default for ramBufferSizeMB from 16 to 32 in java code, to be able to comment out this setting in XML as well.
If <indexDefaults> is found in solrconfig.xml a deprecation warning is printed.
I also removed some attempt to parse an old Solr1.x syntax for <mergeScheduler>, <mergePolicy> and <luceneAutoCommit>.
What do you think?
Why not leave the <indexDefaults> in the config and say they're deprecated (as I said before)?
Disagree. I see three types of people we have to support:
A) Someone simply upgrades the WAR and continue use their old configs. They will get a deprecation warning in the logs but else everyting is as before.
B) Previous Solr users who wants to upgrade config properly. They will clearly see the new preferred config, and if they use it, they are ready for the deprecation without surprise. Or they may copy/paste own customizations with indexDefaults if they choose.
C) Totally new to Solr. Version 3.6 is their first download and they get it all right - with less confusion - out of the door, no surprises in 4.0.
This is how config deprecations should work in my opinion. No need to advertise to new users the use of a syntax that we want to go away. It would be more confusing for the C) people to see deprecation warnings being printed OOTB from their brand new search engine without knowing how to fix it..
What lacks in the patch is fixing all the other variants of solrconfig.xml all around the place..
Here are the other files I believe need the change as well. Many of these are sadly lagging behind, not being updated for the last few versions, the most important to fix being the "example/example-DIH" ones:
lap:SOLR-1052-3x janhoy$ find . -name *solrconfig* | grep -v "\.svn" | xargs grep -n "<indexDefaults>" ./solr/client/ruby/solr-ruby/solr/conf/solrconfig.xml:34: <indexDefaults> ./solr/client/ruby/solr-ruby/test/conf/solrconfig.xml:36: <indexDefaults> ./solr/contrib/clustering/src/test-files/clustering/solr/conf/solrconfig.xml:36: <indexDefaults> ./solr/contrib/dataimporthandler/src/test-files/dih/solr/conf/contentstream-solrconfig.xml:36: <indexDefaults> ./solr/contrib/dataimporthandler/src/test-files/dih/solr/conf/dataimport-nodatasource-solrconfig.xml:36: <indexDefaults> ./solr/contrib/dataimporthandler/src/test-files/dih/solr/conf/dataimport-solrconfig.xml:36: <indexDefaults> ./solr/contrib/dataimporthandler-extras/src/test-files/dihextras/solr/conf/dataimport-solrconfig.xml:36: <indexDefaults> ./solr/contrib/extraction/src/test-files/extraction/solr/conf/solrconfig.xml:34: <indexDefaults> ./solr/contrib/uima/src/test-files/uima/solr/conf/solrconfig.xml:72: WARNING: this <indexDefaults> section only provides defaults for ./solr/contrib/uima/src/test-files/uima/solr/conf/solrconfig.xml:76: <indexDefaults> ./solr/contrib/velocity/src/test-files/velocity/solr/conf/solrconfig.xml:35: <indexDefaults> ./solr/core/src/test-files/solr/conf/bad_solrconfig.xml:27: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-delpolicy1.xml:40: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-delpolicy2.xml:40: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-duh-optimize.xml:38: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-elevate.xml:40: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-enableplugin.xml:35: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-functionquery.xml:40: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-highlight.xml:38: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-legacy.xml:54: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-master.xml:29: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-master1.xml:29: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-master2.xml:29: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-mergepolicy.xml:28: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-nocache.xml:40: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-propinject-indexdefault.xml:54: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-propinject.xml:53: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-repeater.xml:30: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-slave.xml:29: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-slave1.xml:29: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-solcoreproperties.xml:35: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-SOLR-749.xml:39: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-spellchecker.xml:27: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-termindex.xml:45: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig-xinclude.xml:40: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig.xml:55: <indexDefaults> ./solr/core/src/test-files/solr/conf/solrconfig_perf.xml:41: <indexDefaults> ./solr/example/example-DIH/solr/db/conf/solrconfig.xml:36: <indexDefaults> ./solr/example/example-DIH/solr/mail/conf/solrconfig.xml:35: <indexDefaults> ./solr/example/example-DIH/solr/rss/conf/solrconfig.xml:36: <indexDefaults> ./solr/example/example-DIH/solr/solr/conf/solrconfig.xml:36: <indexDefaults> ./solr/example/example-DIH/solr/tika/conf/solrconfig.xml:35: <indexDefaults> ./solr/example/solr/conf/solrconfig.xml:128: Note: As of Solr 3.6, the <indexDefaults> section is deprecated and ./solr/solrj/src/test-files/solrj/solr/conf/solrconfig-slave1.xml:29: <indexDefaults>
This is how config deprecations should work in my opinion. No need to advertise to new users the use of a syntax that we want to go away. It would be more confusing for the C) people to see deprecation warnings being printed OOTB from their brand new search engine without knowing how to fix it
+1
Patch looks good to me. my one suggestion, since there seems to be consensus that solr should complain louder when there are config errors, is that instead of removing the existing "warn" calls on those already deprecated Solr 1.x legacy conf syntax, why not leave in those checks but the "warn(...)" calls with "throw new SolrException(...)" ?
Validation is good to have
New patch:
- Added assertNotPresent() method in SolrIndexConfig and asserts for mergeScheduler, mergePolicy and luceneAutoCommit
- Added upgrade entry to CHANGES
I'm thinking that <indexConfig> is a better name than <mainIndex> once we remove the concept of multiple indexes.
So in next version of patch I'll deprecate both indexDefaults and mainIndex and let indexConfig be the new section name.
I'm also changing some defaults so that the effect of not specifying this section at all will give you the settings from example solrconfig.xml. These are:
- useCompoundFile = false (if luceneMatchVersion >= LUCENE_36, else true)
- ramBufferSizeMB = 32
- lockType = native (if luceneMatchVersion >= LUCENE_36, else use simple)
Updated patch with new defaults as stated above. It aborts if both <mainIndex> and <indexConfig> are specified and logs warning if only deprecated ones are found.
Added a few tests for default checking.
Remaining is to clean up other solrconfig.xml files and some tests explicitly testing on the old sections, and to update WIKI.
Also, this patch includes SOLR-3093. Pending answer from MarkM about the disabling of boolToFilterOptimizer...
Any more comments on this before we move on? How do you feel about leaving <indexConfig> and <mainIndex> sections as is in all the test solrconfigs, but update the DIH example configs? We can tackle upgrading all the test files in 4.x in another issue.
Thanks for tackling this Jan; this needed to happen a looooong time ago!
If you said that your patch will abort if mainIndex & indexConfig are present, then you will be forced to upgrade the test configs; no? Any way, if your opinion is that it's easier to split the work into a separate issue then I think that's fine.
In 3.6 (which the patch is for) it will only print a warning, so old configs will work.
Have not yet started 4.x patch, still need to decide what should happen with old configs in 4.x. Perhaps warn if supplied config < LUCENE_40 but if config version is >= LUCENE_40 we fail?
Updated patch for 3.x.
- Re-entered the commented-out lines for <boolTofilterOptimizer>, see
SOLR-3093 - Checks for old syntax of mergeScheduler, mergePolicy, luceneAutoCommit now WARNs for pre-3.6 configs and fails for 3.6 and above
Adding back 3.6, as the issue is committed on 3.x but not yet on trunk (coming)
Upgraded some defunct tests to the new <indexConfig> section. Again allowing null as "prefix", defaulting to "indexConfig" section.
The TRUNK patch is coming up, with these highlights:
- For luceneMatchVersion <= LUCENE_36 we parse <indexDefaults> and <mainIndex> and print warning
This allows people to do drop-in upgrade keeping their old configs - For luceneMatchVersion >= LUCENE_40 we FAIL fast if we find <indexDefaults> or <mainIndex>
- Converted all tests relying on solrconfig-*.xml files to new <indexConfig> format
- Removed a lot of redundant old <indexDefaults> and <mainIndex> config from test files
Questions:
- All tests which purpose is not to test the indexConfig stuff now uses 100% default settings
- This means among other things that lockType=native and unlockOnStartup=false. Are there known issues with such settings during tests?
Here's the patch for trunk, based on a merge from 3.x. Happy to get some feedback as it changes a lot of files. Good that the patch removes more lines than it introduces
The patch uses <lockType>single</lockType> for all tests, which should be suitable for all platforms.
Tests pass for me, would love to have another pair of eyes on it too.
Jan: looks like you are just waiting for feedback for the port-forward to trunk here?
Nothing remains for 3.x?
That's right. 3.x should be good to go now. Would very much like to get it committed to trunk as well, but I'd rather wait until someone gives it a run first.
That's right. 3.x should be good to go now. Would very much like to get it committed to trunk as well, but I'd rather wait until someone gives it a run first.
do you mean you want to wait unit 3.6 is released and its not breaking things? this makes no sense to me, we should not commit into 3.x and don't do it in trunk. this is dangerous, we should try to commit to trunk and port backwards instead. If you have time please move forward we can fix stuff on trunk easily right now.
I'm of course not suggesting to use a 3.x release as test before committing to trunk The rule is to do trunk first. In this case since this is a deprecation patch with different behaviour in 3.6 and 4.0, we fleshed out the 3.x part first this time. All trunk tests pass for me - so I will commit to trunk now to get any uncaught issues out of the way before easter..
Sounds like a good improvement for 4.0!