From c513ae19997fd6ac78499a93c400706eec3d85cc Mon Sep 17 00:00:00 2001 From: Varun Thacker Date: Mon, 22 Aug 2016 14:28:24 +0530 Subject: [PATCH] SOLR-7362: Fix TestReqParamsAPI test failures --- solr/CHANGES.txt | 13 ++++ .../org/apache/solr/core/RequestParams.java | 7 +- .../solr/handler/SolrConfigHandler.java | 25 +++---- .../apache/solr/handler/TestReqParamsAPI.java | 73 +++++++++++-------- 4 files changed, 68 insertions(+), 50 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 349d0cc6018..8e44175b483 100644 --- a/solr/CHANGES.txt +++ b/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 diff --git a/solr/core/src/java/org/apache/solr/core/RequestParams.java b/solr/core/src/java/org/apache/solr/core/RequestParams.java index 67121062d9e..f430d929da5 100644 --- a/solr/core/src/java/org/apache/solr/core/RequestParams.java +++ b/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) { - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + } catch (KeeperException | InterruptedException e) { + SolrZkClient.checkInterrupted(e); throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } diff --git a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java index 20418f5431e..e68ba0821b0 100644 --- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java +++ b/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 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(), - RequestParams.RESOURCE, - params.toByteArray(), true); - log.info("persisted to version : {} ", latestVersion); + params.getZnodeVersion(), RequestParams.RESOURCE, params.toByteArray(), true); + + log.debug("persisted to version : {} ", latestVersion); waitForAllReplicasState(req.getCore().getCoreDescriptor().getCloudDescriptor().getCollectionName(), - req.getCore().getCoreDescriptor().getCoreContainer().getZkController(), - RequestParams.NAME, - latestVersion, 30); + req.getCore().getCoreDescriptor().getCoreContainer().getZkController(), RequestParams.NAME, latestVersion, 30); } } else { diff --git a/solr/core/src/test/org/apache/solr/handler/TestReqParamsAPI.java b/solr/core/src/test/org/apache/solr/handler/TestReqParamsAPI.java index d555a2d0a48..dde81f7d2a5 100644 --- a/solr/core/src/test/org/apache/solr/handler/TestReqParamsAPI.java +++ b/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.impl.HttpSolrClient; +import org.apache.solr.client.solrj.embedded.JettySolrRunner; +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 restTestHarnesses = new ArrayList<>(); + private static String COLL_NAME = "collection1"; + private void setupHarnesses() { - for (final SolrClient client : clients) { - RestTestHarness harness = new RestTestHarness(() -> ((HttpSolrClient) client).getBaseURL()); + for (final JettySolrRunner jettySolrRunner : cluster.getJettySolrRunners()) { + RestTestHarness harness = new RestTestHarness(() -> jettySolrRunner.getBaseUrl().toString() + "/" + COLL_NAME); restTestHarnesses.add(harness); } } - @Override - public void distribTearDown() throws Exception { - super.distribTearDown(); - for (RestTestHarness r : restTestHarnesses) { - r.close(); - } + @BeforeClass + public static void createCluster() throws Exception { + System.setProperty("managed.schema.mutable", "true"); + configureCluster(2) + .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 { - setupHarnesses(); - testReqParams(); + 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 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); - - } - - }