You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by dw...@apache.org on 2021/03/10 10:09:40 UTC

[lucene] 08/33: SOLR-15004: Start testing the availability zone affinity. Add toString() to make it easier to debug / report. Improve test abstractions to make it easier to set up tests.

This is an automated email from the ASF dual-hosted git repository.

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

commit 9ca59200949ea3d578e30d52066f86072814beb8
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Mon Nov 23 19:42:01 2020 +0100

    SOLR-15004: Start testing the availability zone affinity. Add toString() to make it
    easier to debug / report. Improve test abstractions to make it easier to set up tests.
---
 .../cluster/placement/impl/PlacementPlanImpl.java  | 10 +++
 .../placement/impl/ReplicaPlacementImpl.java       |  5 ++
 .../plugins/AffinityPlacementFactory.java          | 42 ++++++------
 .../impl/AffinityPlacementFactoryTest.java         | 79 ++++++++++++++++++----
 .../placement/impl/ClusterAbstractionsForTest.java | 43 ++++++++++--
 .../cluster/placement/impl/PluginTestHelper.java   | 39 +++++++++--
 6 files changed, 174 insertions(+), 44 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPlanImpl.java b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPlanImpl.java
index 2dde07b..a8a65e0 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPlanImpl.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPlanImpl.java
@@ -42,4 +42,14 @@ class PlacementPlanImpl implements PlacementPlan {
   public Set<ReplicaPlacement> getReplicaPlacements() {
     return replicaPlacements;
   }
+
+  @Override
+  public String toString() {
+    StringBuilder sb = new StringBuilder("PlacementPlan{");
+    for (ReplicaPlacement placement : replicaPlacements) {
+      sb.append("\n").append(placement.toString());
+    }
+    sb.append("\n}");
+    return sb.toString();
+  }
 }
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/ReplicaPlacementImpl.java b/solr/core/src/java/org/apache/solr/cluster/placement/impl/ReplicaPlacementImpl.java
index 0bf7564..69d9718 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/ReplicaPlacementImpl.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/ReplicaPlacementImpl.java
@@ -60,6 +60,11 @@ class ReplicaPlacementImpl implements ReplicaPlacement {
     return replicaType;
   }
 
+  @Override
+  public String toString() {
+    return solrCollection.getName() + "/" + shardName + "/" + replicaType + "->" + node.getName();
+  }
+
   /**
    * Translates a set of {@link ReplicaPlacement} returned by a plugin into a list of {@link ReplicaPosition} expected
    * by {@link org.apache.solr.cloud.api.collections.Assign.AssignStrategy}
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 50ecb8d..9a00903 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
@@ -118,6 +118,26 @@ public class AffinityPlacementFactory implements PlacementPluginFactory {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   /**
+   * <p>Name of the system property on a node indicating which (public cloud) Availability Zone that node is in. The value
+   * is any string, different strings denote different availability zones.
+   *
+   * <p>Nodes on which this system property is not defined are considered being in the same Availability Zone
+   * {@link #UNDEFINED_AVAILABILITY_ZONE} (hopefully the value of this constant is not the name of a real Availability Zone :).
+   */
+  public static final String AVAILABILITY_ZONE_SYSPROP = "availability_zone";
+
+  /**
+   * <p>Name of the system property on a node indicating the type of replicas allowed on that node.
+   * The value of that system property is a comma separated list or a single string of value names of
+   * {@link Replica.ReplicaType} (case insensitive). If that property is not defined, that node is
+   * considered accepting all replica types (i.e. undefined is equivalent to {@code "NRT,Pull,tlog"}).
+   */
+  public static final String REPLICA_TYPE_SYSPROP = "replica_type";
+
+  /** This is the "AZ" name for nodes that do not define an AZ. Should not match a real AZ name (I think we're safe) */
+  public static final String UNDEFINED_AVAILABILITY_ZONE = "uNd3f1NeD";
+
+  /**
    * Empty public constructor is used to instantiate this factory. Using a factory pattern to allow the factory to do one
    * time costly operations if needed, and to only have to instantiate a default constructor class by name, rather than
    * having to call a constructor with more parameters (if we were to instantiate the plugin class directly without going
@@ -138,26 +158,6 @@ public class AffinityPlacementFactory implements PlacementPluginFactory {
    * on what the plugin does.
    */
   static private class AffinityPlacementPlugin implements PlacementPlugin {
-    /**
-     * <p>Name of the system property on a node indicating which (public cloud) Availability Zone that node is in. The value
-     * is any string, different strings denote different availability zones.
-     *
-     * <p>Nodes on which this system property is not defined are considered being in the same Availability Zone
-     * {@link #UNDEFINED_AVAILABILITY_ZONE} (hopefully the value of this constant is not the name of a real Availability Zone :).
-     */
-    public static final String AVAILABILITY_ZONE_SYSPROP = "availability_zone";
-    /** This is the "AZ" name for nodes that do not define an AZ. Should not match a real AZ name (I think we're safe) */
-    public static final String UNDEFINED_AVAILABILITY_ZONE = "uNd3f1NeD";
-
-    /**
-     * <p>Name of the system property on a node indicating the type of replicas allowed on that node.
-     * The value of that system property is a comma separated list or a single string of value names of
-     * {@link org.apache.solr.cluster.Replica.ReplicaType} (case insensitive). If that property is not defined, that node is
-     * considered accepting all replica types (i.e. undefined is equivalent to {@code "NRT,Pull,tlog"}).
-     *
-     * <p>See {@link #getNodesPerReplicaType}.
-     */
-    public static final String REPLICA_TYPE_SYSPROP = "replica_type";
 
     /**
      * If a node has strictly less GB of free disk than this value, the node is excluded from assignment decisions.
@@ -486,7 +486,7 @@ public class AffinityPlacementFactory implements PlacementPluginFactory {
        * The data we sort on is not part of the {@link Node} instances but has to be retrieved from the attributes and configuration.
        * The number of cores per node is passed in a map whereas the free disk is fetched from the attributes due to the
        * fact that we update the number of cores per node as we do allocations, but we do not update the free disk. The
-       * attrValues correpsonding to the number of cores per node are the initial values, but we want to comapre the actual
+       * attrValues corresponding to the number of cores per node are the initial values, but we want to compare the actual
        * value taking into account placement decisions already made during the current execution of the placement plugin.
        */
       CoresAndDiskComparator(AttributeValues attrValues, Map<Node, Integer> coresOnNodes, long deprioritizedFreeDiskGB) {
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/AffinityPlacementFactoryTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/impl/AffinityPlacementFactoryTest.java
index 80f30105..bcfc233 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/AffinityPlacementFactoryTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/impl/AffinityPlacementFactoryTest.java
@@ -20,21 +20,26 @@ package org.apache.solr.cluster.placement.impl;
 import org.apache.solr.cluster.Cluster;
 import org.apache.solr.cluster.Node;
 import org.apache.solr.cluster.Replica;
-import org.apache.solr.cluster.Shard;
 import org.apache.solr.cluster.SolrCollection;
 import org.apache.solr.cluster.placement.*;
 import org.apache.solr.cluster.placement.plugins.AffinityPlacementFactory;
 import org.junit.Assert;
 import org.junit.BeforeClass;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -75,25 +80,22 @@ public class AffinityPlacementFactoryTest extends Assert {
         Node node2 = new ClusterAbstractionsForTest.NodeImpl("node2");
         Set<Node> liveNodes = Set.of(node1, node2);
 
-        ClusterAbstractionsForTest.SolrCollectionImpl solrCollection = new ClusterAbstractionsForTest.SolrCollectionImpl(collectionName, Map.of());
+        ClusterAbstractionsForTest.SolrCollectionImpl solrCollection;
         // Make sure new collections are not visible in the cluster state and existing ones are
         final Map<String, SolrCollection> clusterCollections;
-        final Map<String, Shard> shards;
         if (hasExistingCollection) {
             // An existing collection with a single replica on node 1. Note that new collections already exist by the time the plugin is called, but are empty
-            shards = PluginTestHelper.createShardsAndReplicas(solrCollection, 1, 1, Set.of(node1));
-            solrCollection.setShards(shards);
+            solrCollection = PluginTestHelper.createCollection(collectionName, Map.of(), 1, 1, 0, 0, Set.of(node1));
             clusterCollections = Map.of(solrCollection.getName(), solrCollection);
         } else {
             // A new collection has the shards defined ok but no replicas
-            shards = PluginTestHelper.createShardsAndReplicas(solrCollection, 1, 0, Set.of());
-            solrCollection.setShards(shards);
+            solrCollection = PluginTestHelper.createCollection(collectionName, Map.of(), 1, 0, 0, 0, Set.of());
             clusterCollections = Map.of();
         }
 
         Cluster cluster = new ClusterAbstractionsForTest.ClusterImpl(liveNodes, clusterCollections);
         // Place a new replica for the (only) existing shard of the collection
-        PlacementRequestImpl placementRequest = new PlacementRequestImpl(solrCollection, Set.of(shards.keySet().iterator().next()), liveNodes, 1, 0, 0);
+        PlacementRequestImpl placementRequest = new PlacementRequestImpl(solrCollection, Set.of(solrCollection.shards().iterator().next().getShardName()), liveNodes, 1, 0, 0);
         // More cores on node2
         Map<Node, Integer> nodeToCoreCount = Map.of(node1, 1, node2, 10);
         // A lot of free disk on the two nodes
@@ -111,6 +113,54 @@ public class AffinityPlacementFactoryTest extends Assert {
     }
 
     @Test
+    public void testAvailabilityZones() throws Exception {
+        String collectionName = "testCollection";
+
+        int NUM_NODES = 6;
+        final Set<Node> liveNodes = new HashSet<>();
+        final Map<Node, Long> nodeToFreeDisk = new HashMap<>();
+        final Map<Node, Integer> nodeToCoreCount = new HashMap<>();
+        final Map<String, Map<Node, String>> zones = Map.of(AffinityPlacementFactory.AVAILABILITY_ZONE_SYSPROP, new HashMap<>());
+        final Map<Node, String> sysprops = zones.get(AffinityPlacementFactory.AVAILABILITY_ZONE_SYSPROP);
+        for (int i = 0; i < NUM_NODES; i++) {
+            Node node = new ClusterAbstractionsForTest.NodeImpl("node_" + i);
+            liveNodes.add(node);
+            nodeToFreeDisk.put(node, 100L);
+            nodeToCoreCount.put(node, 0);
+            if (i < NUM_NODES / 2) {
+                sysprops.put(node, "az1");
+            } else {
+                sysprops.put(node, "az2");
+            }
+        }
+
+        ClusterAbstractionsForTest.SolrCollectionImpl solrCollection = PluginTestHelper.createCollection(collectionName,
+            Map.of(), 2, 0, 0, 0, liveNodes);
+        ClusterAbstractionsForTest.ClusterImpl cluster = new ClusterAbstractionsForTest.ClusterImpl(liveNodes, Map.of());
+
+        PlacementRequestImpl placementRequest = new PlacementRequestImpl(solrCollection, solrCollection.getShardNames(), liveNodes, 2, 2, 2);
+
+        AttributeValues attributeValues = new AttributeValuesImpl(nodeToCoreCount, Map.of(), nodeToFreeDisk, Map.of(), Map.of(), Map.of(),
+            zones, Map.of());
+        AttributeFetcher attributeFetcher = new AttributeFetcherForTest(attributeValues);
+        PlacementPlanFactory placementPlanFactory = new PlacementPlanFactoryImpl();
+
+        PlacementPlan pp = plugin.computePlacement(cluster, placementRequest, attributeFetcher, placementPlanFactory);
+        // 2 shards, 5 replicas
+        assertEquals(12, pp.getReplicaPlacements().size());
+        List<ReplicaPlacement> placements = new ArrayList<>(pp.getReplicaPlacements());
+        Collections.sort(placements, Comparator
+            .comparing((ReplicaPlacement p) -> p.getNode().getName())
+            .thenComparing((ReplicaPlacement p) -> p.getShardName())
+            .thenComparing((ReplicaPlacement p) -> p.getReplicaType())
+        );
+        log.info(placements.toString());
+        // AZ -> shard -> replica count
+        //Map<String, Map<String, AtomicInteger>>
+    }
+
+    @Test
+    //@Ignore
     public void testScalability() throws Exception {
         log.info("==== numNodes ====");
         runTestScalability(1000, 100, 40, 40, 20);
@@ -150,13 +200,16 @@ public class AffinityPlacementFactoryTest extends Assert {
             nodeToFreeDisk.put(node, Long.valueOf(numNodes));
             nodeToCoreCount.put(node, 0);
         }
-        ClusterAbstractionsForTest.SolrCollectionImpl solrCollection = new ClusterAbstractionsForTest.SolrCollectionImpl(collectionName, Map.of());
-        Map<String, Shard> shards = PluginTestHelper.createShardsAndReplicas(solrCollection, numShards, 0, Set.of());
-        solrCollection.setShards(shards);
+        ClusterAbstractionsForTest.SolrCollectionImpl solrCollection =
+            PluginTestHelper.createCollection(collectionName, Map.of(), numShards, 0, 0, 0, Set.of());
 
         Cluster cluster = new ClusterAbstractionsForTest.ClusterImpl(liveNodes, Map.of());
-        PlacementRequestImpl placementRequest = new PlacementRequestImpl(solrCollection, shards.keySet(), liveNodes,
-            nrtReplicas, tlogReplicas, pullReplicas);
+        PlacementRequestImpl placementRequest = new PlacementRequestImpl(solrCollection,
+            // XXX awkward!
+            // StreamSupport.stream(solrCollection.shards().spliterator(), false)
+            //     .map(Shard::getShardName).collect(Collectors.toSet()),
+            solrCollection.getShardNames(),
+            liveNodes, nrtReplicas, tlogReplicas, pullReplicas);
 
         AttributeValues attributeValues = new AttributeValuesImpl(nodeToCoreCount, Map.of(), nodeToFreeDisk, Map.of(), Map.of(), Map.of(), Map.of(), Map.of());
         AttributeFetcher attributeFetcher = new AttributeFetcherForTest(attributeValues);
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/ClusterAbstractionsForTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/impl/ClusterAbstractionsForTest.java
index 188e3c3..02395d6 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/ClusterAbstractionsForTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/impl/ClusterAbstractionsForTest.java
@@ -30,12 +30,16 @@ import java.util.stream.Collectors;
 class ClusterAbstractionsForTest {
 
     static class ClusterImpl implements Cluster {
-        private final Set<Node> liveNodes;
-        private final Map<String, SolrCollection> collections;
+        private final Set<Node> liveNodes = new HashSet<>();
+        private final Map<String, SolrCollection> collections = new HashMap<>();
+
+        ClusterImpl() {
+
+        }
 
         ClusterImpl(Set<Node> liveNodes, Map<String, SolrCollection> collections) throws IOException {
-            this.liveNodes = liveNodes;
-            this.collections = collections;
+            this.liveNodes.addAll(liveNodes);
+            this.collections.putAll(collections);
         }
 
         @Override
@@ -58,6 +62,33 @@ class ClusterAbstractionsForTest {
         public Iterable<SolrCollection> collections() {
             return ClusterImpl.this::iterator;
         }
+
+        // for unit tests
+
+        ClusterImpl addNode(Node node) {
+            liveNodes.add(node);
+            return this;
+        }
+
+        ClusterImpl removeNode(Node node) {
+            liveNodes.remove(node);
+            return this;
+        }
+
+        ClusterImpl putCollection(SolrCollection collection) {
+            collections.put(collection.getName(), collection);
+            return this;
+        }
+
+        ClusterImpl removeCollection(String name) {
+            collections.remove(name);
+            return this;
+        }
+
+        ClusterImpl removeAllCollections() {
+            collections.clear();
+            return this;
+        }
     }
 
 
@@ -122,6 +153,10 @@ class ClusterAbstractionsForTest {
             this.shards = shards;
         }
 
+        Set<String> getShardNames() {
+            return shards.keySet();
+        }
+
         @Override
         public String getName() {
             return collectionName;
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PluginTestHelper.java b/solr/core/src/test/org/apache/solr/cluster/placement/impl/PluginTestHelper.java
index 13ef70d..f8b4d87 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/impl/PluginTestHelper.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/impl/PluginTestHelper.java
@@ -29,24 +29,49 @@ import java.util.Set;
 
 public class PluginTestHelper {
 
+    static ClusterAbstractionsForTest.SolrCollectionImpl createCollection(String name, Map<String, String> properties,
+                                           int numShards, int nrtReplicas, int tlogReplicas, int pullReplicas, Set<Node> nodes) {
+        ClusterAbstractionsForTest.SolrCollectionImpl solrCollection = new ClusterAbstractionsForTest.SolrCollectionImpl(name, properties);
+        Map<String, Shard> shards = createShardsAndReplicas(solrCollection, numShards, nrtReplicas, tlogReplicas, pullReplicas, nodes);
+        solrCollection.setShards(shards);
+        return solrCollection;
+    }
+
     /**
      * Builds the representation of shards for a collection, based on the number of shards and replicas for each to create.
      * The replicas are allocated to the provided nodes in a round robin way. The leader is set to the last replica of each shard.
      */
-    static Map<String, Shard> createShardsAndReplicas(SolrCollection collection, int numShards, int numNrtReplicas, Set<Node> nodes) {
+    static Map<String, Shard> createShardsAndReplicas(SolrCollection collection, int numShards,
+                                                      int nrtReplicas, int tlogReplicas, int pullReplicas,
+                                                      Set<Node> nodes) {
         Iterator<Node> nodeIterator = nodes.iterator();
 
         Map<String, Shard> shards = new HashMap<>();
 
         for (int s = 0; s < numShards; s++) {
-            String shardName = collection.getName() + "_s" + s;
+            // "traditional" shard name
+            String shardName = "shard" + (s + 1);
 
             ClusterAbstractionsForTest.ShardImpl shard = new ClusterAbstractionsForTest.ShardImpl(shardName, collection, Shard.ShardState.ACTIVE);
 
             Map<String, Replica> replicas = new HashMap<>();
+
             Replica leader = null;
-            for (int r = 0; r < numNrtReplicas; r++) {
-                String replicaName = shardName + "_r" + r;
+            int totalReplicas = nrtReplicas + tlogReplicas + pullReplicas;
+            for (int r = 0; r < totalReplicas; r++) {
+                Replica.ReplicaType type;
+                String suffix;
+                if (r < nrtReplicas) {
+                    type = Replica.ReplicaType.NRT;
+                    suffix = "n";
+                } else if (r < nrtReplicas + tlogReplicas) {
+                    type = Replica.ReplicaType.TLOG;
+                    suffix = "t";
+                } else {
+                    type = Replica.ReplicaType.PULL;
+                    suffix = "p";
+                }
+                String replicaName = shardName + "_replica_" + suffix + r;
                 String coreName = replicaName + "_c";
                 final Node node;
                 if (!nodeIterator.hasNext()) {
@@ -55,10 +80,12 @@ public class PluginTestHelper {
                 // If the nodes set is empty, this call will fail
                 node = nodeIterator.next();
 
-                Replica replica = new ClusterAbstractionsForTest.ReplicaImpl(replicaName, coreName, shard, Replica.ReplicaType.NRT, Replica.ReplicaState.ACTIVE, node);
+                Replica replica = new ClusterAbstractionsForTest.ReplicaImpl(replicaName, coreName, shard, type, Replica.ReplicaState.ACTIVE, node);
 
                 replicas.put(replica.getReplicaName(), replica);
-                leader = replica;
+                if (replica.getType() == Replica.ReplicaType.NRT) {
+                    leader = replica;
+                }
             }
 
             shard.setReplicas(replicas, leader);