SOLR-7362: Fix TestReqParamsAPI test failures

solr/CHANGES.txt

@@ -63,6 +63,11 @@ Apache UIMA 2.3.1
 Apache ZooKeeper 3.4.6
 Jetty 9.3.8.v20160314
 
+Detailed Change List
+----------------------
+
+New Features
+----------------------
 
 Bug Fixes
 ----------------------
@@ -75,6 +80,14 @@ Other Changes
 * SOLR-9412: Add failOnMissingParams option to MacroExpander, add TestMacroExpander class.
   (Jon Dorando, Christine Poerschke)
 
+Optimizations
+----------------------
+
+Other Changes
+----------------------
+
+* SOLR-7362: Fix TestReqParamsAPI test failures (noble, Varun Thacker)
+
 ==================  6.2.0 ==================
 
 Versions of Major Components

solr/core/src/java/org/apache/solr/core/RequestParams.java

@@ -29,6 +29,7 @@ import java.util.Map;
 import com.google.common.collect.ImmutableMap;
 import org.apache.solr.cloud.ZkSolrResourceLoader;
 import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.params.MapSolrParams;
 import org.apache.solr.common.util.Utils;
 import org.apache.zookeeper.KeeperException;
@@ -156,10 +157,8 @@ public class RequestParams implements MapSerializable {
           requestParams = new RequestParams((Map) o[0], (Integer) o[1]);
           log.info("request params refreshed to version {}", requestParams.getZnodeVersion());
         }
-      } catch (KeeperException e) {
+      } catch (KeeperException | InterruptedException e) {
-        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+        SolrZkClient.checkInterrupted(e);
-      } catch (InterruptedException e) {
-        Thread.currentThread().interrupt();
         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
       }
 

solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java

@@ -188,12 +188,12 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
             resp.add(ZNODEVER, Utils.makeMap(
                 ConfigOverlay.NAME, req.getCore().getSolrConfig().getOverlay().getZnodeVersion(),
                 RequestParams.NAME, req.getCore().getSolrConfig().getRequestParams().getZnodeVersion()));
-            boolean checkStale = false;
+            boolean isStale = false;
             int expectedVersion = req.getParams().getInt(ConfigOverlay.NAME, -1);
             int actualVersion = req.getCore().getSolrConfig().getOverlay().getZnodeVersion();
             if (expectedVersion > actualVersion) {
               log.info("expecting overlay version {} but my version is {}", expectedVersion, actualVersion);
-              checkStale = true;
+              isStale = true;
             } else if (expectedVersion != -1) {
               log.info("I already have the expected version {} of config", expectedVersion);
             }
@@ -201,11 +201,11 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
             actualVersion = req.getCore().getSolrConfig().getRequestParams().getZnodeVersion();
             if (expectedVersion > actualVersion) {
               log.info("expecting params version {} but my version is {}", expectedVersion, actualVersion);
-              checkStale = true;
+              isStale = true;
             } else if (expectedVersion != -1) {
               log.info("I already have the expected version {} of params", expectedVersion);
             }
-            if (checkStale && req.getCore().getResourceLoader() instanceof ZkSolrResourceLoader) {
+            if (isStale && req.getCore().getResourceLoader() instanceof ZkSolrResourceLoader) {
               new Thread(() -> {
                 if (!reloadLock.tryLock()) {
                   log.info("Another reload is in progress . Not doing anything");
@@ -221,7 +221,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
                 }
               }, SolrConfigHandler.class.getSimpleName() + "-refreshconf").start();
             } else {
-              log.info("checkStale {} , resourceloader {}", checkStale, req.getCore().getResourceLoader().getClass().getName());
+              log.info("isStale {} , resourceloader {}", isStale, req.getCore().getResourceLoader().getClass().getName());
             }
 
           } else {
@@ -287,7 +287,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
 
             for (Map.Entry<String, Object> entry : map.entrySet()) {
 
-              Map val = null;
+              Map val;
               String key = entry.getKey();
               if (isNullOrEmpty(key)) {
                 op.addError("null key ");
@@ -354,16 +354,13 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
         if (ops.isEmpty()) {
           ZkController.touchConfDir(zkLoader);
         } else {
-          log.info("persisting params data : {}", Utils.toJSONString(params.toMap()));
+          log.debug("persisting params data : {}", Utils.toJSONString(params.toMap()));
           int latestVersion = ZkController.persistConfigResourceToZooKeeper(zkLoader,
-              params.getZnodeVersion(),
+              params.getZnodeVersion(), RequestParams.RESOURCE, params.toByteArray(), true);
-              RequestParams.RESOURCE,
+
-              params.toByteArray(), true);
+          log.debug("persisted to version : {} ", latestVersion);
-          log.info("persisted to version : {} ", latestVersion);
           waitForAllReplicasState(req.getCore().getCoreDescriptor().getCloudDescriptor().getCollectionName(),
-              req.getCore().getCoreDescriptor().getCoreContainer().getZkController(),
+              req.getCore().getCoreDescriptor().getCoreContainer().getZkController(), RequestParams.NAME, latestVersion, 30);
-              RequestParams.NAME,
-              latestVersion, 30);
         }
 
       } else {

solr/core/src/test/org/apache/solr/handler/TestReqParamsAPI.java

@@ -22,9 +22,10 @@ import java.util.List;
 import java.util.Map;
 import java.util.function.Predicate;
 
-import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
-import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.SolrCloudTestCase;
 import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.Slice;
@@ -32,37 +33,48 @@ import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.core.RequestParams;
 import org.apache.solr.core.TestSolrConfigHandler;
 import org.apache.solr.util.RestTestHarness;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 import static java.util.Arrays.asList;
 import static org.apache.solr.handler.TestSolrConfigHandlerCloud.compareValues;
 
-public class TestReqParamsAPI extends AbstractFullDistribZkTestBase {
+public class TestReqParamsAPI extends SolrCloudTestCase {
   private List<RestTestHarness> restTestHarnesses = new ArrayList<>();
 
+  private static String COLL_NAME = "collection1";
+
   private void setupHarnesses() {
-    for (final SolrClient client : clients) {
+    for (final JettySolrRunner jettySolrRunner : cluster.getJettySolrRunners()) {
-      RestTestHarness harness = new RestTestHarness(() -> ((HttpSolrClient) client).getBaseURL());
+      RestTestHarness harness = new RestTestHarness(() -> jettySolrRunner.getBaseUrl().toString() + "/" + COLL_NAME);
       restTestHarnesses.add(harness);
     }
   }
 
-  @Override
+  @BeforeClass
-  public void distribTearDown() throws Exception {
+  public static void createCluster() throws Exception {
-    super.distribTearDown();
+    System.setProperty("managed.schema.mutable", "true");
-    for (RestTestHarness r : restTestHarnesses) {
+    configureCluster(2)
-      r.close();
+        .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-managed").resolve("conf"))
-    }
+        .configure();
+    cluster.createCollection(COLL_NAME, 1, 2, "conf1", null);
   }
 
   @Test
   public void test() throws Exception {
+    try {
       setupHarnesses();
       testReqParams();
+    } finally {
+      for (RestTestHarness r : restTestHarnesses) {
+        r.close();
+      }
+    }
   }
 
   private void testReqParams() throws Exception {
-    DocCollection coll = cloudClient.getZkStateReader().getClusterState().getCollection("collection1");
+    CloudSolrClient cloudClient = cluster.getSolrClient();
+    DocCollection coll = cloudClient.getZkStateReader().getClusterState().getCollection(COLL_NAME);
     List<String> urls = new ArrayList<>();
     for (Slice slice : coll.getSlices()) {
       for (Replica replica : slice.getReplicas())
@@ -70,14 +82,27 @@ public class TestReqParamsAPI extends AbstractFullDistribZkTestBase {
     }
 
     RestTestHarness writeHarness = restTestHarnesses.get(random().nextInt(restTestHarnesses.size()));
-    String payload = " {\n" +
+
+    String payload = "{\n" +
+        "'create-requesthandler' : { 'name' : '/dump0', 'class': 'org.apache.solr.handler.DumpRequestHandler' }\n" +
+        "}";
+
+    TestSolrConfigHandler.runConfigCommand(writeHarness, "/config?wt=json", payload);
+
+    payload = "{\n" +
+        "'create-requesthandler' : { 'name' : '/dump1', 'class': 'org.apache.solr.handler.DumpRequestHandler', 'useParams':'x' }\n" +
+        "}";
+    TestSolrConfigHandler.runConfigCommand(writeHarness, "/config?wt=json", payload);
+
+    AbstractFullDistribZkTestBase.waitForRecoveriesToFinish(COLL_NAME, cloudClient.getZkStateReader(), false, true, 90);
+
+    payload = " {\n" +
         "  'set' : {'x': {" +
         "                    'a':'A val',\n" +
         "                    'b': 'B val'}\n" +
         "             }\n" +
         "  }";
 
-
     TestSolrConfigHandler.runConfigCommand(writeHarness, "/config/params?wt=json", payload);
 
     Map result = TestSolrConfigHandler.testForResponseElement(null,
@@ -89,12 +114,6 @@ public class TestReqParamsAPI extends AbstractFullDistribZkTestBase {
         10);
     compareValues(result, "B val", asList("response", "params", "x", "b"));
 
-    payload = "{\n" +
-        "'create-requesthandler' : { 'name' : '/dump0', 'class': 'org.apache.solr.handler.DumpRequestHandler' }\n" +
-        "}";
-
-    TestSolrConfigHandler.runConfigCommand(writeHarness, "/config?wt=json", payload);
-
     TestSolrConfigHandler.testForResponseElement(null,
         urls.get(random().nextInt(urls.size())),
         "/config/overlay?wt=json",
@@ -120,12 +139,6 @@ public class TestReqParamsAPI extends AbstractFullDistribZkTestBase {
         "fomrequest",
         5);
 
-    payload = "{\n" +
-        "'create-requesthandler' : { 'name' : '/dump1', 'class': 'org.apache.solr.handler.DumpRequestHandler', 'useParams':'x' }\n" +
-        "}";
-
-    TestSolrConfigHandler.runConfigCommand(writeHarness, "/config?wt=json", payload);
-
     result = TestSolrConfigHandler.testForResponseElement(null,
         urls.get(random().nextInt(urls.size())),
         "/config/overlay?wt=json",
@@ -263,9 +276,5 @@ public class TestReqParamsAPI extends AbstractFullDistribZkTestBase {
         asList("response", "params", "y", "p"),
         null,
         10);
-
-
   }
-
-
 }