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 2021/01/06 14:48:10 UTC
[lucene-solr] branch jira/solr-15055 updated: SOLR-15055: Some
cleanup from review. Add missing pieces in other collection commands.
This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch jira/solr-15055
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/jira/solr-15055 by this push:
new 380287c SOLR-15055: Some cleanup from review. Add missing pieces in other collection commands.
380287c is described below
commit 380287cdca0e893bd96f79e749242024a39bc8ea
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Wed Jan 6 15:47:19 2021 +0100
SOLR-15055: Some cleanup from review. Add missing pieces in other collection commands.
---
.../solr/cloud/api/collections/AddReplicaCmd.java | 30 ++++++-
.../cloud/api/collections/CreateCollectionCmd.java | 19 ++---
.../cloud/api/collections/DeleteCollectionCmd.java | 21 +++++
.../plugins/AffinityPlacementFactory.java | 92 +++++++++++++---------
.../solrj/request/CollectionAdminRequest.java | 9 ++-
.../apache/solr/common/cloud/ReplicaPosition.java | 5 +-
6 files changed, 123 insertions(+), 53 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java
index b24e442..8fc607d 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java
@@ -23,6 +23,8 @@ import static org.apache.solr.cloud.api.collections.OverseerCollectionMessageHan
import static org.apache.solr.common.cloud.ZkStateReader.*;
import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF;
import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CollectionAdminParams.SKIP_NODE_ASSIGNMENT;
+import static org.apache.solr.common.params.CollectionAdminParams.WITH_COLLECTION;
import static org.apache.solr.common.params.CollectionParams.CollectionAction.ADDREPLICA;
import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
import static org.apache.solr.common.params.CommonAdminParams.TIMEOUT;
@@ -56,6 +58,7 @@ import org.apache.solr.common.cloud.Slice;
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.CommonAdminParams;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.ShardParams;
@@ -197,6 +200,29 @@ public class AddReplicaCmd implements OverseerCollectionMessageHandler.Cmd {
}
private ModifiableSolrParams getReplicaParams(ClusterState clusterState, ZkNodeProps message, @SuppressWarnings({"rawtypes"})NamedList results, String collectionName, DocCollection coll, boolean skipCreateReplicaInClusterState, String asyncId, ShardHandler shardHandler, CreateReplica createReplica) throws IOException, InterruptedException, KeeperException {
+ if (coll.getStr(WITH_COLLECTION) != null) {
+ String withCollectionName = coll.getStr(WITH_COLLECTION);
+ DocCollection withCollection = clusterState.getCollection(withCollectionName);
+ if (withCollection.getActiveSlices().size() > 1) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "The `withCollection` must have only one shard, found: " + withCollection.getActiveSlices().size());
+ }
+ String withCollectionShard = withCollection.getActiveSlices().iterator().next().getName();
+
+ List<Replica> replicas = withCollection.getReplicas(createReplica.node);
+ if (replicas == null || replicas.isEmpty()) {
+ // create a replica of withCollection on the identified node before proceeding further
+ ZkNodeProps props = new ZkNodeProps(
+ Overseer.QUEUE_OPERATION, ADDREPLICA.toString(),
+ ZkStateReader.COLLECTION_PROP, withCollectionName,
+ ZkStateReader.SHARD_ID_PROP, withCollectionShard,
+ "node", createReplica.node,
+ // since we already computed node assignments (which include assigning a node for this withCollection replica) we want to skip the assignment step
+ SKIP_NODE_ASSIGNMENT, "true",
+ CommonAdminParams.WAIT_FOR_FINAL_STATE, Boolean.TRUE.toString()); // set to true because we want `withCollection` to be ready after this collection is created
+ addReplica(clusterState, props, results, null);
+ }
+ }
+
ModifiableSolrParams params = new ModifiableSolrParams();
ZkStateReader zkStateReader = ocmh.zkStateReader;
@@ -265,7 +291,7 @@ public class AddReplicaCmd implements OverseerCollectionMessageHandler.Cmd {
ZkNodeProps message, ReplicaPosition replicaPosition) {
boolean skipCreateReplicaInClusterState = message.getBool(SKIP_CREATE_REPLICA_IN_CLUSTER_STATE, false);
- String collection = message.getStr(COLLECTION_PROP);
+ String collection = replicaPosition.collection;
String node = replicaPosition.node;
String shard = message.getStr(SHARD_ID_PROP);
String coreName = message.getStr(CoreAdminParams.NAME);
@@ -342,7 +368,7 @@ public class AddReplicaCmd implements OverseerCollectionMessageHandler.Cmd {
int i = 0;
for (Map.Entry<Replica.Type, Integer> entry : replicaTypeVsCount.entrySet()) {
for (int j = 0; j < entry.getValue(); j++) {
- positions.add(new ReplicaPosition(sliceName, i++, entry.getKey(), node));
+ positions.add(new ReplicaPosition(collectionName, sliceName, i++, entry.getKey(), node));
}
}
}
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 45d1abc..f0c2ad7 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
@@ -122,21 +122,17 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
}
String withCollection = message.getStr(WITH_COLLECTION);
- String withCollectionShard = null;
if (withCollection != null) {
- String realWithCollection = aliases.resolveSimpleAlias(withCollection);
- if (!clusterState.hasCollection(realWithCollection)) {
- throw new SolrException(ErrorCode.BAD_REQUEST, "The 'withCollection' does not exist: " + realWithCollection);
+ // nocommit: should this honor the followAliases flag?
+ // we should probably disallow using aliases in this context
+ //String realWithCollection = aliases.resolveSimpleAlias(withCollection);
+ if (!clusterState.hasCollection(withCollection)) {
+ throw new SolrException(ErrorCode.BAD_REQUEST, "The 'withCollection' does not exist: " + withCollection);
} else {
- DocCollection collection = clusterState.getCollection(realWithCollection);
- if (collection.getActiveSlices().size() > 1) {
+ DocCollection collection = clusterState.getCollection(withCollection);
+ if (collection.getActiveSlices().size() != 1) {
throw new SolrException(ErrorCode.BAD_REQUEST, "The `withCollection` must have only one shard, found: " + collection.getActiveSlices().size());
}
- withCollectionShard = collection.getActiveSlices().iterator().next().getName();
- }
- if (!realWithCollection.equals(withCollection)) {
- message = message.plus(WITH_COLLECTION, realWithCollection);
- withCollection = realWithCollection;
}
}
@@ -294,7 +290,6 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
"node", replicaPosition.node,
CommonAdminParams.WAIT_FOR_FINAL_STATE, Boolean.TRUE.toString()); // set to true because we want `withCollection` to be ready after this collection is created
new AddReplicaCmd(ocmh).call(clusterState, props, results);
- clusterState = zkStateReader.getClusterState(); // refresh
}
}
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 d9b6679..5c08057 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
@@ -52,7 +52,9 @@ import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.solr.common.params.CollectionAdminParams.COLOCATED_WITH;
import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CollectionAdminParams.WITH_COLLECTION;
import static org.apache.solr.common.params.CollectionParams.CollectionAction.DELETE;
import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
import static org.apache.solr.common.params.CommonParams.NAME;
@@ -92,6 +94,8 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
collection = extCollection;
}
+ checkNotColocatedWith(zkStateReader, collection);
+
final boolean deleteHistory = message.getBool(CoreAdminParams.DELETE_METRICS_HISTORY, true);
boolean removeCounterNode = true;
@@ -258,4 +262,21 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
.map(Map.Entry::getKey) // alias name
.collect(Collectors.toList());
}
+
+ private void checkNotColocatedWith(ZkStateReader zkStateReader, String collection) throws Exception {
+ DocCollection docCollection = zkStateReader.getClusterState().getCollectionOrNull(collection);
+ if (docCollection != null) {
+ String colocatedWith = docCollection.getStr(COLOCATED_WITH);
+ if (colocatedWith != null) {
+ DocCollection colocatedCollection = zkStateReader.getClusterState().getCollectionOrNull(colocatedWith);
+ if (colocatedCollection != null && collection.equals(colocatedCollection.getStr(WITH_COLLECTION))) {
+ // todo how do we clean up if reverse-link is not present?
+ // can't delete this collection because it is still co-located with another collection
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "Collection: " + collection + " is co-located with collection: " + colocatedWith
+ + " remove the link using modify collection API or delete the co-located collection: " + colocatedWith);
+ }
+ }
+ }
+ }
}
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
index 2166a0f..4d116b8 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
@@ -195,22 +195,6 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
Set<Node> nodes = request.getTargetNodes();
SolrCollection solrCollection = request.getCollection();
- final SolrCollection secondaryCollection;
- String withCollection = solrCollection.getCustomProperty(CollectionAdminParams.WITH_COLLECTION);
- if (withCollection != null) {
- try {
- secondaryCollection = cluster.getCollection(withCollection);
- int numShards = secondaryCollection.getShardNames().size();
- if (numShards != 1) {
- throw new PlacementException("Secondary collection '" + withCollection + "' has " + numShards + " but must have exactly 1.");
- }
- } catch (IOException e) {
- throw new PlacementException("Error retrieving secondary collection '" + withCollection + "' information", e);
- }
- } else {
- secondaryCollection = null;
- }
-
// Request all needed attributes
attributeFetcher.requestNodeSystemProperty(AVAILABILITY_ZONE_SYSPROP).requestNodeSystemProperty(REPLICA_TYPE_SYSPROP);
attributeFetcher
@@ -260,38 +244,72 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
}
}
+ postProcess(cluster, request, attributeFetcher, placementPlanFactory, replicaPlacements);
+
+ return placementPlanFactory.createPlacementPlan(request, replicaPlacements);
+ }
+
+ /**
+ * Extension hook for post-processing the list of replica placements generated by this plugin, just
+ * before they are returned to the caller. Default implementation adds replicas needed to
+ * satisfy the <code>withCollection</code> constraint.
+ * @param cluster instance of the current cluster
+ * @param request original placement request
+ * @param attributeFetcher attribute fetcher used for retrieving additional cluster/replica attributes if needed.
+ * @param placementPlanFactory factory to create replica placements
+ * @param placements computed replica placements to process (in place)
+ */
+ protected void postProcess(Cluster cluster, PlacementRequest request, AttributeFetcher attributeFetcher,
+ PlacementPlanFactory placementPlanFactory, Set<ReplicaPlacement> placements) throws PlacementException {
+ addWithColllectionReplicas(cluster, request, placementPlanFactory, placements);
+ }
+
+ protected void addWithColllectionReplicas(Cluster cluster, PlacementRequest request,
+ PlacementPlanFactory placementPlanFactory,
+ Set<ReplicaPlacement> placements) throws PlacementException {
+ final SolrCollection secondaryCollection;
+ String withCollection = request.getCollection().getCustomProperty(CollectionAdminParams.WITH_COLLECTION);
+ if (withCollection != null) {
+ try {
+ secondaryCollection = cluster.getCollection(withCollection);
+ int numShards = secondaryCollection.getShardNames().size();
+ if (numShards != 1) {
+ throw new PlacementException("Secondary collection '" + withCollection + "' has " + numShards + " but must have exactly 1.");
+ }
+ } catch (IOException e) {
+ throw new PlacementException("Error retrieving secondary collection '" + withCollection + "' information", e);
+ }
+ } else {
+ secondaryCollection = null;
+ }
+
if (secondaryCollection != null) {
- // 2nd phase to allocate required secondary collection replicas
+ //allocate required secondary collection replicas
Set<Node> secondaryNodes = new HashSet<>();
Shard shard1 = secondaryCollection.iterator().next();
shard1.replicas().forEach(r -> {
secondaryNodes.add(r.getNode());
});
Set<ReplicaPlacement> secondaryPlacements = new HashSet<>();
- Set<Node> alreadyAdded = new HashSet<>();
- replicaPlacements.forEach(primaryPlacement -> {
+ placements.forEach(primaryPlacement -> {
if (!secondaryNodes.contains(primaryPlacement.getNode())) {
- if (!alreadyAdded.contains(primaryPlacement.getNode())) {
- // missing secondary replica on the node - add it
- secondaryPlacements.add(placementPlanFactory
- .createReplicaPlacement(
- secondaryCollection,
- shard1.getShardName(),
- primaryPlacement.getNode(),
- // TODO: make this configurable
- // we default to PULL because if additional indexing
- // capacity is required the admin can manually
- // add NRT/TLOG replicas as needed
- Replica.ReplicaType.PULL));
- // avoid adding multiple replicas for multiple shards
- alreadyAdded.add(primaryPlacement.getNode());
- }
+ // missing secondary replica on the node - add it
+ secondaryPlacements.add(placementPlanFactory
+ .createReplicaPlacement(
+ secondaryCollection,
+ shard1.getShardName(),
+ primaryPlacement.getNode(),
+ // TODO: make this configurable
+ // we default to PULL because if additional indexing
+ // capacity is required the admin can manually
+ // add NRT/TLOG replicas as needed
+ Replica.ReplicaType.PULL));
+ // avoid adding multiple replicas for multiple shards
+ secondaryNodes.add(primaryPlacement.getNode());
}
});
- replicaPlacements.addAll(secondaryPlacements);
+ placements.addAll(secondaryPlacements);
}
-
- return placementPlanFactory.createPlacementPlan(request, replicaPlacements);
}
private Set<String> getZonesFromNodes(Set<Node> nodes, final AttributeValues attrValues) {
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
index f3f620d..cde6fe1 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
@@ -59,8 +59,15 @@ import static org.apache.solr.common.cloud.ZkStateReader.PULL_REPLICAS;
import static org.apache.solr.common.cloud.ZkStateReader.READ_ONLY;
import static org.apache.solr.common.cloud.ZkStateReader.REPLICATION_FACTOR;
import static org.apache.solr.common.cloud.ZkStateReader.TLOG_REPLICAS;
-import static org.apache.solr.common.params.CollectionAdminParams.*;
+import static org.apache.solr.common.params.CollectionAdminParams.ALIAS;
+import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF;
import static org.apache.solr.common.params.CollectionAdminParams.COLOCATED_WITH;
+import static org.apache.solr.common.params.CollectionAdminParams.COUNT_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
+import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_SHUFFLE_PARAM;
+import static org.apache.solr.common.params.CollectionAdminParams.ROUTER_PREFIX;
+import static org.apache.solr.common.params.CollectionAdminParams.SKIP_NODE_ASSIGNMENT;
+import static org.apache.solr.common.params.CollectionAdminParams.WITH_COLLECTION;
/**
* This class is experimental and subject to change.
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ReplicaPosition.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ReplicaPosition.java
index bb854d6..72aebb3 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ReplicaPosition.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ReplicaPosition.java
@@ -17,7 +17,10 @@
package org.apache.solr.common.cloud;
-
+/**
+ * Representation of a new replica position (placement) on a node in the cluster.
+ * <p>Note: this class has a natural ordering that is inconsistent with equals.</p>
+ */
public class ReplicaPosition implements Comparable<ReplicaPosition> {
public final String collection;
public final String shard;