lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a.@apache.org
Subject [lucene-solr] branch jira/solr-15055-2 updated: SOLR-15055: More refactoring after review.
Date Tue, 19 Jan 2021 18:04:22 GMT
This is an automated email from the ASF dual-hosted git repository.

ab pushed a commit to branch jira/solr-15055-2
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/jira/solr-15055-2 by this push:
     new cb82e2f  SOLR-15055: More refactoring after review.
cb82e2f is described below

commit cb82e2f13dada9afd60af2259fb2d6da2d68bb5a
Author: Andrzej Bialecki <ab@apache.org>
AuthorDate: Tue Jan 19 19:03:59 2021 +0100

    SOLR-15055: More refactoring after review.
---
 .../apache/solr/cloud/ExclusiveSliceProperty.java  |  2 +-
 .../apache/solr/cloud/api/collections/Assign.java  | 17 +++++-
 .../cloud/api/collections/DeleteCollectionCmd.java | 20 ++-----
 .../cloud/api/collections/DeleteReplicaCmd.java    | 62 ++++++--------------
 .../apache/solr/cloud/overseer/ReplicaMutator.java |  2 +-
 .../cluster/placement/DeleteCollectionRequest.java | 23 ++++++++
 .../placement/impl/ModificationRequestImpl.java    | 12 ++--
 .../impl/PlacementPluginAssignStrategy.java        | 27 ++++++++-
 .../impl/SimpleClusterAbstractionsImpl.java        |  6 +-
 ...xtImpl.java => SimplePlacementContextImpl.java} |  4 +-
 .../placement/plugins/AffinityPlacementConfig.java |  4 +-
 .../plugins/AffinityPlacementFactory.java          | 68 ++++++++++++++--------
 .../impl/PlacementPluginIntegrationTest.java       |  2 +-
 13 files changed, 147 insertions(+), 102 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/ExclusiveSliceProperty.java b/solr/core/src/java/org/apache/solr/cloud/ExclusiveSliceProperty.java
index d17976e..01592ff 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ExclusiveSliceProperty.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ExclusiveSliceProperty.java
@@ -74,7 +74,7 @@ class ExclusiveSliceProperty {
   ExclusiveSliceProperty(ClusterState clusterState, ZkNodeProps message) {
     this.clusterState = clusterState;
     String tmp = message.getStr(ZkStateReader.PROPERTY_PROP);
-    if (StringUtils.startsWith(tmp, CollectionAdminParams.PROPERTY_PREFIX) == false) {
+    if (!StringUtils.startsWith(tmp, CollectionAdminParams.PROPERTY_PREFIX)) {
       tmp = CollectionAdminParams.PROPERTY_PREFIX + tmp;
     }
     this.property = tmp.toLowerCase(Locale.ROOT);
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
index 786bfa9..d0d1f88 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
@@ -40,6 +40,7 @@ import org.apache.solr.client.solrj.cloud.SolrCloudManager;
 import org.apache.solr.client.solrj.cloud.AlreadyExistsException;
 import org.apache.solr.client.solrj.cloud.BadVersionException;
 import org.apache.solr.client.solrj.cloud.VersionedData;
+import org.apache.solr.cluster.placement.ModificationRequest;
 import org.apache.solr.cluster.placement.PlacementPlugin;
 import org.apache.solr.cluster.placement.impl.PlacementPluginAssignStrategy;
 import org.apache.solr.common.SolrException;
@@ -381,7 +382,11 @@ public class Assign {
 
   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)
+        throws AssignmentException, IOException, InterruptedException;
+    void verifyDeleteReplicas(SolrCloudManager solrCloudManager, DocCollection collection,
String shardId, Set<String> replicaNames)
+        throws AssignmentException, IOException, InterruptedException;
   }
 
   public static class AssignRequest {
@@ -479,6 +484,16 @@ public class Assign {
       return result;
     }
 
+    @Override
+    public void verifyDeleteCollection(SolrCloudManager solrCloudManager, DocCollection collection)
throws AssignmentException, IOException, InterruptedException {
+      // no-op
+    }
+
+    @Override
+    public void verifyDeleteReplicas(SolrCloudManager solrCloudManager, DocCollection collection,
String shardId, Set<String> replicaNames) throws AssignmentException, IOException, InterruptedException
{
+      // no-op
+    }
+
     // keeps this big ugly construction block out of otherwise legible code
     private ImmutableMap<Replica.Type, Integer> countsPerReplicaType(AssignRequest
assignRequest) {
       return ImmutableMap.of(
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 7674577..4f189db 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
@@ -29,18 +29,13 @@ import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
 import org.apache.solr.cloud.Overseer;
-import org.apache.solr.cluster.placement.DeleteReplicasRequest;
-import org.apache.solr.cluster.placement.PlacementContext;
 import org.apache.solr.cluster.placement.PlacementPlugin;
-import org.apache.solr.cluster.placement.impl.ModificationRequestImpl;
-import org.apache.solr.cluster.placement.impl.PlacementContextImpl;
 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.Replica;
-import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
@@ -99,17 +94,10 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
     }
 
     PlacementPlugin placementPlugin = ocmh.overseer.getCoreContainer().getPlacementPluginFactory().createPluginInstance();
-    if (placementPlugin != null) {
-      // verify the placement modifications caused by the deletion are allowed
-      PlacementContext placementContext = new PlacementContextImpl(ocmh.cloudManager);
-      DocCollection coll = state.getCollection(collection);
-      for (Slice slice : coll.getActiveSlices()) {
-        DeleteReplicasRequest deleteReplicasRequest = ModificationRequestImpl
-            .deleteReplicasRequest(coll, slice.getName(),
-                slice.getReplicas().stream().map(Replica::getName).collect(Collectors.toSet()));
-        placementPlugin.verifyAllowedModification(deleteReplicasRequest, placementContext);
-      }
-    }
+    // verify the placement modifications caused by the deletion are allowed
+    DocCollection coll = state.getCollection(collection);
+    Assign.AssignStrategy assignStrategy = Assign.createAssignStrategy(placementPlugin, state,
coll);
+    assignStrategy.verifyDeleteCollection(ocmh.cloudManager, coll);
 
     final boolean deleteHistory = message.getBool(CoreAdminParams.DELETE_METRICS_HISTORY,
true);
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteReplicaCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteReplicaCmd.java
index f5cf6db..1d4eede 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteReplicaCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteReplicaCmd.java
@@ -35,14 +35,7 @@ import java.util.concurrent.Callable;
 
 import org.apache.solr.cloud.api.collections.OverseerCollectionMessageHandler.Cmd;
 import org.apache.solr.cloud.api.collections.OverseerCollectionMessageHandler.ShardRequestTracker;
-import org.apache.solr.cluster.placement.AttributeFetcher;
-import org.apache.solr.cluster.placement.DeleteReplicasRequest;
-import org.apache.solr.cluster.placement.PlacementContext;
-import org.apache.solr.cluster.placement.PlacementException;
 import org.apache.solr.cluster.placement.PlacementPlugin;
-import org.apache.solr.cluster.placement.impl.AttributeFetcherImpl;
-import org.apache.solr.cluster.placement.impl.ModificationRequestImpl;
-import org.apache.solr.cluster.placement.impl.PlacementContextImpl;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.ClusterState;
 import org.apache.solr.common.cloud.DocCollection;
@@ -85,15 +78,9 @@ public class DeleteReplicaCmd implements Cmd {
     boolean parallel = message.getBool("parallel", false);
 
     PlacementPlugin placementPlugin = ocmh.overseer.getCoreContainer().getPlacementPluginFactory().createPluginInstance();
-    PlacementContext placementContext = new PlacementContextImpl(ocmh.cloudManager);
     //If a count is specified the strategy needs be different
     if (message.getStr(COUNT_PROP) != null) {
-      try {
-        deleteReplicaBasedOnCount(clusterState, message, results, onComplete, parallel, placementPlugin,
placementContext);
-      } catch (PlacementException pe) {
-        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
-            "Delete replica(s) rejected by replica placement plugin: " + pe.toString(), pe);
-      }
+      deleteReplicaBasedOnCount(clusterState, message, results, onComplete, parallel, placementPlugin);
       return;
     }
 
@@ -117,14 +104,7 @@ public class DeleteReplicaCmd implements Cmd {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
               "Invalid shard name : " +  shard + " in collection : " +  collectionName);
     }
-
-    try {
-      deleteCore(coll, shard, replicaName, message, results, onComplete, parallel, placementPlugin,
placementContext);
-    } catch (PlacementException pe) {
-      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
-          "Delete replica rejected by replica placement plugin: " + pe.toString(), pe);
-    }
-
+    deleteCore(clusterState, coll, shard, replicaName, message, results, onComplete, parallel,
placementPlugin);
   }
 
 
@@ -138,9 +118,8 @@ public class DeleteReplicaCmd implements Cmd {
                                  @SuppressWarnings({"rawtypes"})NamedList results,
                                  Runnable onComplete,
                                  boolean parallel,
-                                 PlacementPlugin placementPlugin,
-                                 PlacementContext placementContext)
-          throws KeeperException, PlacementException, InterruptedException {
+                                 PlacementPlugin placementPlugin)
+          throws KeeperException, IOException, InterruptedException {
     ocmh.checkRequired(message, COLLECTION_PROP, COUNT_PROP);
     int count = Integer.parseInt(message.getStr(COUNT_PROP));
     String collectionName = message.getStr(COLLECTION_PROP);
@@ -170,16 +149,13 @@ public class DeleteReplicaCmd implements Cmd {
       }
     }
 
-    if (placementPlugin != null) {
-      // verify that all replicas can be deleted
-      for (Map.Entry<Slice, Set<String>> entry : shardToReplicasMapping.entrySet())
{
-        Slice shardSlice = entry.getKey();
-        String shardId = shardSlice.getName();
-        Set<String> replicas = entry.getValue();
-        //verify all replicas
-        DeleteReplicasRequest deleteReplicasRequest = ModificationRequestImpl.deleteReplicasRequest(coll,
shardId, replicas);
-        placementPlugin.verifyAllowedModification(deleteReplicasRequest, placementContext);
-      }
+    // verify that all replicas can be deleted
+    Assign.AssignStrategy assignStrategy = Assign.createAssignStrategy(placementPlugin, clusterState,
coll);
+    for (Map.Entry<Slice, Set<String>> entry : shardToReplicasMapping.entrySet())
{
+      Slice shardSlice = entry.getKey();
+      String shardId = shardSlice.getName();
+      Set<String> replicas = entry.getValue();
+      assignStrategy.verifyDeleteReplicas(ocmh.cloudManager, coll, shardId, replicas);
     }
 
     for (Map.Entry<Slice, Set<String>> entry : shardToReplicasMapping.entrySet())
{
@@ -190,7 +166,7 @@ public class DeleteReplicaCmd implements Cmd {
       for (String replica: replicas) {
         log.debug("Deleting replica {}  for shard {} based on count {}", replica, shardId,
count);
         // don't verify with the placement plugin - we already did it
-        deleteCore(coll, shardId, replica, message, results, onComplete, parallel, null,
null);
+        deleteCore(clusterState, coll, shardId, replica, message, results, onComplete, parallel,
null);
       }
       results.add("shard_id", shardId);
       results.add("replicas_deleted", replicas);
@@ -248,15 +224,14 @@ public class DeleteReplicaCmd implements Cmd {
   }
 
   @SuppressWarnings({"unchecked"})
-  void deleteCore(DocCollection coll,
+  void deleteCore(ClusterState clusterState, DocCollection coll,
                   String shardId,
                   String replicaName,
                   ZkNodeProps message,
                   @SuppressWarnings({"rawtypes"})NamedList results,
                   Runnable onComplete,
                   boolean parallel,
-                  PlacementPlugin placementPlugin,
-                  PlacementContext placementContext) throws KeeperException, PlacementException,
InterruptedException {
+                  PlacementPlugin placementPlugin) throws KeeperException, IOException, InterruptedException
{
 
     Slice slice = coll.getSlice(shardId);
     Replica replica = slice.getReplica(replicaName);
@@ -276,11 +251,10 @@ public class DeleteReplicaCmd implements Cmd {
               " with onlyIfDown='true', but state is '" + replica.getStr(ZkStateReader.STATE_PROP)
+ "'");
     }
 
-    if (placementPlugin != null) {
-      // verify that we are allowed to delete this replica
-      DeleteReplicasRequest deleteReplicasRequest = ModificationRequestImpl.deleteReplicasRequest(coll,
shardId, Set.of(replicaName));
-      placementPlugin.verifyAllowedModification(deleteReplicasRequest, placementContext);
-    }
+    // verify that we are allowed to delete this replica
+    Assign.AssignStrategy assignStrategy = Assign.createAssignStrategy(placementPlugin, clusterState,
coll);
+    assignStrategy.verifyDeleteReplicas(ocmh.cloudManager, coll, shardId, Set.of(replicaName));
+
     ShardHandler shardHandler = ocmh.shardHandlerFactory.getShardHandler();
     String core = replica.getStr(ZkStateReader.CORE_NAME_PROP);
     String asyncId = message.getStr(ASYNC);
diff --git a/solr/core/src/java/org/apache/solr/cloud/overseer/ReplicaMutator.java b/solr/core/src/java/org/apache/solr/cloud/overseer/ReplicaMutator.java
index d386880..be8f07a 100644
--- a/solr/core/src/java/org/apache/solr/cloud/overseer/ReplicaMutator.java
+++ b/solr/core/src/java/org/apache/solr/cloud/overseer/ReplicaMutator.java
@@ -116,7 +116,7 @@ public class ReplicaMutator {
     String sliceName = message.getStr(ZkStateReader.SHARD_ID_PROP);
     String replicaName = message.getStr(ZkStateReader.REPLICA_PROP);
     String property = message.getStr(ZkStateReader.PROPERTY_PROP).toLowerCase(Locale.ROOT);
-    if (StringUtils.startsWith(property, CollectionAdminParams.PROPERTY_PREFIX) == false)
{
+    if (!StringUtils.startsWith(property, CollectionAdminParams.PROPERTY_PREFIX)) {
       property = CollectionAdminParams.PROPERTY_PREFIX + property;
     }
     property = property.toLowerCase(Locale.ROOT);
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/DeleteCollectionRequest.java
b/solr/core/src/java/org/apache/solr/cluster/placement/DeleteCollectionRequest.java
new file mode 100644
index 0000000..b5dabc5
--- /dev/null
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/DeleteCollectionRequest.java
@@ -0,0 +1,23 @@
+/*
+ * 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;
+
+/**
+ *
+ */
+public interface DeleteCollectionRequest extends ModificationRequest {
+}
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/ModificationRequestImpl.java
b/solr/core/src/java/org/apache/solr/cluster/placement/impl/ModificationRequestImpl.java
index 7a62e28..f6889d2 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/ModificationRequestImpl.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/ModificationRequestImpl.java
@@ -20,6 +20,7 @@ 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;
@@ -33,6 +34,11 @@ import java.util.Set;
  */
 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
@@ -70,11 +76,7 @@ public class ModificationRequestImpl {
     Shard shard = solrCollection.getShard(shardName);
     Slice slice = docCollection.getSlice(shardName);
     Set<Replica> solrReplicas = new HashSet<>();
-    replicaNames.forEach(name -> {
-      org.apache.solr.common.cloud.Replica replica = slice.getReplica(name);
-      Replica solrReplica = new SimpleClusterAbstractionsImpl.ReplicaImpl(replica.getName(),
shard, replica);
-      solrReplicas.add(solrReplica);
-    });
+    replicaNames.forEach(name -> solrReplicas.add(shard.getReplica(name)));
     return deleteReplicasRequest(solrCollection, solrReplicas);
   }
 
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginAssignStrategy.java
b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginAssignStrategy.java
index a226ab8..5b7c12f 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginAssignStrategy.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginAssignStrategy.java
@@ -19,10 +19,13 @@ package org.apache.solr.cluster.placement.impl;
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.solr.client.solrj.cloud.SolrCloudManager;
 import org.apache.solr.cloud.api.collections.Assign;
 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.PlacementContext;
 import org.apache.solr.cluster.placement.PlacementException;
 import org.apache.solr.cluster.placement.PlacementPlugin;
@@ -51,7 +54,7 @@ public class PlacementPluginAssignStrategy implements Assign.AssignStrategy
{
   public List<ReplicaPosition> assign(SolrCloudManager solrCloudManager, Assign.AssignRequest
assignRequest)
       throws Assign.AssignmentException, IOException, InterruptedException {
 
-    PlacementContext placementContext = new PlacementContextImpl(solrCloudManager);
+    PlacementContext placementContext = new SimplePlacementContextImpl(solrCloudManager);
     SolrCollection solrCollection = placementContext.getCluster().getCollection(collection.getName());
 
     PlacementRequestImpl placementRequest = PlacementRequestImpl.toPlacementRequest(placementContext.getCluster(),
solrCollection, assignRequest);
@@ -65,4 +68,26 @@ public class PlacementPluginAssignStrategy implements Assign.AssignStrategy
{
 
     return ReplicaPlacementImpl.toReplicaPositions(placementPlan.getReplicaPlacements());
   }
+
+  @Override
+  public void verifyDeleteCollection(SolrCloudManager solrCloudManager, DocCollection collection)
throws Assign.AssignmentException, IOException, InterruptedException {
+    PlacementContext placementContext = new SimplePlacementContextImpl(solrCloudManager);
+    DeleteCollectionRequest modificationRequest = ModificationRequestImpl.deleteCollectionRequest(collection);
+    try {
+      plugin.verifyAllowedModification(modificationRequest, placementContext);
+    } catch (PlacementException pe) {
+      throw new Assign.AssignmentException(pe);
+    }
+  }
+
+  @Override
+  public void verifyDeleteReplicas(SolrCloudManager solrCloudManager, DocCollection collection,
String shardId, Set<String> replicaNames) throws Assign.AssignmentException, IOException,
InterruptedException {
+    PlacementContext placementContext = new SimplePlacementContextImpl(solrCloudManager);
+    DeleteReplicasRequest modificationRequest = ModificationRequestImpl.deleteReplicasRequest(collection,
shardId, replicaNames);
+    try {
+      plugin.verifyAllowedModification(modificationRequest, placementContext);
+    } catch (PlacementException pe) {
+      throw new Assign.AssignmentException(pe);
+    }
+  }
 }
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
index 292fe5c..e26a374 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
@@ -324,7 +324,7 @@ class SimpleClusterAbstractionsImpl {
       return new Pair<>(replicas, leader);
     }
 
-    ReplicaImpl(String replicaName, Shard shard, org.apache.solr.common.cloud.Replica sliceReplica)
{
+    private ReplicaImpl(String replicaName, Shard shard, org.apache.solr.common.cloud.Replica
sliceReplica) {
       this.replicaName = replicaName;
       this.coreName = sliceReplica.getCoreName();
       this.shard = shard;
@@ -334,7 +334,7 @@ class SimpleClusterAbstractionsImpl {
       this.node = new NodeImpl(sliceReplica.getNodeName());
     }
 
-    Replica.ReplicaType translateType(org.apache.solr.common.cloud.Replica.Type type) {
+    private Replica.ReplicaType translateType(org.apache.solr.common.cloud.Replica.Type type)
{
       switch (type) {
         case NRT:
           return Replica.ReplicaType.NRT;
@@ -347,7 +347,7 @@ class SimpleClusterAbstractionsImpl {
       }
     }
 
-    Replica.ReplicaState translateState(org.apache.solr.common.cloud.Replica.State state)
{
+    private Replica.ReplicaState translateState(org.apache.solr.common.cloud.Replica.State
state) {
       switch (state) {
         case ACTIVE:
           return Replica.ReplicaState.ACTIVE;
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementContextImpl.java
b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimplePlacementContextImpl.java
similarity index 85%
rename from solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementContextImpl.java
rename to solr/core/src/java/org/apache/solr/cluster/placement/impl/SimplePlacementContextImpl.java
index eed649b..c04812c 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementContextImpl.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimplePlacementContextImpl.java
@@ -11,13 +11,13 @@ import java.io.IOException;
 /**
  *
  */
-public class PlacementContextImpl implements PlacementContext {
+public class SimplePlacementContextImpl implements PlacementContext {
 
   private final Cluster cluster;
   private final AttributeFetcher attributeFetcher;
   private final PlacementPlanFactory placementPlanFactory = new PlacementPlanFactoryImpl();
 
-  public PlacementContextImpl(SolrCloudManager solrCloudManager) throws IOException {
+  public SimplePlacementContextImpl(SolrCloudManager solrCloudManager) throws IOException
{
     cluster = new SimpleClusterAbstractionsImpl.ClusterImpl(solrCloudManager);
     attributeFetcher = new AttributeFetcherImpl(solrCloudManager);
   }
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
index 2866392..f091654 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
@@ -57,9 +57,7 @@ public class AffinityPlacementConfig implements PlacementPluginConfig {
 
   // no-arg public constructor required for deserialization
   public AffinityPlacementConfig() {
-    minimalFreeDiskGB = 20L;
-    prioritizedFreeDiskGB = 100L;
-    withCollections = Map.of();
+    this (20L, 100L, Map.of());
   }
 
   public AffinityPlacementConfig(long minimalFreeDiskGB, long prioritizedFreeDiskGB) {
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 ad75053..0ad62ac 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
@@ -174,7 +174,10 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
 
     private final long prioritizedFreeDiskGB;
 
+    // primary to secondary (1:1)
     private final Map<String, String> withCollections;
+    // secondary to primary (1:N)
+    private final Map<String, Set<String>> colocatedWith;
 
     private final Random replicaPlacementRandom = new Random(); // ok even if random sequence
is predictable.
 
@@ -184,7 +187,16 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
     private AffinityPlacementPlugin(long minimalFreeDiskGB, long prioritizedFreeDiskGB, Map<String,
String> withCollections) {
       this.minimalFreeDiskGB = minimalFreeDiskGB;
       this.prioritizedFreeDiskGB = prioritizedFreeDiskGB;
+      Objects.requireNonNull(withCollections, "withCollections must not be null");
       this.withCollections = withCollections;
+      if (withCollections.isEmpty()) {
+        colocatedWith = Map.of();
+      } else {
+        colocatedWith = new HashMap<>();
+        withCollections.forEach((primary, secondary) ->
+            colocatedWith.computeIfAbsent(secondary, s -> new HashSet<>())
+                .add(primary));
+      }
 
       // We make things reproducible in tests by using test seed if any
       String seed = System.getProperty("tests.seed");
@@ -256,14 +268,30 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
 
     @Override
     public void verifyAllowedModification(ModificationRequest modificationRequest, PlacementContext
placementContext) throws PlacementModificationException, InterruptedException {
+      Cluster cluster = placementContext.getCluster();
       if (modificationRequest instanceof DeleteShardsRequest) {
         throw new UnsupportedOperationException("not implemented yet");
+      } else if (modificationRequest instanceof DeleteCollectionRequest) {
+        DeleteCollectionRequest deleteCollectionRequest = (DeleteCollectionRequest) modificationRequest;
+        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);
+          }
+        }
+        return;
       } else if (!(modificationRequest instanceof DeleteReplicasRequest)) {
         throw new UnsupportedOperationException("unsupported request type " + modificationRequest.getClass().getName());
       }
       DeleteReplicasRequest request = (DeleteReplicasRequest) modificationRequest;
       SolrCollection secondaryCollection = request.getCollection();
-      if (!withCollections.values().contains(secondaryCollection.getName())) {
+      Set<String> colocatedCollections = colocatedWith.get(secondaryCollection.getName());
+      if (colocatedCollections == null) {
         return;
       }
       Map<Node, Map<String, AtomicInteger>> secondaryNodeShardReplicas = new
HashMap<>();
@@ -275,36 +303,28 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
           }));
 
       // find the colocated-with collections
-      Cluster cluster = placementContext.getCluster();
-      Set<SolrCollection> colocatedCollections = new HashSet<>();
-      AtomicReference<IOException> exc = new AtomicReference<>();
-      withCollections.forEach((primaryName, secondaryName) -> {
-        if (exc.get() != null) { // there were errors before
+      Map<Node, Set<String>> colocatingNodes = new HashMap<>();
+      AtomicReference<Exception> exc = new AtomicReference<>();
+      colocatedCollections.forEach(collName -> {
+        if (exc.get() != null) {
           return;
         }
-        if (secondaryCollection.getName().equals(secondaryName)) {
-          try {
-            SolrCollection primary = cluster.getCollection(primaryName);
-            if (primary != null) { // still exists
-              colocatedCollections.add(primary);
-            }
-          } catch (IOException e) {
-            // IO error, not a missing collection - fail
-            exc.set(e);
-            return;
-          }
+        SolrCollection coll;
+        try {
+          coll = cluster.getCollection(collName);
+        } catch (Exception e) {
+          exc.set(e);
+          return;
         }
+        coll.shards().forEach(shard ->
+            shard.replicas().forEach(replica -> {
+              colocatingNodes.computeIfAbsent(replica.getNode(), n -> new HashSet<>())
+                  .add(coll.getName());
+            }));
       });
       if (exc.get() != null) {
         throw new PlacementModificationException("failed to retrieve colocated collection
information", exc.get());
       }
-      Map<Node, Set<String>> colocatingNodes = new HashMap<>();
-      colocatedCollections.forEach(coll ->
-          coll.shards().forEach(shard ->
-              shard.replicas().forEach(replica -> {
-                colocatingNodes.computeIfAbsent(replica.getNode(), n -> new HashSet<>())
-                    .add(coll.getName());
-              })));
       PlacementModificationException exception = null;
       for (Replica replica : request.getReplicas()) {
         if (!colocatingNodes.containsKey(replica.getNode())) {
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
b/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
index e0dfc46..199d779 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/impl/PlacementPluginIntegrationTest.java
@@ -310,7 +310,7 @@ public class PlacementPluginIntegrationTest extends SolrCloudTestCase
{
           .process(cluster.getSolrClient());
       fail("should have failed: " + rsp);
     } catch (Exception e) {
-      assertTrue(e.toString(), e.toString().contains("delete replica(s) rejected"));
+      assertTrue(e.toString(), e.toString().contains("colocated collection"));
     }
   }
 


Mime
View raw message