SOLR-4934: Fix handling of <mergePolicy> init arg "useCompoundFile" needed after changes in LUCENE-5038

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1494348 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -59,6 +59,16 @@ Apache ZooKeeper 3.4.5
 Upgrading from Solr 4.3.0
 ----------------------
 
+* TieredMergePolicy and the various subtypes of LogMergePolicy no longer have 
+  an explicit "setUseCompoundFile" method.  Instead this behavior is now driven
+  entirely by the value of "setNoCFSRatio".  If you have explicitly configured 
+  one of these classes using <mergePolicy> and include an init arg like this...
+     <bool name="useCompoundFile">true</bool>
+  ...this will now be ignored and a warning will be logged.  Users with an 
+  explicitly declared <mergePolicy> are encouraged to review the current 
+  javadocs for their MergePolicy subclass and review their configured options 
+  carefully.  See SOLR-4934 and LUCENE-5038 for more information.
+
 * SOLR-4778: The signature of LogWatcher.registerListener has changed, from
   (ListenerConfig, CoreContainer) to (ListenerConfig).  Users implementing their
   own LogWatcher classes will need to change their code accordingly.
@@ -164,6 +174,10 @@ Bug Fixes
   defined in the original. Benign since the default values are saved, but still incorrect.
   (Erick Erickson, thanks Shawn Heisey for helping test!)
 
+* SOLR-4934: Fix handling of <mergePolicy> init arg "useCompoundFile" needed 
+  after changes in LUCENE-5038 (hossman)
+
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java

@@ -221,6 +221,9 @@ public class SolrIndexConfig {
 
       if (mergeFactor != -1)
         logMergePolicy.setMergeFactor(mergeFactor);
+
+      fixUseCFInitArg(LogMergePolicy.class);
+
     } else if (policy instanceof TieredMergePolicy) {
       TieredMergePolicy tieredMergePolicy = (TieredMergePolicy) policy;
       
@@ -230,8 +233,11 @@ public class SolrIndexConfig {
         tieredMergePolicy.setMaxMergeAtOnce(mergeFactor);
         tieredMergePolicy.setSegmentsPerTier(mergeFactor);
       }
-    } else {
-      log.warn("Use of compound file format or mergefactor cannot be configured if merge policy is not an instance of LogMergePolicy or TieredMergePolicy. The configured policy's defaults will be used.");
+
+      fixUseCFInitArg(TieredMergePolicy.class);
+
+    } else if (useCompoundFile && (mergeFactor != -1)) {
+      log.warn("Use of <useCompoundFile> or <mergeFactor> cannot be configured if merge policy is not an instance of LogMergePolicy or TieredMergePolicy. The configured policy's defaults will be used.");
     }
 
     if (mergePolicyInfo != null)
@@ -249,4 +255,20 @@ public class SolrIndexConfig {
 
     return scheduler;
   }
+
+  /**
+   * Lucene 4.4 removed the setUseCompoundFile(boolean) method from the two 
+   * conrete MergePolicies provided with Lucene/Solr.  In the event that users 
+   * have a value explicitly configured for this setter in their mergePolicy 
+   * init args, we remove it for them and warn about it.
+   */
+  private void fixUseCFInitArg(Class c) {
+
+    if (null == mergePolicyInfo || null == mergePolicyInfo.initArgs) return;
+
+    Object useCFSArg = mergePolicyInfo.initArgs.remove("useCompoundFile");
+    if (null != useCFSArg) {
+      log.warn("Ignoring 'useCompoundFile' specified as an init arg for the <mergePolicy> since it is no longer supported by " + c.getSimpleName());
+    }
+  }
 }

solr/core/src/test-files/solr/collection1/conf/solrconfig-mergepolicy-defaults.xml

@@ -0,0 +1,32 @@
+<?xml version="1.0" ?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<config>
+  <luceneMatchVersion>${tests.luceneMatchVersion:LUCENE_CURRENT}</luceneMatchVersion>
+  <directoryFactory name="DirectoryFactory" class="${solr.directoryFactory:solr.RAMDirectoryFactory}"/>
+
+  <indexConfig>
+    <!-- do not put any merge policy, merge factor 
+         or CFS related settings here 
+    -->
+  </indexConfig>
+
+  <requestHandler name="standard" class="solr.StandardRequestHandler"></requestHandler>
+
+</config>

solr/core/src/test-files/solr/collection1/conf/solrconfig-mergepolicy-legacy.xml

@@ -0,0 +1,31 @@
+<?xml version="1.0" ?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<config>
+  <luceneMatchVersion>${tests.luceneMatchVersion:LUCENE_CURRENT}</luceneMatchVersion>
+  <directoryFactory name="DirectoryFactory" class="${solr.directoryFactory:solr.RAMDirectoryFactory}"/>
+
+  <indexConfig>
+    <mergeFactor>7</mergeFactor>
+    <useCompoundFile>true</useCompoundFile>
+  </indexConfig>
+
+  <requestHandler name="standard" class="solr.StandardRequestHandler"></requestHandler>
+
+</config>

solr/core/src/test-files/solr/collection1/conf/solrconfig-mergepolicy.xml

@@ -26,7 +26,13 @@
     <mergePolicy class="org.apache.lucene.index.TieredMergePolicy">
       <int name="maxMergeAtOnceExplicit">19</int>
       <int name="segmentsPerTier">9</int>
-      <double name="noCFSRatio">1.0</double>
+      <double name="noCFSRatio">0.6</double>
+
+      <!-- Setter for this was removed in Lucene 4.4, but we should
+           ignore it with a warning if users have it in their configs
+      -->
+      <bool name="useCompoundFile">true</bool>
+
     </mergePolicy>
   </indexConfig>
 

solr/core/src/test/org/apache/solr/core/TestMergePolicyConfig.java

@@ -21,31 +21,64 @@ import org.apache.lucene.index.IndexWriterConfig;
 import org.apache.lucene.index.MergePolicy;
 import org.apache.lucene.index.TieredMergePolicy;
 import org.apache.solr.SolrTestCaseJ4;
-import org.junit.BeforeClass;
+import org.junit.After;
 
 public class TestMergePolicyConfig extends SolrTestCaseJ4 {
 
-  @BeforeClass
-  public static void beforeClass() throws Exception {
-    initCore("solrconfig-mergepolicy.xml","schema.xml");
+  @After
+  public void after() throws Exception {
+    deleteCore();
+  }
+
+  public void testDefaultMergePolicyConfig() throws Exception {
+    initCore("solrconfig-mergepolicy-defaults.xml","schema-minimal.xml");
+    IndexWriterConfig iwc = solrConfig.indexConfig.toIndexWriterConfig(h.getCore().getLatestSchema());
+
+    TieredMergePolicy tieredMP = assertAndCast(TieredMergePolicy.class,
+                                               iwc.getMergePolicy());
+    assertEquals(0.0D, tieredMP.getNoCFSRatio(), 0.0D);
+
+  }
+
+  public void testLegacyMergePolicyConfig() throws Exception {
+    initCore("solrconfig-mergepolicy-legacy.xml","schema-minimal.xml");
+    IndexWriterConfig iwc = solrConfig.indexConfig.toIndexWriterConfig(h.getCore().getLatestSchema());
+    TieredMergePolicy tieredMP = assertAndCast(TieredMergePolicy.class,
+                                               iwc.getMergePolicy());
+
+    assertEquals(7, tieredMP.getMaxMergeAtOnce());
+    assertEquals(7.0D, tieredMP.getSegmentsPerTier(), 0.0D);
+    assertEquals(1.0D, tieredMP.getNoCFSRatio(), 0.0D);
   }
   
   public void testTieredMergePolicyConfig() throws Exception {
+    initCore("solrconfig-mergepolicy.xml","schema-minimal.xml");
     IndexWriterConfig iwc = solrConfig.indexConfig.toIndexWriterConfig(h.getCore().getLatestSchema());
-    MergePolicy mp = iwc.getMergePolicy();
-    assertTrue(mp instanceof TieredMergePolicy);
-    TieredMergePolicy tieredMP = (TieredMergePolicy) mp;
-
-    // mp-specific setter
-    assertEquals(19, tieredMP.getMaxMergeAtOnceExplicit());
+    TieredMergePolicy tieredMP = assertAndCast(TieredMergePolicy.class,
+                                               iwc.getMergePolicy());
 
+    // set by legacy <mergeFactor> setting
     assertEquals(7, tieredMP.getMaxMergeAtOnce());
     
-    // make sure we overrode segmentsPerTier (split from maxMergeAtOnce out of mergeFactor)
+    // mp-specific setters
+    assertEquals(19, tieredMP.getMaxMergeAtOnceExplicit());
+    assertEquals(0.6D, tieredMP.getNoCFSRatio(), 0.001);
+    // make sure we overrode segmentsPerTier 
+    // (split from maxMergeAtOnce out of mergeFactor)
     assertEquals(9D, tieredMP.getSegmentsPerTier(), 0.001);
     
-    // make sure we overrode noCFSRatio (useless because we disabled useCompoundFile,
-    // but just to make sure it works)
-    assertEquals(1.0D, tieredMP.getNoCFSRatio(), 0.001);
   }
+
+  /**
+   * Given a Type and an object asserts that the object is non-null and an 
+   * instance of the specified Type.  The object is then cast to that type and 
+   * returned.
+   */
+  public static <T> T assertAndCast(Class<? extends T> clazz, Object o) {
+    assertNotNull(clazz);
+    assertNotNull(o);
+    assertTrue(clazz.isInstance(o));
+    return clazz.cast(o);
+  }
+
 }