murblanc commented on a change in pull request #2199:
URL: https://github.com/apache/lucene-solr/pull/2199#discussion_r563645684
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
##########
@@ -238,11 +258,93 @@ public PlacementPlan computePlacement(Cluster cluster,
PlacementRequest request,
// failure. Current code does fail if placement is impossible
(constraint is at most one replica of a shard on any node).
for (Replica.ReplicaType replicaType : Replica.ReplicaType.values()) {
makePlacementDecisions(solrCollection, shardName, availabilityZones,
replicaType, request.getCountReplicasToCreate(replicaType),
- attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementPlanFactory, replicaPlacements);
+ attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementContext.getPlacementPlanFactory(), replicaPlacements);
}
}
- return placementPlanFactory.createPlacementPlan(request,
replicaPlacements);
+ return
placementContext.getPlacementPlanFactory().createPlacementPlan(request,
replicaPlacements);
+ }
+
+ @Override
+ public void verifyAllowedModification(ModificationRequest
modificationRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ if (modificationRequest instanceof DeleteShardsRequest) {
+ throw new UnsupportedOperationException("not implemented yet");
+ } else if (modificationRequest instanceof DeleteCollectionRequest) {
+ verifyDeleteCollection((DeleteCollectionRequest) modificationRequest,
placementContext);
+ } else if (modificationRequest instanceof DeleteReplicasRequest) {
+ verifyDeleteReplicas((DeleteReplicasRequest) modificationRequest,
placementContext);
+ } else {
+ throw new UnsupportedOperationException("unsupported request type " +
modificationRequest.getClass().getName());
+ }
+ }
+
+ private void verifyDeleteCollection(DeleteCollectionRequest
deleteCollectionRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
Review comment:
Can we have cycles in the `withCollection` graph? Should we allow a way
to override the vetting checks from the Collection API?
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/DeleteShardsRequest.java
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cluster.placement;
+
+import java.util.Set;
+
+/**
+ * Delete shards request.
+ */
+public interface DeleteShardsRequest extends ModificationRequest {
Review comment:
If we don't use this interface (i.e. the class that implements it) I
suggest we do not include either in this PR. Or at least define and call the
corresponding method in `AssignStrategy` from the appropriate `*Cmd` even if
nothing does a real implementation and vetting based on it (but it would be
ready to be consumed maybe by another plugin written by some user).
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
##########
@@ -238,11 +258,93 @@ public PlacementPlan computePlacement(Cluster cluster,
PlacementRequest request,
// failure. Current code does fail if placement is impossible
(constraint is at most one replica of a shard on any node).
for (Replica.ReplicaType replicaType : Replica.ReplicaType.values()) {
makePlacementDecisions(solrCollection, shardName, availabilityZones,
replicaType, request.getCountReplicasToCreate(replicaType),
- attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementPlanFactory, replicaPlacements);
+ attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementContext.getPlacementPlanFactory(), replicaPlacements);
}
}
- return placementPlanFactory.createPlacementPlan(request,
replicaPlacements);
+ return
placementContext.getPlacementPlanFactory().createPlacementPlan(request,
replicaPlacements);
+ }
+
+ @Override
+ public void verifyAllowedModification(ModificationRequest
modificationRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ if (modificationRequest instanceof DeleteShardsRequest) {
+ throw new UnsupportedOperationException("not implemented yet");
+ } else if (modificationRequest instanceof DeleteCollectionRequest) {
+ verifyDeleteCollection((DeleteCollectionRequest) modificationRequest,
placementContext);
+ } else if (modificationRequest instanceof DeleteReplicasRequest) {
+ verifyDeleteReplicas((DeleteReplicasRequest) modificationRequest,
placementContext);
+ } else {
+ throw new UnsupportedOperationException("unsupported request type " +
modificationRequest.getClass().getName());
Review comment:
here too. Allow everything we don't plan to explicitly disallow.
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
##########
@@ -467,7 +569,7 @@ private void makePlacementDecisions(SolrCollection
solrCollection, String shardN
if (candidateAzEntries == null) {
// This can happen because not enough nodes for the placement
request or already too many nodes with replicas of
// the shard that can't accept new replicas or not enough nodes with
enough free disk space.
- throw new PlacementException("Not enough nodes to place " +
numReplicas + " replica(s) of type " + replicaType +
+ throw new PlacementException("Not enough eligible nodes to place " +
numReplicas + " replica(s) of type " + replicaType +
Review comment:
Not sure if it's useful here or output elsewhere already though.
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
##########
@@ -43,14 +46,28 @@
@JsonProperty
public long prioritizedFreeDiskGB;
+ /**
+ * This property defines an additional constraint that primary collections
(keys) should be
+ * located on the same nodes as the secondary collections (values). The
plugin will assume
+ * that the secondary collection replicas are already in place and ignore
candidate nodes where
+ * they are not already present.
+ */
+ @JsonProperty
+ public Map<String, String> withCollections;
+
// no-arg public constructor required for deserialization
public AffinityPlacementConfig() {
- minimalFreeDiskGB = 20L;
- prioritizedFreeDiskGB = 100L;
+ this (20L, 100L, Map.of());
Review comment:
Also, this constructor should call the constructor not requiring a
`Map`, it will add an empty one.
BTW, the deserialization constructor should not pass "meaningful" default
values since those will not be used.
I'd call `this(0L, 0L)` here instead, and switch the
`AffinityPlacementConfig DEFAULT` static variable to use the constructor
accepting values. That will also make seeing what are the defaults easier when
reading the code.
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
##########
@@ -467,7 +569,7 @@ private void makePlacementDecisions(SolrCollection
solrCollection, String shardN
if (candidateAzEntries == null) {
// This can happen because not enough nodes for the placement
request or already too many nodes with replicas of
// the shard that can't accept new replicas or not enough nodes with
enough free disk space.
- throw new PlacementException("Not enough nodes to place " +
numReplicas + " replica(s) of type " + replicaType +
+ throw new PlacementException("Not enough eligible nodes to place " +
numReplicas + " replica(s) of type " + replicaType +
Review comment:
Can we add in the message if there were `withCollection` constraints
used in the placement logic? No details, not even indicating that it's the
failure reason, just mentioning it...
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
##########
@@ -238,11 +258,93 @@ public PlacementPlan computePlacement(Cluster cluster,
PlacementRequest request,
// failure. Current code does fail if placement is impossible
(constraint is at most one replica of a shard on any node).
for (Replica.ReplicaType replicaType : Replica.ReplicaType.values()) {
makePlacementDecisions(solrCollection, shardName, availabilityZones,
replicaType, request.getCountReplicasToCreate(replicaType),
- attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementPlanFactory, replicaPlacements);
+ attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementContext.getPlacementPlanFactory(), replicaPlacements);
}
}
- return placementPlanFactory.createPlacementPlan(request,
replicaPlacements);
+ return
placementContext.getPlacementPlanFactory().createPlacementPlan(request,
replicaPlacements);
+ }
+
+ @Override
+ public void verifyAllowedModification(ModificationRequest
modificationRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ if (modificationRequest instanceof DeleteShardsRequest) {
+ throw new UnsupportedOperationException("not implemented yet");
Review comment:
If not implemented we should let it happen rather than have it fail.
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
##########
@@ -238,11 +258,93 @@ public PlacementPlan computePlacement(Cluster cluster,
PlacementRequest request,
// failure. Current code does fail if placement is impossible
(constraint is at most one replica of a shard on any node).
for (Replica.ReplicaType replicaType : Replica.ReplicaType.values()) {
makePlacementDecisions(solrCollection, shardName, availabilityZones,
replicaType, request.getCountReplicasToCreate(replicaType),
- attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementPlanFactory, replicaPlacements);
+ attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementContext.getPlacementPlanFactory(), replicaPlacements);
}
}
- return placementPlanFactory.createPlacementPlan(request,
replicaPlacements);
+ return
placementContext.getPlacementPlanFactory().createPlacementPlan(request,
replicaPlacements);
+ }
+
+ @Override
+ public void verifyAllowedModification(ModificationRequest
modificationRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ if (modificationRequest instanceof DeleteShardsRequest) {
+ throw new UnsupportedOperationException("not implemented yet");
+ } else if (modificationRequest instanceof DeleteCollectionRequest) {
+ verifyDeleteCollection((DeleteCollectionRequest) modificationRequest,
placementContext);
+ } else if (modificationRequest instanceof DeleteReplicasRequest) {
+ verifyDeleteReplicas((DeleteReplicasRequest) modificationRequest,
placementContext);
+ } else {
+ throw new UnsupportedOperationException("unsupported request type " +
modificationRequest.getClass().getName());
+ }
+ }
+
+ private void verifyDeleteCollection(DeleteCollectionRequest
deleteCollectionRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ Cluster cluster = placementContext.getCluster();
+ Set<String> colocatedCollections =
colocatedWith.getOrDefault(deleteCollectionRequest.getCollection().getName(),
Set.of());
+ for (String primaryName : colocatedCollections) {
+ try {
+ if (cluster.getCollection(primaryName) != null) {
+ // still exists
+ throw new PlacementModificationException("colocated collection " +
primaryName + " still present");
+ }
+ } catch (IOException e) {
+ throw new PlacementModificationException("failed to retrieve
colocated collection information", e);
+ }
+ }
+ }
+
+ private void verifyDeleteReplicas(DeleteReplicasRequest
deleteReplicasRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ Cluster cluster = placementContext.getCluster();
+ SolrCollection secondaryCollection =
deleteReplicasRequest.getCollection();
+ Set<String> colocatedCollections =
colocatedWith.get(secondaryCollection.getName());
+ if (colocatedCollections == null) {
+ return;
+ }
+ Map<Node, Map<String, AtomicInteger>> secondaryNodeShardReplicas = new
HashMap<>();
+ secondaryCollection.shards().forEach(shard ->
+ shard.replicas().forEach(replica -> {
+ secondaryNodeShardReplicas.computeIfAbsent(replica.getNode(), n ->
new HashMap<>())
+ .computeIfAbsent(replica.getShard().getShardName(), s -> new
AtomicInteger())
+ .incrementAndGet();
+ }));
+
+ // find the colocated-with collections
+ Map<Node, Set<String>> colocatingNodes = new HashMap<>();
+ try {
+ for (String colocatedCollection : colocatedCollections) {
+ SolrCollection coll = cluster.getCollection(colocatedCollection);
+ coll.shards().forEach(shard ->
+ shard.replicas().forEach(replica -> {
+ colocatingNodes.computeIfAbsent(replica.getNode(), n -> new
HashSet<>())
+ .add(coll.getName());
+ }));
+ }
+ } catch (IOException ioe) {
+ throw new PlacementModificationException("failed to retrieve colocated
collection information", ioe);
+ }
+ PlacementModificationException exception = null;
+ for (Replica replica : deleteReplicasRequest.getReplicas()) {
+ if (!colocatingNodes.containsKey(replica.getNode())) {
+ continue;
+ }
+ // check that there will be at least one replica remaining
+ AtomicInteger secondaryCount = secondaryNodeShardReplicas
+ .getOrDefault(replica.getNode(), Map.of())
+ .getOrDefault(replica.getShard().getShardName(), new
AtomicInteger());
+ if (secondaryCount.get() > 1) {
+ // we can delete it - record the deletion
+ secondaryCount.decrementAndGet();
+ continue;
+ }
+ // fail - this replica cannot be removed
+ if (exception == null) {
+ exception = new PlacementModificationException("delete replica(s)
rejected");
+ }
+ exception.addRejectedModification(replica.toString(), "co-located with
replicas of " + colocatingNodes.get(replica.getNode()));
Review comment:
Add the problematic node in the rejection reason so that client can
delete the colocated collection replicas there to fix the issue.
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
##########
@@ -238,11 +258,93 @@ public PlacementPlan computePlacement(Cluster cluster,
PlacementRequest request,
// failure. Current code does fail if placement is impossible
(constraint is at most one replica of a shard on any node).
for (Replica.ReplicaType replicaType : Replica.ReplicaType.values()) {
makePlacementDecisions(solrCollection, shardName, availabilityZones,
replicaType, request.getCountReplicasToCreate(replicaType),
- attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementPlanFactory, replicaPlacements);
+ attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementContext.getPlacementPlanFactory(), replicaPlacements);
}
}
- return placementPlanFactory.createPlacementPlan(request,
replicaPlacements);
+ return
placementContext.getPlacementPlanFactory().createPlacementPlan(request,
replicaPlacements);
+ }
+
+ @Override
+ public void verifyAllowedModification(ModificationRequest
modificationRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ if (modificationRequest instanceof DeleteShardsRequest) {
+ throw new UnsupportedOperationException("not implemented yet");
+ } else if (modificationRequest instanceof DeleteCollectionRequest) {
+ verifyDeleteCollection((DeleteCollectionRequest) modificationRequest,
placementContext);
+ } else if (modificationRequest instanceof DeleteReplicasRequest) {
+ verifyDeleteReplicas((DeleteReplicasRequest) modificationRequest,
placementContext);
+ } else {
+ throw new UnsupportedOperationException("unsupported request type " +
modificationRequest.getClass().getName());
+ }
+ }
+
+ private void verifyDeleteCollection(DeleteCollectionRequest
deleteCollectionRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ Cluster cluster = placementContext.getCluster();
+ Set<String> colocatedCollections =
colocatedWith.getOrDefault(deleteCollectionRequest.getCollection().getName(),
Set.of());
+ for (String primaryName : colocatedCollections) {
+ try {
+ if (cluster.getCollection(primaryName) != null) {
+ // still exists
+ throw new PlacementModificationException("colocated collection " +
primaryName + " still present");
Review comment:
The exception should also tell which collection we refuse to delete
(unless that context will be added higher up the call stack?).
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/impl/ModificationRequestImpl.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cluster.placement.impl;
+
+import org.apache.solr.cluster.Replica;
+import org.apache.solr.cluster.Shard;
+import org.apache.solr.cluster.SolrCollection;
+import org.apache.solr.cluster.placement.DeleteCollectionRequest;
+import org.apache.solr.cluster.placement.DeleteReplicasRequest;
+import org.apache.solr.cluster.placement.DeleteShardsRequest;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.Slice;
+
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Helper class to create modification request instances.
+ */
+public class ModificationRequestImpl {
+
+ public static DeleteCollectionRequest deleteCollectionRequest(DocCollection
docCollection) {
+ SolrCollection solrCollection =
SimpleClusterAbstractionsImpl.SolrCollectionImpl.fromDocCollection(docCollection);
+ return () -> solrCollection;
+ }
+
+ /**
+ * Create a delete replicas request.
+ * @param collection collection to delete replicas from
+ * @param replicas replicas to delete
+ */
+ public static DeleteReplicasRequest deleteReplicasRequest(SolrCollection
collection, Set<Replica> replicas) {
Review comment:
Method should be called `createDeleteReplicasRequest`?
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
##########
@@ -43,14 +46,28 @@
@JsonProperty
public long prioritizedFreeDiskGB;
+ /**
+ * This property defines an additional constraint that primary collections
(keys) should be
+ * located on the same nodes as the secondary collections (values). The
plugin will assume
+ * that the secondary collection replicas are already in place and ignore
candidate nodes where
+ * they are not already present.
+ */
+ @JsonProperty
+ public Map<String, String> withCollections;
+
// no-arg public constructor required for deserialization
public AffinityPlacementConfig() {
- minimalFreeDiskGB = 20L;
- prioritizedFreeDiskGB = 100L;
+ this (20L, 100L, Map.of());
Review comment:
minor: remove space after `this`.
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/impl/ModificationRequestImpl.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cluster.placement.impl;
+
+import org.apache.solr.cluster.Replica;
+import org.apache.solr.cluster.Shard;
+import org.apache.solr.cluster.SolrCollection;
+import org.apache.solr.cluster.placement.DeleteCollectionRequest;
+import org.apache.solr.cluster.placement.DeleteReplicasRequest;
+import org.apache.solr.cluster.placement.DeleteShardsRequest;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.Slice;
+
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Helper class to create modification request instances.
+ */
+public class ModificationRequestImpl {
+
+ public static DeleteCollectionRequest deleteCollectionRequest(DocCollection
docCollection) {
+ SolrCollection solrCollection =
SimpleClusterAbstractionsImpl.SolrCollectionImpl.fromDocCollection(docCollection);
+ return () -> solrCollection;
+ }
+
+ /**
+ * Create a delete replicas request.
+ * @param collection collection to delete replicas from
+ * @param replicas replicas to delete
+ */
+ public static DeleteReplicasRequest deleteReplicasRequest(SolrCollection
collection, Set<Replica> replicas) {
+ return new DeleteReplicasRequest() {
+ @Override
+ public Set<Replica> getReplicas() {
+ return replicas;
+ }
+
+ @Override
+ public SolrCollection getCollection() {
+ return collection;
+ }
+
+ @Override
+ public String toString() {
+ return "DeleteReplicasRequest{collection=" + collection.getName() +
+ ",replicas=" + replicas;
+ }
+ };
+ }
+
+ /**
+ * Create a delete replicas request using the internal Solr API.
+ * @param docCollection Solr collection
+ * @param shardName shard name
+ * @param replicaNames replica names (aka. core-node names)
+ */
+ public static DeleteReplicasRequest deleteReplicasRequest(DocCollection
docCollection, String shardName, Set<String> replicaNames) {
Review comment:
The signature is not consistent with `deleteReplicasRequest` above that
takes `Replica` instances rather than `String` names. I prefer the strongly
typed signature.
##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -381,7 +382,11 @@ public AssignmentException(String message, Throwable
cause, boolean enableSuppre
public interface AssignStrategy {
List<ReplicaPosition> assign(SolrCloudManager solrCloudManager,
AssignRequest assignRequest)
- throws Assign.AssignmentException, IOException, InterruptedException;
+ throws AssignmentException, IOException, InterruptedException;
+ void verifyDeleteCollection(SolrCloudManager solrCloudManager,
DocCollection collection)
Review comment:
Maybe add default implementation here to not force empty implementation
in legacy or other places?
I'd add javadoc at least for added methods...
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java
##########
@@ -238,11 +258,93 @@ public PlacementPlan computePlacement(Cluster cluster,
PlacementRequest request,
// failure. Current code does fail if placement is impossible
(constraint is at most one replica of a shard on any node).
for (Replica.ReplicaType replicaType : Replica.ReplicaType.values()) {
makePlacementDecisions(solrCollection, shardName, availabilityZones,
replicaType, request.getCountReplicasToCreate(replicaType),
- attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementPlanFactory, replicaPlacements);
+ attrValues, replicaTypeToNodes, nodesWithReplicas, coresOnNodes,
placementContext.getPlacementPlanFactory(), replicaPlacements);
}
}
- return placementPlanFactory.createPlacementPlan(request,
replicaPlacements);
+ return
placementContext.getPlacementPlanFactory().createPlacementPlan(request,
replicaPlacements);
+ }
+
+ @Override
+ public void verifyAllowedModification(ModificationRequest
modificationRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ if (modificationRequest instanceof DeleteShardsRequest) {
+ throw new UnsupportedOperationException("not implemented yet");
+ } else if (modificationRequest instanceof DeleteCollectionRequest) {
+ verifyDeleteCollection((DeleteCollectionRequest) modificationRequest,
placementContext);
+ } else if (modificationRequest instanceof DeleteReplicasRequest) {
+ verifyDeleteReplicas((DeleteReplicasRequest) modificationRequest,
placementContext);
+ } else {
+ throw new UnsupportedOperationException("unsupported request type " +
modificationRequest.getClass().getName());
+ }
+ }
+
+ private void verifyDeleteCollection(DeleteCollectionRequest
deleteCollectionRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ Cluster cluster = placementContext.getCluster();
+ Set<String> colocatedCollections =
colocatedWith.getOrDefault(deleteCollectionRequest.getCollection().getName(),
Set.of());
+ for (String primaryName : colocatedCollections) {
+ try {
+ if (cluster.getCollection(primaryName) != null) {
+ // still exists
+ throw new PlacementModificationException("colocated collection " +
primaryName + " still present");
+ }
+ } catch (IOException e) {
+ throw new PlacementModificationException("failed to retrieve
colocated collection information", e);
+ }
+ }
+ }
+
+ private void verifyDeleteReplicas(DeleteReplicasRequest
deleteReplicasRequest, PlacementContext placementContext) throws
PlacementModificationException, InterruptedException {
+ Cluster cluster = placementContext.getCluster();
+ SolrCollection secondaryCollection =
deleteReplicasRequest.getCollection();
+ Set<String> colocatedCollections =
colocatedWith.get(secondaryCollection.getName());
+ if (colocatedCollections == null) {
+ return;
+ }
+ Map<Node, Map<String, AtomicInteger>> secondaryNodeShardReplicas = new
HashMap<>();
+ secondaryCollection.shards().forEach(shard ->
+ shard.replicas().forEach(replica -> {
+ secondaryNodeShardReplicas.computeIfAbsent(replica.getNode(), n ->
new HashMap<>())
+ .computeIfAbsent(replica.getShard().getShardName(), s -> new
AtomicInteger())
+ .incrementAndGet();
+ }));
+
+ // find the colocated-with collections
+ Map<Node, Set<String>> colocatingNodes = new HashMap<>();
+ try {
+ for (String colocatedCollection : colocatedCollections) {
+ SolrCollection coll = cluster.getCollection(colocatedCollection);
+ coll.shards().forEach(shard ->
+ shard.replicas().forEach(replica -> {
+ colocatingNodes.computeIfAbsent(replica.getNode(), n -> new
HashSet<>())
+ .add(coll.getName());
+ }));
+ }
+ } catch (IOException ioe) {
+ throw new PlacementModificationException("failed to retrieve colocated
collection information", ioe);
+ }
+ PlacementModificationException exception = null;
+ for (Replica replica : deleteReplicasRequest.getReplicas()) {
+ if (!colocatingNodes.containsKey(replica.getNode())) {
+ continue;
+ }
+ // check that there will be at least one replica remaining
+ AtomicInteger secondaryCount = secondaryNodeShardReplicas
+ .getOrDefault(replica.getNode(), Map.of())
+ .getOrDefault(replica.getShard().getShardName(), new
AtomicInteger());
+ if (secondaryCount.get() > 1) {
+ // we can delete it - record the deletion
+ secondaryCount.decrementAndGet();
+ continue;
+ }
+ // fail - this replica cannot be removed
+ if (exception == null) {
+ exception = new PlacementModificationException("delete replica(s)
rejected");
+ }
+ exception.addRejectedModification(replica.toString(), "co-located with
replicas of " + colocatingNodes.get(replica.getNode()));
Review comment:
We do not support in Solr returning errors from Collection API calls in
a way that is consumable by code, right? Would be a nice addition...
##########
File path:
solr/core/src/java/org/apache/solr/cluster/placement/impl/ModificationRequestImpl.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cluster.placement.impl;
+
+import org.apache.solr.cluster.Replica;
+import org.apache.solr.cluster.Shard;
+import org.apache.solr.cluster.SolrCollection;
+import org.apache.solr.cluster.placement.DeleteCollectionRequest;
+import org.apache.solr.cluster.placement.DeleteReplicasRequest;
+import org.apache.solr.cluster.placement.DeleteShardsRequest;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.Slice;
+
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Helper class to create modification request instances.
+ */
+public class ModificationRequestImpl {
+
+ public static DeleteCollectionRequest deleteCollectionRequest(DocCollection
docCollection) {
+ SolrCollection solrCollection =
SimpleClusterAbstractionsImpl.SolrCollectionImpl.fromDocCollection(docCollection);
+ return () -> solrCollection;
+ }
+
+ /**
+ * Create a delete replicas request.
+ * @param collection collection to delete replicas from
+ * @param replicas replicas to delete
+ */
+ public static DeleteReplicasRequest deleteReplicasRequest(SolrCollection
collection, Set<Replica> replicas) {
+ return new DeleteReplicasRequest() {
+ @Override
+ public Set<Replica> getReplicas() {
+ return replicas;
+ }
+
+ @Override
+ public SolrCollection getCollection() {
+ return collection;
+ }
+
+ @Override
+ public String toString() {
+ return "DeleteReplicasRequest{collection=" + collection.getName() +
+ ",replicas=" + replicas;
+ }
+ };
+ }
+
+ /**
+ * Create a delete replicas request using the internal Solr API.
+ * @param docCollection Solr collection
+ * @param shardName shard name
+ * @param replicaNames replica names (aka. core-node names)
+ */
+ public static DeleteReplicasRequest deleteReplicasRequest(DocCollection
docCollection, String shardName, Set<String> replicaNames) {
+ SolrCollection solrCollection =
SimpleClusterAbstractionsImpl.SolrCollectionImpl.fromDocCollection(docCollection);
+ Shard shard = solrCollection.getShard(shardName);
+ Slice slice = docCollection.getSlice(shardName);
+ Set<Replica> solrReplicas = new HashSet<>();
+ replicaNames.forEach(name -> solrReplicas.add(shard.getReplica(name)));
+ return deleteReplicasRequest(solrCollection, solrReplicas);
+ }
+
+
+ public static DeleteShardsRequest deleteShardsRequest(SolrCollection
collection, Set<String> shardNames) {
Review comment:
This and next method should really be called `createDeleteShardsRequest`
to convey what they really do, right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
