Apache
/
lucene
mirror of https://github.com/apache/lucene.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SOLR-8621: WrapperMergePolicyFactory logic tweaks 

* fix so that getMergePolicy() can now be called more than once
 * added WrapperMergePolicyFactoryTest.testUpgradeIndexMergePolicyFactory()
 * account for overlap between wrapping and wrapped setters (and disallow it)
** illustration:
   <mergePolicyFactory class="UpgradeMergePolicyFactory">
      <int name="noCFSRatio">0.24</int>
      <str name="wrapped.prefix">mergePolicy</str>
                      <str name="mergePolicy.class">TieredMergePolicyFactory</str>
                      <int name="mergePolicy.noCFSRatio">0.42</int>
   </mergePolicyFactory>
** implementation details: the wrapping MP's setter calls the wrapped MP's setter and in the current code the wrapping MP's value prevails i.e. the 0.24 value in the illustration since the wrapped MP is constructed before the wrapping MP. an end-user however might reasonably assume that the wrapped MP's 0.42 value will prevail. at best configuring the same setter twice within the same overall <mergePolicyFactory> element is ambiguous and so the code now disallows it.
This commit is contained in:
Christine Poerschke 2016-02-10 22:19:06 +00:00
parent 588e3ff084
commit 6b6932e8e1
2 changed files with 62 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
solr/core/src/java/org/apache/solr/index/WrapperMergePolicyFactory.java
View File

@ -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<String> 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},

43
solr/core/src/test/org/apache/solr/index/WrapperMergePolicyFactoryTest.java
View File

@ -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) {