diff --git a/solr/core/src/java/org/apache/solr/index/WrapperMergePolicyFactory.java b/solr/core/src/java/org/apache/solr/index/WrapperMergePolicyFactory.java index 24ba2378159..61088a8dbc0 100644 --- a/solr/core/src/java/org/apache/solr/index/WrapperMergePolicyFactory.java +++ b/solr/core/src/java/org/apache/solr/index/WrapperMergePolicyFactory.java @@ -16,7 +16,9 @@ */ package org.apache.solr.index; +import java.util.HashSet; import java.util.Iterator; +import java.util.Set; import org.apache.lucene.index.MergePolicy; import org.apache.solr.core.SolrResourceLoader; @@ -34,10 +36,26 @@ public abstract class WrapperMergePolicyFactory extends MergePolicyFactory { static final String WRAPPED_PREFIX = "wrapped.prefix"; // not private so that test(s) can use it private final MergePolicyFactoryArgs wrappedMergePolicyArgs; + private final String wrappedMergePolicyClassName; protected WrapperMergePolicyFactory(SolrResourceLoader resourceLoader, MergePolicyFactoryArgs args, IndexSchema schema) { super(resourceLoader, args, schema); wrappedMergePolicyArgs = filterWrappedMergePolicyFactoryArgs(); + if (wrappedMergePolicyArgs == null) { + wrappedMergePolicyClassName = null; + } else { + wrappedMergePolicyClassName = (String) wrappedMergePolicyArgs.remove(CLASS); + if (wrappedMergePolicyClassName == null) { + throw new IllegalArgumentException("Class name not defined for wrapped MergePolicyFactory!"); + } + } + if (wrappedMergePolicyArgs != null) { + final Set overshadowedWrappedMergePolicyArgs = new HashSet<>(wrappedMergePolicyArgs.keys()); + overshadowedWrappedMergePolicyArgs.retainAll(args.keys()); + if (!overshadowedWrappedMergePolicyArgs.isEmpty()) { + throw new IllegalArgumentException("Wrapping and wrapped merge policy args overlap! "+overshadowedWrappedMergePolicyArgs); + } + } } /** @@ -55,13 +73,8 @@ public abstract class WrapperMergePolicyFactory extends MergePolicyFactory { return getDefaultWrappedMergePolicy(); } - final String className = (String) wrappedMergePolicyArgs.remove(CLASS); - if (className == null) { - throw new IllegalArgumentException("Class name not defined for wrapped MergePolicyFactory!"); - } - final MergePolicyFactory mpf = resourceLoader.newInstance( - className, + wrappedMergePolicyClassName, MergePolicyFactory.class, NO_SUB_PACKAGES, new Class[] {SolrResourceLoader.class, MergePolicyFactoryArgs.class, IndexSchema.class}, diff --git a/solr/core/src/test/org/apache/solr/index/WrapperMergePolicyFactoryTest.java b/solr/core/src/test/org/apache/solr/index/WrapperMergePolicyFactoryTest.java index cd904f5455f..e4c7b3dc83e 100644 --- a/solr/core/src/test/org/apache/solr/index/WrapperMergePolicyFactoryTest.java +++ b/solr/core/src/test/org/apache/solr/index/WrapperMergePolicyFactoryTest.java @@ -19,6 +19,7 @@ package org.apache.solr.index; import org.apache.lucene.index.MergePolicy; import org.apache.lucene.index.NoMergePolicy; import org.apache.lucene.index.TieredMergePolicy; +import org.apache.lucene.index.UpgradeIndexMergePolicy; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.core.SolrResourceLoader; import org.apache.solr.schema.IndexSchema; @@ -68,6 +69,48 @@ public class WrapperMergePolicyFactoryTest extends SolrTestCaseJ4 { assertEquals("maxMergedSegmentMB", testMaxMergedSegmentMB, tmp.getMaxMergedSegmentMB(), 0.0d); } + public void testUpgradeIndexMergePolicyFactory() { + final int N = 10; + final Double wrappingNoCFSRatio = random().nextBoolean() ? null : random().nextInt(N+1)/((double)N); // must be: 0.0 <= value <= 1.0 + final Double wrappedNoCFSRatio = random().nextBoolean() ? null : random().nextInt(N+1)/((double)N); // must be: 0.0 <= value <= 1.0 + implTestUpgradeIndexMergePolicyFactory(wrappingNoCFSRatio, wrappedNoCFSRatio); + } + + private void implTestUpgradeIndexMergePolicyFactory(Double wrappingNoCFSRatio, Double wrappedNoCFSRatio) { + final MergePolicyFactoryArgs args = new MergePolicyFactoryArgs(); + if (wrappingNoCFSRatio != null) { + args.add("noCFSRatio", wrappingNoCFSRatio); // noCFSRatio for the wrapping merge policy + } + args.add(WrapperMergePolicyFactory.WRAPPED_PREFIX, "wrapped"); + args.add("wrapped.class", TieredMergePolicyFactory.class.getName()); + if (wrappedNoCFSRatio != null) { + args.add("wrapped.noCFSRatio", wrappedNoCFSRatio); // noCFSRatio for the wrapped merge policy + } + + MergePolicyFactory mpf; + try { + mpf = new UpgradeIndexMergePolicyFactory(resourceLoader, args, null); + assertFalse("Should only reach here if wrapping and wrapped args don't overlap!", + (wrappingNoCFSRatio != null && wrappedNoCFSRatio != null)); + + for (int ii=1; ii<=2; ++ii) { // it should be okay to call getMergePolicy() more than once + final MergePolicy mp = mpf.getMergePolicy(); + if (wrappingNoCFSRatio != null) { + assertEquals("#"+ii+" wrappingNoCFSRatio", wrappingNoCFSRatio.doubleValue(), mp.getNoCFSRatio(), 0.0d); + } + if (wrappedNoCFSRatio != null) { + assertEquals("#"+ii+" wrappedNoCFSRatio", wrappedNoCFSRatio.doubleValue(), mp.getNoCFSRatio(), 0.0d); + } + assertSame(mp.getClass(), UpgradeIndexMergePolicy.class); + } + + } catch (IllegalArgumentException iae) { + assertEquals("Wrapping and wrapped merge policy args overlap! [noCFSRatio]", iae.getMessage()); + assertTrue("Should only reach here if wrapping and wrapped args do overlap!", + (wrappingNoCFSRatio != null && wrappedNoCFSRatio != null)); + } + } + private static class DefaultingWrapperMergePolicyFactory extends WrapperMergePolicyFactory { DefaultingWrapperMergePolicyFactory(SolrResourceLoader resourceLoader, MergePolicyFactoryArgs wrapperArgs, IndexSchema schema) {