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/13 21:03:11 UTC

[lucene-solr] branch jira/solr-15055-2 updated: SOLR-15055: More API changes, start implementing the verify* method + unit test.

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 7b8baef  SOLR-15055: More API changes, start implementing the verify* method + unit test.
7b8baef is described below

commit 7b8baefda09bdceceb3c68a2075b8cd10a69140f
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Wed Jan 13 22:02:34 2021 +0100

    SOLR-15055: More API changes, start implementing the verify* method + unit test.
---
 .../placement/PlacementModificationException.java  | 52 +++++++++++++++++
 .../solr/cluster/placement/PlacementPlugin.java    |  2 +-
 .../placement/impl/ModificationRequestImpl.java    | 12 ++++
 .../plugins/AffinityPlacementFactory.java          | 68 +++++++++++++++++++++-
 .../plugins/MinimizeCoresPlacementFactory.java     |  2 +-
 .../placement/plugins/RandomPlacementFactory.java  |  2 +-
 .../plugins/AffinityPlacementFactoryTest.java      | 52 ++++++++++++++++-
 7 files changed, 183 insertions(+), 7 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/PlacementModificationException.java b/solr/core/src/java/org/apache/solr/cluster/placement/PlacementModificationException.java
new file mode 100644
index 0000000..19a7dd7
--- /dev/null
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/PlacementModificationException.java
@@ -0,0 +1,52 @@
+package org.apache.solr.cluster.placement;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ *
+ */
+public class PlacementModificationException extends PlacementException {
+  private final Map<String, String> rejectedModifications = new HashMap<>();
+
+  public PlacementModificationException() {
+    super();
+  }
+
+  public PlacementModificationException(String message) {
+    super(message);
+  }
+
+  public PlacementModificationException(String message, Throwable cause) {
+    super(message, cause);
+  }
+
+  public PlacementModificationException(Throwable cause) {
+    super(cause);
+  }
+
+  public void addRejectedModification(String modification, String reason) {
+    rejectedModifications.put(modification, reason);
+  }
+
+  public Map<String, String> getRejectedModifications() {
+    return rejectedModifications;
+  }
+
+  @Override
+  public String toString() {
+    StringBuilder sb = new StringBuilder(super.toString());
+    if (!rejectedModifications.isEmpty()) {
+        sb.append(": ")
+          .append(rejectedModifications.size())
+          .append(" rejections:");
+      rejectedModifications.forEach((modification, reason) ->
+          sb.append("\n")
+              .append(modification)
+              .append("\t")
+              .append(reason));
+
+    }
+    return sb.toString();
+  }
+}
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java b/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
index c1e4c35..c7e2ca8 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java
@@ -40,5 +40,5 @@ public interface PlacementPlugin {
   PlacementPlan computePlacement(PlacementRequest placementRequest, PlacementContext placementContext) throws PlacementException, InterruptedException;
 
   void verifyAllowedModification(ModificationRequest modificationRequest, PlacementContext placementContext)
-    throws PlacementException, InterruptedException;
+    throws PlacementModificationException, InterruptedException;
 }
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 5161869..8ae4935 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
@@ -44,6 +44,12 @@ public class ModificationRequestImpl {
       public SolrCollection getCollection() {
         return collection;
       }
+
+      @Override
+      public String toString() {
+        return "DeleteReplicasRequest{collection=" + collection.getName() +
+            ",replicas=" + replicas;
+      }
     };
   }
 
@@ -72,6 +78,12 @@ public class ModificationRequestImpl {
       public SolrCollection getCollection() {
         return collection;
       }
+
+      @Override
+      public String toString() {
+        return "DeleteShardsRequest{collection=" + collection.getName() +
+            ",shards=" + shardNames;
+      }
     };
   }
 
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 8a561c3..799562e 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
@@ -27,8 +27,10 @@ import org.apache.solr.common.util.SuppressForbidden;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.util.*;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
 /**
@@ -252,15 +254,75 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
     }
 
     @Override
-    public void verifyAllowedModification(ModificationRequest modificationRequest, PlacementContext placementContext) throws PlacementException, InterruptedException {
+    public void verifyAllowedModification(ModificationRequest modificationRequest, PlacementContext placementContext) throws PlacementModificationException, InterruptedException {
       if (modificationRequest instanceof DeleteShardsRequest) {
         throw new UnsupportedOperationException("not implemented yet");
       } else if (!(modificationRequest instanceof DeleteReplicasRequest)) {
         throw new UnsupportedOperationException("unsupported request type " + modificationRequest.getClass().getName());
       }
       DeleteReplicasRequest request = (DeleteReplicasRequest) modificationRequest;
-
-      // XXX to be completed...
+      SolrCollection secondaryCollection = request.getCollection();
+      if (!withCollections.values().contains(secondaryCollection.getName())) {
+        return;
+      }
+      // 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
+          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;
+          }
+        }
+      });
+      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())) {
+          continue;
+        }
+        // check that this is the only replica of this shard on this node
+        Shard shard = replica.getShard();
+        boolean hasOtherReplicas = false;
+        for (Replica otherReplica : shard.replicas()) {
+          if (otherReplica.getNode().equals(replica.getNode())) {
+            hasOtherReplicas = true;
+            break;
+          }
+        }
+        // ok to delete when at least one remains on this node
+        if (hasOtherReplicas) {
+          return;
+        }
+        // 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()));
+      }
+      if (exception != null) {
+        throw exception;
+      }
     }
 
     private Set<String> getZonesFromNodes(Set<Node> nodes, final AttributeValues attrValues) {
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/MinimizeCoresPlacementFactory.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/MinimizeCoresPlacementFactory.java
index d8e1370..b28b518 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/MinimizeCoresPlacementFactory.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/MinimizeCoresPlacementFactory.java
@@ -115,7 +115,7 @@ public class MinimizeCoresPlacementFactory implements PlacementPluginFactory<Pla
     }
 
     @Override
-    public void verifyAllowedModification(ModificationRequest modificationRequest, PlacementContext placementContext) throws PlacementException, InterruptedException {
+    public void verifyAllowedModification(ModificationRequest modificationRequest, PlacementContext placementContext) throws PlacementModificationException, InterruptedException {
       // no-op
     }
 
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/RandomPlacementFactory.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/RandomPlacementFactory.java
index 065353a..3043d2c 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/RandomPlacementFactory.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/RandomPlacementFactory.java
@@ -81,7 +81,7 @@ public class RandomPlacementFactory implements PlacementPluginFactory<PlacementP
     }
 
     @Override
-    public void verifyAllowedModification(ModificationRequest modificationRequest, PlacementContext placementContext) throws PlacementException, InterruptedException {
+    public void verifyAllowedModification(ModificationRequest modificationRequest, PlacementContext placementContext) throws PlacementModificationException, InterruptedException {
       // no-op
     }
 
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactoryTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactoryTest.java
index c5cfe2e..b0e28aa 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactoryTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactoryTest.java
@@ -25,6 +25,7 @@ import org.apache.solr.cluster.Shard;
 import org.apache.solr.cluster.SolrCollection;
 import org.apache.solr.cluster.placement.*;
 import org.apache.solr.cluster.placement.Builders;
+import org.apache.solr.cluster.placement.impl.ModificationRequestImpl;
 import org.apache.solr.cluster.placement.impl.PlacementPlanFactoryImpl;
 import org.apache.solr.cluster.placement.impl.PlacementRequestImpl;
 import org.apache.solr.common.util.Pair;
@@ -652,7 +653,7 @@ public class AffinityPlacementFactoryTest extends SolrTestCaseJ4 {
   }
 
   @Test
-  public void testWithCollectionConstraints() throws Exception {
+  public void testWithCollectionPlacement() throws Exception {
     int NUM_NODES = 3;
     Builders.ClusterBuilder clusterBuilder = Builders.newClusterBuilder().initializeLiveNodes(NUM_NODES);
     Builders.CollectionBuilder collectionBuilder = Builders.newCollectionBuilder(secondaryCollectionName);
@@ -693,6 +694,55 @@ public class AffinityPlacementFactoryTest extends SolrTestCaseJ4 {
     }
   }
 
+  @Test
+  public void testWithCollectionModificationRejected() throws Exception {
+    int NUM_NODES = 2;
+    Builders.ClusterBuilder clusterBuilder = Builders.newClusterBuilder().initializeLiveNodes(NUM_NODES);
+    Builders.CollectionBuilder collectionBuilder = Builders.newCollectionBuilder(secondaryCollectionName);
+    collectionBuilder.initializeShardsReplicas(1, 4, 0, 0, clusterBuilder.getLiveNodeBuilders());
+    clusterBuilder.addCollection(collectionBuilder);
+
+    collectionBuilder = Builders.newCollectionBuilder(primaryCollectionName);
+    collectionBuilder.initializeShardsReplicas(2, 2, 0, 0, clusterBuilder.getLiveNodeBuilders());
+    clusterBuilder.addCollection(collectionBuilder);
+
+    PlacementContext placementContext = clusterBuilder.buildPlacementContext();
+    Cluster cluster = placementContext.getCluster();
+
+    SolrCollection secondaryCollection = cluster.getCollection(secondaryCollectionName);
+    SolrCollection primaryCollection = cluster.getCollection(primaryCollectionName);
+
+    Node node = cluster.getLiveNodes().iterator().next();
+    Set<Replica> secondaryReplicas = new HashSet<>();
+    secondaryCollection.shards().forEach(shard ->
+        shard.replicas().forEach(replica -> {
+          if (secondaryReplicas.size() < 1 && replica.getNode().equals(node)) {
+            secondaryReplicas.add(replica);
+          }
+        }));
+
+    DeleteReplicasRequest deleteReplicasRequest = ModificationRequestImpl.deleteReplicasRequest(secondaryCollection, secondaryReplicas);
+    try {
+      plugin.verifyAllowedModification(deleteReplicasRequest, placementContext);
+    } catch (PlacementException pe) {
+      fail("should have succeeded: " + pe.toString());
+    }
+
+    secondaryCollection.shards().forEach(shard ->
+        shard.replicas().forEach(replica -> {
+          if (secondaryReplicas.size() < 2 && replica.getNode().equals(node)) {
+            secondaryReplicas.add(replica);
+          }
+        }));
+
+    deleteReplicasRequest = ModificationRequestImpl.deleteReplicasRequest(secondaryCollection, secondaryReplicas);
+    try {
+      plugin.verifyAllowedModification(deleteReplicasRequest, placementContext);
+      fail("should have failed: " + deleteReplicasRequest);
+    } catch (PlacementException pe) {
+    }
+  }
+
   @Test @Slow
   public void testScalability() throws Exception {
     log.info("==== numNodes ====");