You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2020/04/09 10:52:25 UTC
[lucene-solr] branch jira/solr-12847 updated: SOLR-12847: Correctly
record the auto-created policy,
and improve the cleanup when deleting a collection with an auto-created
policy.
This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch jira/solr-12847
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/jira/solr-12847 by this push:
new 8e0ccc7 SOLR-12847: Correctly record the auto-created policy, and improve the cleanup when deleting a collection with an auto-created policy.
8e0ccc7 is described below
commit 8e0ccc7906f9b55248e3f87e6b2ac5a745caecbf
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Thu Apr 9 12:51:35 2020 +0200
SOLR-12847: Correctly record the auto-created policy, and improve the cleanup when
deleting a collection with an auto-created policy.
---
.../cloud/api/collections/CreateCollectionCmd.java | 66 +++++++++++++++-------
.../cloud/api/collections/DeleteCollectionCmd.java | 38 ++++++++++++-
.../solr/cloud/autoscaling/TestPolicyCloud.java | 7 +++
.../solr/common/params/CollectionAdminParams.java | 4 ++
4 files changed, 93 insertions(+), 22 deletions(-)
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 ee8e489..e686f25 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
@@ -61,6 +61,7 @@ import org.apache.solr.common.cloud.ZkNodeProps;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.cloud.ZooKeeperException;
import org.apache.solr.common.params.CollectionAdminParams;
+import org.apache.solr.common.params.CollectionParams;
import org.apache.solr.common.params.CommonAdminParams;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.params.ModifiableSolrParams;
@@ -191,10 +192,11 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
// refresh cluster state
clusterState = ocmh.cloudManager.getClusterStateProvider().getClusterState();
-
+ DocCollection docCollection = clusterState.getCollection(collectionName);
+ String initialPolicy = docCollection.getPolicyName();
List<ReplicaPosition> replicaPositions = null;
try {
- replicaPositions = buildReplicaPositions(ocmh.cloudManager, clusterState, clusterState.getCollection(collectionName),
+ replicaPositions = buildReplicaPositions(ocmh.cloudManager, clusterState, docCollection,
message, shardNames, sessionWrapper, configToRestore);
} catch (Assign.AssignmentException e) {
failure = true;
@@ -209,6 +211,14 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
return;
}
+ if (docCollection.getStr(Policy.POLICY) != initialPolicy) {
+ ZkNodeProps props = new ZkNodeProps(
+ Overseer.QUEUE_OPERATION, CollectionParams.CollectionAction.MODIFYCOLLECTION.toLower(),
+ ZkStateReader.COLLECTION_PROP, collectionName,
+ Policy.POLICY, docCollection.getStr(Policy.POLICY));
+ ocmh.overseer.offerStateUpdate(Utils.toJSON(props));
+ }
+
final ShardRequestTracker shardRequestTracker = ocmh.asyncRequestTracker(async);
log.debug(formatString("Creating SolrCores for new collection {0}, shardNames {1} , message : {2}",
collectionName, shardNames, message));
@@ -358,7 +368,7 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
public static void restoreAutoScalingConfig(SolrCloudManager cloudManager, AutoScalingConfig config) throws IOException, InterruptedException {
try {
- cloudManager.getDistribStateManager().setData(SOLR_AUTOSCALING_CONF_PATH, Utils.toJSON(config), -1);
+ cloudManager.getDistribStateManager().setData(SOLR_AUTOSCALING_CONF_PATH, Utils.toJSON(config), config.getZkVersion());
} catch (BadVersionException | KeeperException e) {
log.warn("Error restoring autoscaling config", e);
}
@@ -461,36 +471,50 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
return replicaPositions;
}
+ /**
+ * Optionally add a policy that implements 'maxShardsPerNode' and set the collection to use this policy.
+ * @param cloudManager SolrCloudManager
+ * @param maxReplicasPerNode max number of replicas per node
+ * @param docCollection collection (its properties may be modified as a side-effect)
+ * @param configToRestore if AutoScalingConfig has been changed as a result of this method the original
+ * value is set here in order to restore it in case of failures
+ */
public static void maybeAddMaxReplicasRule(SolrCloudManager cloudManager, int maxReplicasPerNode, DocCollection docCollection,
AtomicReference<AutoScalingConfig> configToRestore) throws IOException, InterruptedException {
AutoScalingConfig initialConfig = cloudManager.getDistribStateManager().getAutoScalingConfig();
Policy policy = initialConfig.getPolicy();
String policyName = docCollection.getPolicyName();
+ if (policyName == null) {
+ policyName = CollectionAdminParams.AUTO_PREFIX + docCollection.getName();
+ docCollection.getProperties().put(Policy.POLICY, policyName);
+ }
Map<String, List<Clause>> policies = policy.getPolicies();
- List<Clause> clauses;
- if (policyName != null) {
- clauses = policies.get(policyName);
- if (clauses != null) {
- for (Clause clause : clauses) {
- if (clause.getReplica() != null) {
- throw new Assign.AssignmentException("Both an existing policy=" + policyName + " and " +
- MAX_SHARDS_PER_NODE + "=" + maxReplicasPerNode +
- " was specified, cannot determine the correct replica count in policy: " + clauses +
- ". Edit the policy to include rules equivalent to " + MAX_SHARDS_PER_NODE);
- }
+ List<Clause> clauses = policies.get(policyName);
+ maxReplicasPerNode++; // only < supported
+ Clause c = Clause.create("{'replica': '<" + maxReplicasPerNode +"' , 'shard': '#ANY', 'node': '#ANY'}");
+ boolean alreadyExists = false;
+ if (clauses != null) {
+ for (Clause clause : clauses) {
+ if (clause.equals(c)) {
+ alreadyExists = true;
+ break;
+ }
+ if (clause.getReplica() != null) {
+ throw new Assign.AssignmentException("Both an existing policy=" + policyName + " and " +
+ MAX_SHARDS_PER_NODE + "=" + maxReplicasPerNode +
+ " was specified, cannot determine the correct replica count in policy: " + clauses +
+ ". Edit the policy to include rules equivalent to " + MAX_SHARDS_PER_NODE);
}
- clauses = new ArrayList<>(clauses);
- } else {
- clauses = new ArrayList<>();
}
+ clauses = new ArrayList<>(clauses);
} else {
- policyName = ".auto_" + docCollection.getName();
clauses = new ArrayList<>();
- docCollection.getProperties().put(Policy.POLICY, policyName);
+ }
+ if (alreadyExists) {
+ // the same clause already exists, and we modified the collection to use the policy
+ return;
}
// add a clause
- maxReplicasPerNode++; // only < supported
- Clause c = Clause.create("{'replica': '<" + maxReplicasPerNode +"' , 'shard': '#ANY', 'node': '#ANY'}");
clauses.add(c);
policies = new HashMap<>(policies);
policies.put(policyName, clauses);
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
index 648f5ba..5defc62 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
@@ -20,6 +20,7 @@ package org.apache.solr.cloud.api.collections;
import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -28,6 +29,9 @@ import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
+import org.apache.solr.client.solrj.cloud.autoscaling.AutoScalingConfig;
+import org.apache.solr.client.solrj.cloud.autoscaling.Clause;
+import org.apache.solr.client.solrj.cloud.autoscaling.Policy;
import org.apache.solr.cloud.Overseer;
import org.apache.solr.common.NonExistentCoreException;
import org.apache.solr.common.SolrException;
@@ -38,6 +42,7 @@ import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.SolrZkClient;
import org.apache.solr.common.cloud.ZkNodeProps;
import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.params.CollectionAdminParams;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.NamedList;
@@ -104,7 +109,8 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
SolrZkClient zkClient = zkStateReader.getZkClient();
SolrSnapshotManager.cleanupCollectionLevelSnapshots(zkClient, collection);
- if (zkStateReader.getClusterState().getCollectionOrNull(collection) == null) {
+ DocCollection docCollection = zkStateReader.getClusterState().getCollectionOrNull(collection);
+ if (docCollection == null) {
if (zkStateReader.getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collection, true)) {
// if the collection is not in the clusterstate, but is listed in zk, do nothing, it will just
// be removed in the finally - we cannot continue, because the below code will error if the collection
@@ -159,6 +165,36 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
});
}
+ // remove auto-created policy if this is the only collection that uses it
+ String policyName = docCollection.getStr(Policy.POLICY);
+ if (policyName != null && policyName.startsWith(CollectionAdminParams.AUTO_PREFIX)) {
+ // check that no other collection uses it
+ boolean inUse = false;
+ for (DocCollection coll : state.getCollectionsMap().values()) {
+ if (coll.getName().equals(docCollection.getName())) {
+ continue;
+ }
+ if (policyName.equals(coll.getPolicyName())) {
+ inUse = true;
+ break;
+ }
+ }
+ if (!inUse) {
+ // remove it and persist the new AutoScalingConfig
+ AutoScalingConfig autoScalingConfig = zkStateReader.getAutoScalingConfig();
+ Policy policy = autoScalingConfig.getPolicy();
+ Map<String, List<Clause>> policies = new HashMap<>(policy.getPolicies());
+ policies.remove(policyName);
+ policy = policy.withPolicies(policies);
+ autoScalingConfig = autoScalingConfig.withPolicy(policy);
+ try {
+ zkClient.setData(ZkStateReader.SOLR_AUTOSCALING_CONF_PATH, Utils.toJSON(autoScalingConfig), autoScalingConfig.getZkVersion(), true);
+ } catch (Exception e) {
+ log.warn("Error removing auto-created policy " + policyName, e);
+ }
+ }
+ }
+
// TimeOut timeout = new TimeOut(60, TimeUnit.SECONDS, timeSource);
// boolean removed = false;
// while (! timeout.hasTimedOut()) {
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
index 2dbb266..bc086c8 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
@@ -592,6 +592,13 @@ public class TestPolicyCloud extends SolrCloudTestCase {
List<Clause> p = policies.get(policyName);
assertEquals("unexpected size of policy " + policyName + ": " + p, 1, p.size());
assertTrue("incorrect clause: " + p.get(0), p.get(0).toString().contains("\"replica\":\"<2\""));
+
+ // test deletion of auto-created policy
+ CollectionAdminRequest.deleteCollection(collectionName).process(cluster.getSolrClient());
+ autoScalingConfig = cluster.getSolrClient().getZkStateReader().getAutoScalingConfig();
+ policies = autoScalingConfig.getPolicy().getPolicies();
+ assertNull("auto-create policy still exists after collection has been deleted: " + policies, policies.get(policyName));
+
}
}
diff --git a/solr/solrj/src/java/org/apache/solr/common/params/CollectionAdminParams.java b/solr/solrj/src/java/org/apache/solr/common/params/CollectionAdminParams.java
index 5af2345..62d5c68 100644
--- a/solr/solrj/src/java/org/apache/solr/common/params/CollectionAdminParams.java
+++ b/solr/solrj/src/java/org/apache/solr/common/params/CollectionAdminParams.java
@@ -127,4 +127,8 @@ public interface CollectionAdminParams {
/** Option to follow aliases when deciding the target of a collection admin command. */
String FOLLOW_ALIASES = "followAliases";
+
+ /** Prefix for automatically created config elements. */
+ String AUTO_PREFIX = ".auto_";
+
}