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/11 14:25:21 UTC
[2/2] lucene-solr git commit: SOLR-8621: WrapperMergePolicyFactory
logic tweaks
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/3f06f9a7
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/3f06f9a7
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/3f06f9a7
Branch: refs/heads/branch_5_5
Commit: 3f06f9a76f97203445ff9b35306ec0dcda59e0d8
Parents: 47055d5
Author: Christine Poerschke <cp...@apache.org>
Authored: Wed Feb 10 22:19:06 2016 +0000
Committer: Christine Poerschke <cp...@apache.org>
Committed: Thu Feb 11 13:10:28 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/3f06f9a7/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/3f06f9a7/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) {