You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by is...@apache.org on 2019/04/28 17:46:44 UTC
[lucene-solr] branch master updated: SOLR-5970: Return correct
status upon collection creation failure
This is an automated email from the ASF dual-hosted git repository.
ishan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new dd9899b SOLR-5970: Return correct status upon collection creation failure
dd9899b is described below
commit dd9899b1c16aa06fd7acb43d2f43bcac44a97776
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Sun Apr 28 23:16:29 2019 +0530
SOLR-5970: Return correct status upon collection creation failure
---
solr/CHANGES.txt | 3 ++
.../cloud/api/collections/CreateCollectionCmd.java | 2 +-
.../solr/cloud/CreateCollectionCleanupTest.java | 6 ++-
.../org/apache/solr/cloud/TestConfigSetsAPI.java | 43 +++++++++++-----------
.../CollectionsAPIDistributedZkTest.java | 18 +++------
.../cloud/api/collections/TestCollectionAPI.java | 34 +++++++++++++++++
.../java/org/apache/solr/cloud/ZkTestServer.java | 15 ++++++--
7 files changed, 80 insertions(+), 41 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a72e982..35b5b11 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -225,6 +225,9 @@ Bug Fixes
* SOLR-12248, SOLR-4647: Grouping is broken on docValues-only fields (non-stored, non-indexed)
(Erick Ericsson, Adrien Grand, Munendra S N, Scott Stults, Ishan Chattopadhyaya)
+* SOLR-5970: Return correct status upon collection creation failure (Abraham Elmahrek, Ishan Chattopadhyaya,
+ Jason Gerlowski, Kesharee Nandan Vishwakarma)
+
Improvements
----------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
index 500aae0..dbbaa26 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
@@ -301,7 +301,7 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
// element, which may be interpreted by the user as a positive ack
ocmh.cleanupCollection(collectionName, new NamedList<Object>());
log.info("Cleaned up artifacts for failed create collection for [{}]", collectionName);
- return;
+ throw new SolrException(ErrorCode.BAD_REQUEST, "Underlying core creation failed while creating collection: " + collectionName);
} else {
log.debug("Finished create command on all shards for collection: {}", collectionName);
diff --git a/solr/core/src/test/org/apache/solr/cloud/CreateCollectionCleanupTest.java b/solr/core/src/test/org/apache/solr/cloud/CreateCollectionCleanupTest.java
index c569581..733a4a1 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CreateCollectionCleanupTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CreateCollectionCleanupTest.java
@@ -23,6 +23,7 @@ import static org.hamcrest.CoreMatchers.not;
import java.util.Properties;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
import org.apache.solr.client.solrj.response.RequestStatusState;
@@ -78,8 +79,9 @@ public class CreateCollectionCleanupTest extends SolrCloudTestCase {
Properties properties = new Properties();
properties.put(CoreAdminParams.DATA_DIR, "/some_invalid_dir/foo");
create.setProperties(properties);
- CollectionAdminResponse rsp = create.process(cloudClient);
- assertFalse(rsp.isSuccess());
+ expectThrows(HttpSolrClient.RemoteSolrException.class, () -> {
+ CollectionAdminResponse rsp = create.process(cloudClient);
+ });
// Confirm using LIST that the collection does not exist
assertThat("Failed collection is still in the clusterstate: " + cluster.getSolrClient().getClusterStateProvider().getClusterState().getCollectionOrNull(collectionName),
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
index f513645..5933f8f 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
@@ -61,6 +61,7 @@ import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.embedded.JettySolrRunner;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.impl.HttpClientUtil;
import org.apache.solr.client.solrj.request.ConfigSetAdminRequest;
import org.apache.solr.client.solrj.request.ConfigSetAdminRequest.Create;
@@ -89,6 +90,7 @@ import org.apache.zookeeper.KeeperException;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import static org.junit.matchers.JUnitMatchers.containsString;
import org.noggit.JSONParser;
import org.noggit.ObjectBuilder;
import org.slf4j.Logger;
@@ -344,29 +346,26 @@ public class TestConfigSetsAPI extends SolrTestCaseJ4 {
@Test
public void testUploadWithScriptUpdateProcessor() throws Exception {
- for (boolean withAuthorization: Arrays.asList(false, true)) {
- String suffix;
- if (withAuthorization) {
- suffix = "-trusted";
- protectConfigsHandler();
- uploadConfigSetWithAssertions("with-script-processor", suffix, "solr", "SolrRocks");
- } else {
- suffix = "-untrusted";
- uploadConfigSetWithAssertions("with-script-processor", suffix, null, null);
- }
+ // Authorization off
+ final String untrustedSuffix = "-untrusted";
+ uploadConfigSetWithAssertions("with-script-processor", untrustedSuffix, null, null);
// try to create a collection with the uploaded configset
- CollectionAdminResponse resp = createCollection("newcollection2", "with-script-processor"+suffix,
- 1, 1, solrCluster.getSolrClient());
-
- if (withAuthorization) {
- scriptRequest("newcollection2");
- } else {
- log.info("Client saw errors: "+resp.getErrorMessages());
- assertTrue(resp.getErrorMessages() != null && resp.getErrorMessages().size() > 0);
- assertTrue(resp.getErrorMessages().getVal(0).
- contains("The configset for this collection was uploaded without any authentication"));
- }
- }
+ Throwable thrown = expectThrows(HttpSolrClient.RemoteSolrException.class, () -> {
+ createCollection("newcollection2", "with-script-processor" + untrustedSuffix,
+ 1, 1, solrCluster.getSolrClient());
+ });
+
+ assertThat(thrown.getMessage(), containsString("Underlying core creation failed"));
+
+ // Authorization on
+ final String trustedSuffix = "-trusted";
+ protectConfigsHandler();
+ uploadConfigSetWithAssertions("with-script-processor", trustedSuffix, "solr", "SolrRocks");
+ // try to create a collection with the uploaded configset
+ CollectionAdminResponse resp = createCollection("newcollection2", "with-script-processor" + trustedSuffix,
+ 1, 1, solrCluster.getSolrClient());
+ scriptRequest("newcollection2");
+
}
protected SolrZkClient zkClient() {
diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistributedZkTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistributedZkTest.java
index 4718a10..5c3d73c 100644
--- a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistributedZkTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistributedZkTest.java
@@ -286,19 +286,11 @@ public class CollectionsAPIDistributedZkTest extends SolrCloudTestCase {
String nn1 = cluster.getJettySolrRunner(0).getNodeName();
String nn2 = cluster.getJettySolrRunner(1).getNodeName();
- CollectionAdminResponse resp = CollectionAdminRequest.createCollection("halfcollection", "conf", 2, 1)
- .setCreateNodeSet(nn1 + "," + nn2)
- .process(cluster.getSolrClient());
-
- SimpleOrderedMap success = (SimpleOrderedMap) resp.getResponse().get("success");
- SimpleOrderedMap failure = (SimpleOrderedMap) resp.getResponse().get("failure");
-
- assertNotNull(resp.toString(), success);
- assertNotNull(resp.toString(), failure);
-
- String val1 = success.getVal(0).toString();
- String val2 = failure.getVal(0).toString();
- assertTrue(val1.contains("SolrException") || val2.contains("SolrException"));
+ expectThrows(HttpSolrClient.RemoteSolrException.class, () -> {
+ CollectionAdminResponse resp = CollectionAdminRequest.createCollection("halfcollection", "conf", 2, 1)
+ .setCreateNodeSet(nn1 + "," + nn2)
+ .process(cluster.getSolrClient());
+ });
}
@Test
diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
index 531e154..2adc13e 100644
--- a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
@@ -25,13 +25,16 @@ import java.util.Map;
import java.util.concurrent.atomic.AtomicLong;
import com.google.common.collect.Lists;
+import org.apache.solr.cloud.ZkTestServer;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteSolrException;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.response.CollectionAdminResponse;
import org.apache.solr.client.solrj.request.V2Request;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrException;
@@ -40,6 +43,7 @@ import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.Slice;
+import org.apache.solr.common.cloud.SolrZkClient;
import org.apache.solr.common.params.CollectionParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.ShardParams;
@@ -971,4 +975,34 @@ public class TestCollectionAPI extends ReplicaPropertiesBase {
se.getMessage().toLowerCase(Locale.ROOT).contains("missing required parameter:"));
}
}
+
+ /**
+ * After a failed attempt to create a collection (due to bad configs), assert that
+ * the collection can be created with a good collection.
+ */
+ @Test
+ @ShardsFixed(num = 2)
+ public void testRecreateCollectionAfterFailure() throws Exception {
+ // Upload a bad configset
+ SolrZkClient zkClient = new SolrZkClient(zkServer.getZkHost(), ZkTestServer.TIMEOUT,
+ ZkTestServer.TIMEOUT, null);
+ ZkTestServer.putConfig("badconf", zkClient, "/solr", ZkTestServer.SOLRHOME, "bad-error-solrconfig.xml", "solrconfig.xml");
+ ZkTestServer.putConfig("badconf", zkClient, "/solr", ZkTestServer.SOLRHOME, "schema-minimal.xml", "schema.xml");
+ zkClient.close();
+
+ try (CloudSolrClient client = createCloudClient(null)) {
+ // first, try creating a collection with badconf
+ HttpSolrClient.RemoteSolrException rse = expectThrows(HttpSolrClient.RemoteSolrException.class, () -> {
+ CollectionAdminResponse rsp = CollectionAdminRequest.createCollection
+ ("testcollection", "badconf", 1, 2).process(client);
+ });
+ assertNotNull(rse.getMessage());
+ assertNotSame(0, rse.code());
+
+ CollectionAdminResponse rsp = CollectionAdminRequest.createCollection
+ ("testcollection", "conf1", 1, 2).process(client);
+ assertNull(rsp.getErrorMessages());
+ assertSame(0, rsp.getStatus());
+ }
+ }
}
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/ZkTestServer.java b/solr/test-framework/src/java/org/apache/solr/cloud/ZkTestServer.java
index 27c551e..4e5f869 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/ZkTestServer.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/ZkTestServer.java
@@ -788,11 +788,17 @@ public class ZkTestServer {
public static void putConfig(String confName, SolrZkClient zkClient, File solrhome, final String name)
throws Exception {
- putConfig(confName, zkClient, solrhome, name, name);
+ putConfig(confName, zkClient, null, solrhome, name, name);
}
- public static void putConfig(String confName, SolrZkClient zkClient, File solrhome, final String srcName, String destName)
- throws Exception {
+
+ public static void putConfig(String confName, SolrZkClient zkClient, File solrhome, final String srcName,
+ String destName) throws Exception {
+ putConfig(confName, zkClient, null, solrhome, srcName, destName);
+ }
+
+ public static void putConfig(String confName, SolrZkClient zkClient, String zkChroot, File solrhome,
+ final String srcName, String destName) throws Exception {
File file = new File(solrhome, "collection1"
+ File.separator + "conf" + File.separator + srcName);
if (!file.exists()) {
@@ -801,6 +807,9 @@ public class ZkTestServer {
}
String destPath = "/configs/" + confName + "/" + destName;
+ if (zkChroot != null) {
+ destPath = zkChroot + destPath;
+ }
log.info("put " + file.getAbsolutePath() + " to " + destPath);
zkClient.makePath(destPath, file, false, true);
}