You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by cp...@apache.org on 2016/02/10 23:27:03 UTC
lucene-solr git commit: SOLR-8621: WrapperMergePolicyFactory logic
tweaks
Repository: lucene-solr
Updated Branches:
refs/heads/master 588e3ff08 -> 6b6932e8e
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.
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/6b6932e8
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/6b6932e8
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/6b6932e8
Branch: refs/heads/master
Commit: 6b6932e8e1f72caf29a078f0532a56c284711f9f
Parents: 588e3ff
Author: Christine Poerschke <cp...@apache.org>
Authored: Wed Feb 10 22:19:06 2016 +0000
Committer: Christine Poerschke <cp...@apache.org>
Committed: Wed Feb 10 22:24:15 2016 +0000
----------------------------------------------------------------------
.../solr/index/WrapperMergePolicyFactory.java | 25 +++++++++---
.../index/WrapperMergePolicyFactoryTest.java | 43 ++++++++++++++++++++
2 files changed, 62 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6b6932e8/solr/core/src/java/org/apache/solr/index/WrapperMergePolicyFactory.java
----------------------------------------------------------------------
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 24ba237..61088a8 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<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},
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6b6932e8/solr/core/src/test/org/apache/solr/index/WrapperMergePolicyFactoryTest.java
----------------------------------------------------------------------
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 cd904f5..e4c7b3d 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) {