You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by da...@apache.org on 2018/01/11 08:01:20 UTC
[44/50] [abbrv] lucene-solr:jira/solr-11702: SOLR-11218: Fail and
return an error when attempting to delete a collection that's part of an
alias
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/jira/solr-11702
Commit: 4471c1b77cbfa9d2cd7f804232655dc56fc859c2
Parents: e538792
Author: Erick Erickson <er...@apache.org>
Authored: Tue Jan 9 17:27:12 2018 -0800
Committer: Erick Erickson <er...@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()