lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From er...@apache.org
Subject lucene-solr:master: SOLR-11218: Fail and return an error when attempting to delete a collection that's part of an alias
Date Wed, 10 Jan 2018 01:27:18 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/master e538792d2 -> 4471c1b77


SOLR-11218: Fail and return an error when attempting to delete a collection that's part of
an alias


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/4471c1b7
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/4471c1b7
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/4471c1b7

Branch: refs/heads/master
Commit: 4471c1b77cbfa9d2cd7f804232655dc56fc859c2
Parents: e538792
Author: Erick Erickson <erick@apache.org>
Authored: Tue Jan 9 17:27:12 2018 -0800
Committer: Erick Erickson <erick@apache.org>
Committed: Tue Jan 9 17:27:12 2018 -0800

----------------------------------------------------------------------
 solr/CHANGES.txt                                |   4 +-
 .../apache/solr/cloud/DeleteCollectionCmd.java  |  13 +-
 .../apache/solr/cloud/AliasIntegrationTest.java | 160 ++++++++++++++++++-
 .../client/solrj/impl/CloudSolrClientTest.java  |  23 ---
 4 files changed, 172 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4471c1b7/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 14f80a3..4ba50b9 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -95,7 +95,7 @@ Bug Fixes
 * SOLR-11821: ConcurrentModificationException in SimSolrCloudTestCase.tearDown (shalin)
 
 * SOLR-11631: The Schema API should return non-zero status when there are failures.
-  (Noble Paul, Steve Rowe) 
+  (Noble Paul, Steve Rowe)
 
 Optimizations
 ----------------------
@@ -134,6 +134,8 @@ Other Changes
 * SOLR-11692: SolrDispatchFilter's use of a "close shield" in tests should not be applied
to
   further servlet chain processing.  (Jeff Miller, David Smiley)
 
+* SOLR-11218: Fail and return an error when attempting to delete a collection that's part
of an alias (Erick Erickson)
+
 ==================  7.2.1 ==================
 
 Bug Fixes

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4471c1b7/solr/core/src/java/org/apache/solr/cloud/DeleteCollectionCmd.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/DeleteCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/DeleteCollectionCmd.java
index dc91905..8ee0168 100644
--- a/solr/core/src/java/org/apache/solr/cloud/DeleteCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/DeleteCollectionCmd.java
@@ -21,14 +21,15 @@ package org.apache.solr.cloud;
 import java.lang.invoke.MethodHandles;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.solr.common.NonExistentCoreException;
 import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.Aliases;
 import org.apache.solr.common.cloud.ClusterState;
-import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
@@ -60,9 +61,15 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
   @Override
   public void call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception
{
     ZkStateReader zkStateReader = ocmh.zkStateReader;
+    Aliases aliases = zkStateReader.getAliases();
     final String collection = message.getStr(NAME);
-    DocCollection coll = state.getCollectionOrNull(collection);
-    String policy = coll == null ? null : coll.getPolicyName();
+    for (Map.Entry<String, List<String>> ent :  aliases.getCollectionAliasListMap().entrySet())
{
+      if (ent.getValue().contains(collection)) {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+            "Collection : " + collection + " is part of alias " + ent.getKey() + " remove
or modify the alias before removing this collection.");
+      }
+    }
+
     try {
       // Remove the snapshots meta-data for this collection in ZK. Deleting actual index
files
       // should be taken care of as part of collection delete operation.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4471c1b7/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
index d0f0e80..1c20b30 100644
--- a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
@@ -32,6 +32,7 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.UpdateRequest;
 import org.apache.solr.client.solrj.request.V2Request;
 import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.client.solrj.response.RequestStatusState;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.Aliases;
 import org.apache.solr.common.cloud.SolrZkClient;
@@ -176,7 +177,163 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
       }
     }
   }
-  
+
+  // Rather a long title, but it's common to recommend when people need to re-index for any
reason that they:
+  // 1> create a new collection
+  // 2> index the corpus to the new collection and verify it
+  // 3> create an alias pointing to the new collection WITH THE SAME NAME as their original
collection
+  // 4> delete the old collection.
+  //
+  // They may or may not have an alias already pointing to the old collection that's being
replaced.
+  // If they don't already have an alias, this leaves them with:
+  //
+  // > a collection named old_collection
+  // > a collection named new_collection
+  // > an alias old_collection->new_collection
+  //
+  // What happens when they delete old_collection now?
+  //
+  // Current behavior is that delete "does the right thing" and deletes old_collection rather
than new_collection,
+  // but if this behavior changes it could be disastrous for users so this test insures that
this behavior.
+  //
+  @Test
+  public void testDeleteAliasWithExistingCollectionName() throws Exception {
+    CollectionAdminRequest.createCollection("collection_old", "conf", 2, 1).process(cluster.getSolrClient());
+    CollectionAdminRequest.createCollection("collection_new", "conf", 1, 1).process(cluster.getSolrClient());
+    waitForState("Expected collection_old to be created with 2 shards and 1 replica", "collection_old",
clusterShape(2, 1));
+    waitForState("Expected collection_new to be created with 1 shard and 1 replica", "collection_new",
clusterShape(1, 1));
+
+    new UpdateRequest()
+        .add("id", "6", "a_t", "humpty dumpy sat on a wall")
+        .add("id", "7", "a_t", "humpty dumpy3 sat on a walls")
+        .add("id", "8", "a_t", "humpty dumpy2 sat on a walled")
+        .commit(cluster.getSolrClient(), "collection_old");
+
+    new UpdateRequest()
+        .add("id", "1", "a_t", "humpty dumpy sat on an unfortunate wall")
+        .commit(cluster.getSolrClient(), "collection_new");
+
+    QueryResponse res = cluster.getSolrClient().query("collection_old", new SolrQuery("*:*"));
+    assertEquals(3, res.getResults().getNumFound());
+
+    // Let's insure we have a "handle" to the old collection
+    CollectionAdminRequest.createAlias("collection_old_reserve", "collection_old").process(cluster.getSolrClient());
+
+    // This is the critical bit. The alias uses the _old collection name.
+    CollectionAdminRequest.createAlias("collection_old", "collection_new").process(cluster.getSolrClient());
+
+    // aliases: collection_old->collection_new, collection_old_reserve -> collection_old
-> collection_new
+    // collections: collection_new and collection_old
+
+    // Now we should only see the doc in collection_new through the collection_old alias
+    res = cluster.getSolrClient().query("collection_old", new SolrQuery("*:*"));
+    assertEquals(1, res.getResults().getNumFound());
+
+    // Now we should still transitively see collection_new
+    res = cluster.getSolrClient().query("collection_old_reserve", new SolrQuery("*:*"));
+    assertEquals(1, res.getResults().getNumFound());
+
+    // Now delete the old collection. This should fail since the collection_old_reserve points
to collection_old
+    RequestStatusState delResp = CollectionAdminRequest.deleteCollection("collection_old").processAndWait(cluster.getSolrClient(),
60);
+    assertEquals("Should have failed to delete collection: ", delResp, RequestStatusState.FAILED);
+
+    // assure ourselves that the old colletion is, indeed, still there.
+    assertNotNull("collection_old should exist!", cluster.getSolrClient().getZkStateReader().getClusterState().getCollectionOrNull("collection_old"));
+
+    // Now we should still succeed using the alias collection_old which points to collection_new
+    // aliase: collection_old -> collection_new, collection_old_reserve -> collection_old
-> collection_new
+    // collections: collection_old, collection_new
+    res = cluster.getSolrClient().query("collection_old", new SolrQuery("*:*"));
+    assertEquals(1, res.getResults().getNumFound());
+
+    Aliases aliases = cluster.getSolrClient().getZkStateReader().getAliases();
+    assertTrue("collection_old should point to collection_new", aliases.resolveAliases("collection_old").contains("collection_new"));
+    assertTrue("collection_old_reserve should point to collection_new", aliases.resolveAliases("collection_old_reserve").contains("collection_new"));
+
+    // Clean up
+    CollectionAdminRequest.deleteAlias("collection_old_reserve").processAndWait(cluster.getSolrClient(),
60);
+    CollectionAdminRequest.deleteAlias("collection_old").processAndWait(cluster.getSolrClient(),
60);
+    CollectionAdminRequest.deleteCollection("collection_new").processAndWait(cluster.getSolrClient(),
60);
+    CollectionAdminRequest.deleteCollection("collection_old").processAndWait(cluster.getSolrClient(),
60);
+    // collection_old already deleted as well as collection_old_reserve
+
+    assertNull("collection_old_reserve should be gone", cluster.getSolrClient().getZkStateReader().getAliases().getCollectionAliasMap().get("collection_old_reserve"));
+    assertNull("collection_old should be gone", cluster.getSolrClient().getZkStateReader().getAliases().getCollectionAliasMap().get("collection_old"));
+
+    assertFalse("collection_new should be gone",
+        cluster.getSolrClient().getZkStateReader().getClusterState().hasCollection("collection_new"));
+
+    assertFalse("collection_old should be gone",
+        cluster.getSolrClient().getZkStateReader().getClusterState().hasCollection("collection_old"));
+  }
+
+  // While writing the above test I wondered what happens when an alias points to two collections
and one of them
+  // is deleted.
+  @Test
+  public void testDeleteOneOfTwoCollectionsAliased() throws Exception {
+    CollectionAdminRequest.createCollection("collection_one", "conf", 2, 1).process(cluster.getSolrClient());
+    CollectionAdminRequest.createCollection("collection_two", "conf", 1, 1).process(cluster.getSolrClient());
+    waitForState("Expected collection_one to be created with 2 shards and 1 replica", "collection_one",
clusterShape(2, 1));
+    waitForState("Expected collection_two to be created with 1 shard and 1 replica", "collection_two",
clusterShape(1, 1));
+
+    new UpdateRequest()
+        .add("id", "1", "a_t", "humpty dumpy sat on a wall")
+        .commit(cluster.getSolrClient(), "collection_one");
+
+
+    new UpdateRequest()
+        .add("id", "10", "a_t", "humpty dumpy sat on a high wall")
+        .add("id", "11", "a_t", "humpty dumpy sat on a low wall")
+        .commit(cluster.getSolrClient(), "collection_two");
+
+    // Create an alias pointing to both
+    CollectionAdminRequest.createAlias("collection_alias_pair", "collection_one,collection_two").process(cluster.getSolrClient());
+
+    QueryResponse res = cluster.getSolrClient().query("collection_alias_pair", new SolrQuery("*:*"));
+    assertEquals(3, res.getResults().getNumFound());
+
+    // Now delete one of the collections, should fail since an alias points to it.
+    RequestStatusState delResp = CollectionAdminRequest.deleteCollection("collection_one").processAndWait(cluster.getSolrClient(),
60);
+
+    assertEquals("Should have failed to delete collection: ", delResp, RequestStatusState.FAILED);
+
+    // Now redefine the alias to only point to colletion two
+    CollectionAdminRequest.createAlias("collection_alias_pair", "collection_two").process(cluster.getSolrClient());
+
+    //Delete collection_one.
+    delResp = CollectionAdminRequest.deleteCollection("collection_one").processAndWait(cluster.getSolrClient(),
60);
+
+    assertEquals("Should not have failed to delete collection, it was removed from the alias:
", delResp, RequestStatusState.COMPLETED);
+
+    // Should only see two docs now in second collection
+    res = cluster.getSolrClient().query("collection_alias_pair", new SolrQuery("*:*"));
+    assertEquals(2, res.getResults().getNumFound());
+
+    // We shouldn't be able to ping the deleted collection directly as
+    // was deleted (and, assuming that it only points to collection_old).
+    try {
+      cluster.getSolrClient().query("collection_one", new SolrQuery("*:*"));
+    } catch (SolrServerException se) {
+      assertTrue(se.getMessage().contains("No live SolrServers"));
+    }
+
+    // Clean up
+    CollectionAdminRequest.deleteAlias("collection_alias_pair").processAndWait(cluster.getSolrClient(),
60);
+    CollectionAdminRequest.deleteCollection("collection_two").processAndWait(cluster.getSolrClient(),
60);
+    // collection_one already deleted
+
+    assertNull("collection_alias_pair should be gone",
+        cluster.getSolrClient().getZkStateReader().getAliases().getCollectionAliasMap().get("collection_alias_pair"));
+
+    assertFalse("collection_one should be gone",
+        cluster.getSolrClient().getZkStateReader().getClusterState().hasCollection("collection_one"));
+
+    assertFalse("collection_two should be gone",
+        cluster.getSolrClient().getZkStateReader().getClusterState().hasCollection("collection_two"));
+
+  }
+
+
   @Test
   public void test() throws Exception {
     CollectionAdminRequest.createCollection("collection1", "conf", 2, 1).process(cluster.getSolrClient());
@@ -332,6 +489,7 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
     }
   }
 
+  @Test
   public void testErrorChecks() throws Exception {
     CollectionAdminRequest.createCollection("testErrorChecks-collection", "conf", 2, 1).process(cluster.getSolrClient());
     waitForState("Expected testErrorChecks-collection to be created with 2 shards and 1 replica",
"testErrorChecks-collection", clusterShape(2, 1));

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4471c1b7/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
index c0580ed..9539846 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
@@ -220,29 +220,6 @@ public class CloudSolrClientTest extends SolrCloudTestCase {
   }
 
   @Test
-  public void testHandlingOfStaleAlias() throws Exception {
-    CloudSolrClient client = getRandomClient();
-
-    CollectionAdminRequest.createCollection("nemesis", "conf", 2, 1).process(client);
-    CollectionAdminRequest.createAlias("misconfigured-alias", "nemesis").process(client);
-    CollectionAdminRequest.deleteCollection("nemesis").process(client);
-
-    List<SolrInputDocument> docs = new ArrayList<>();
-
-    SolrInputDocument doc = new SolrInputDocument();
-    doc.addField(id, Integer.toString(1));
-    docs.add(doc);
-
-    try {
-      client.add("misconfigured-alias", docs);
-      fail("Alias points to non-existing collection, add should fail");
-    } catch (SolrException e) {
-      assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
-      assertTrue("Unexpected exception", e.getMessage().contains("Collection not found"));
-    }
-  }
-
-  @Test
   public void testRouting() throws Exception {
     
     AbstractUpdateRequest request = new UpdateRequest()


Mime
View raw message