[jira] [Commented] (SOLR-8668) Remove support for (in favour of )

["Hoss Man (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-8668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16023854#comment-16023854 ] 

Hoss Man commented on SOLR-8668:
--------------------------------

Christine: I just skimmed {{git diff master...jira/solr-8668}} ...

Most everything looked sane to me.  In a few places I got a little confused as to why some tests/configs were removed instead of just replacing the existing config with a MergePolicyFactory config -- but I'm guessing that's because an equivilent MergePolicyFactory based test already exists?

The one thing that jumped out at me is {{SolrIndexConfig.effectiveUseCompoundFileSetting}}.  Now that {{SolrIndexConfig.fixUseCFMergePolicyInitArg}} is removed, I think we can/should simplify/remove the (mutable) {{effectiveUseCompoundFileSetting}} and assocaited methods/hacks into a {{finel boolean useCompoundFile}} just like every other SOlrIndexConfig setting. right?

bq. Hoping to commit this later this month ...

+1

{quote}
bq. do we need to throw an exception if anyone still configures <mergePolicy> or is it fair to silently ignore it
I think a warning might be a fair compromise, instead of an exception or silent ignoring.
{quote}

bq. part 2: small incremental change (replace throwing-exception with log-warning)


Whoa, whoa ... what? ... no, no no! ... -1!

{{<mergePolicy>}} was deprecated (and stopped appearing in example configs) in 5.5! If someone has been upgrading Solr since then and never bothered to notice the {{WARN}} in their log file informing them that they should switch to {{<mergePolicyFactory>}}, there is no sane reason to assume they will notice a {{WARN}} in their 7.0 logs that it's being completely ignored.   Doing that is effectively the same as silently ignoring their config -- which means the implicit default {{<mergePolicyFactory>}} will be used while they *think* they are using some very explicit {{<mergePolicy/>}} and beating their heads against the wall trying to understand why 7.0 is ignoring their "maxMergedSegmentMB" etc....

A WARNing is appropriate when config option X is deprecated but still supported -- but once X is no longer supported at all we should _absolutely_ hard fail if people are still trying to configure X -- so that so they _know_ they need to change their config.


> Remove support for <mergePolicy> (in favour of <mergePolicyFactory>)
> --------------------------------------------------------------------
>
>                 Key: SOLR-8668
>                 URL: https://issues.apache.org/jira/browse/SOLR-8668
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Shai Erera
>            Assignee: Christine Poerschke
>            Priority: Blocker
>             Fix For: master (7.0)
>
>         Attachments: SOLR-8668-part1.patch, SOLR-8668-part1.patch
>
>
> Following SOLR-8621, we should remove support for {{<mergePolicy>}} (and related {{<mergeFactor>}} and {{<maxMergeDocs>}}) in trunk/6x.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org