From 6458d4f63e38dfb37356b29c5bb8a72f6ce73398 Mon Sep 17 00:00:00 2001
From: Shalin Shekhar Mangar <shalin@apache.org>
Date: Thu, 23 Jan 2020 15:19:53 +0530
Subject: [PATCH] SOLR-14172: Collection metadata remains in zookeeper if too
 many shards are requested.

This also fixes a bug where an inability to assign a node based on existing autoscaling policy resulted in a server error instead of a bad request.

This closes #1152.

(cherry picked from commit 84270dc6cfae79efccea188175b07e43c6b5d5b7)
---
 solr/CHANGES.txt                              |  4 +++
 .../api/collections/CreateCollectionCmd.java  |  4 +--
 .../api/collections/TestCollectionAPI.java    | 30 +++++++++++++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 27ebb44d6ff..b35e8a394bf 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -150,6 +150,10 @@ Bug Fixes
 
 * SOLR-14207: Fix logging statements with less or more arguments than placeholders. (shalin)
 
+* SOLR-14172: Collection metadata remains in zookeeper if too many shards are requested. This also fixes a bug where
+  an inability to assign a node based on existing autoscaling policy resulted in a server error instead of a bad request.
+  (Andras Salamon, Kevin Risden, shalin)
+
 Other Changes
 ---------------------
 
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 bc2a0b5d3bc..fe8718268e7 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
@@ -193,7 +193,7 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
         ZkNodeProps deleteMessage = new ZkNodeProps("name", collectionName);
         new DeleteCollectionCmd(ocmh).call(clusterState, deleteMessage, results);
         // unwrap the exception
-        throw new SolrException(ErrorCode.SERVER_ERROR, e.getMessage(), e.getCause());
+        throw new SolrException(ErrorCode.BAD_REQUEST, e.getMessage(), e.getCause());
       }
 
       if (replicaPositions.isEmpty()) {
@@ -387,7 +387,7 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
           maxShardsPerNode * nodeList.size();
       int requestedShardsToCreate = numSlices * totalNumReplicas;
       if (maxShardsAllowedToCreate < requestedShardsToCreate) {
-        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Cannot create collection " + collectionName + ". Value of "
+        throw new Assign.AssignmentException("Cannot create collection " + collectionName + ". Value of "
             + MAX_SHARDS_PER_NODE + " is " + maxShardsPerNode
             + ", and the number of nodes currently live or live and part of your "+OverseerCollectionMessageHandler.CREATE_NODE_SET+" is " + nodeList.size()
             + ". This allows a maximum of " + maxShardsAllowedToCreate
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 39dd0688908..1fb46a28caa 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
@@ -97,6 +97,7 @@ public class TestCollectionAPI extends ReplicaPropertiesBase {
     clusterStatusZNodeVersion();
     testClusterStateMigration();
     testCollectionCreationCollectionNameValidation();
+    testCollectionCreationTooManyShards();
     testReplicationFactorValidaton();
     testCollectionCreationShardNameValidation();
     testAliasCreationNameValidation();
@@ -105,6 +106,35 @@ public class TestCollectionAPI extends ReplicaPropertiesBase {
     testModifyCollection(); // deletes replicationFactor property from collections, be careful adding new tests after this one!
   }
 
+  private void testCollectionCreationTooManyShards() throws Exception {
+    try (CloudSolrClient client = createCloudClient(null)) {
+      ModifiableSolrParams params = new ModifiableSolrParams();
+      params.set("action", CollectionParams.CollectionAction.CREATE.toString());
+      params.set("name", "collection_too_many");
+      params.set("router.name", "implicit");
+      params.set("numShards", "10");
+      params.set("maxShardsPerNode", 1);
+      params.set("shards", "b0,b1,b2,b3,b4,b5,b6,b7,b8,b9");
+      SolrRequest request = new QueryRequest(params);
+      request.setPath("/admin/collections");
+
+      try {
+        client.request(request);
+        fail("A collection creation request with too many shards than allowed by maxShardsPerNode should not have succeeded");
+      } catch (RemoteSolrException e) {
+        final String errorMessage = e.getMessage();
+        assertTrue(errorMessage.contains("Cannot create collection"));
+        assertTrue(errorMessage.contains("This requires 10 shards to be created (higher than the allowed number)"));
+        assertMissingCollection(client, "collection_too_many");
+      }
+    }
+  }
+
+  private void assertMissingCollection(CloudSolrClient client, String collectionName) throws Exception {
+    ClusterState clusterState = client.getZkStateReader().getClusterState();
+    assertNull(clusterState.getCollectionOrNull(collectionName));
+  }
+
   private void testModifyCollection() throws Exception {
     try (CloudSolrClient client = createCloudClient(null)) {
       ModifiableSolrParams params = new ModifiableSolrParams();