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:50 UTC

[lucene] 18/33: SOLR-15004: Some renaming to clarify the purpose of plugin config params.

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 5705765b37b03a5c23f5384f6171d1b82333dbcd
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Wed Nov 25 20:25:16 2020 +0100

    SOLR-15004: Some renaming to clarify the purpose of plugin config params.
---
 .../plugins/AffinityPlacementFactory.java          | 48 ++++++++++++----------
 .../plugins/AffinityPlacementFactoryTest.java      | 46 ++++++++++++++++++---
 2 files changed, 66 insertions(+), 28 deletions(-)

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 ddddfd7..9690a86 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
@@ -140,6 +140,20 @@ public class AffinityPlacementFactory implements PlacementPluginFactory {
   public static final String UNDEFINED_AVAILABILITY_ZONE = "uNd3f1NeD";
 
   /**
+   * If a node has strictly less GB of free disk than this value, the node is excluded from assignment decisions.
+   * Set to 0 or less to disable.
+   */
+  public static final String MINIMAL_FREE_DISK_GB = "minimalFreeDiskGB";
+
+  /**
+   * Replica allocation will assign replicas to nodes with at least this number of GB of free disk space regardless
+   * of the number of cores on these nodes rather than assigning replicas to nodes with less than this amount of free
+   * disk space if that's an option (if that's not an option, replicas can still be assigned to nodes with less than this
+   * amount of free space).
+   */
+  public static final String PRIORITIZED_FREE_DISK_GB = "prioritizedFreeDiskGB";
+
+  /**
    * 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
@@ -150,9 +164,9 @@ public class AffinityPlacementFactory implements PlacementPluginFactory {
 
   @Override
   public PlacementPlugin createPluginInstance(PlacementPluginConfig config) {
-    final long minimalFreeDiskGB = config.getLongConfig("minimalFreeDiskGB", 20L);
-    final long deprioritizedFreeDiskGB = config.getLongConfig("deprioritizedFreeDiskGB", 100L);
-    return new AffinityPlacementPlugin(minimalFreeDiskGB, deprioritizedFreeDiskGB);
+    final long minimalFreeDiskGB = config.getLongConfig(MINIMAL_FREE_DISK_GB, 20L);
+    final long prioritizedFreeDiskGB = config.getLongConfig(PRIORITIZED_FREE_DISK_GB, 100L);
+    return new AffinityPlacementPlugin(minimalFreeDiskGB, prioritizedFreeDiskGB);
   }
 
   /**
@@ -161,28 +175,18 @@ public class AffinityPlacementFactory implements PlacementPluginFactory {
    */
   static class AffinityPlacementPlugin implements PlacementPlugin {
 
-    /**
-     * If a node has strictly less GB of free disk than this value, the node is excluded from assignment decisions.
-     * Set to 0 or less to disable.
-     */
     private final long minimalFreeDiskGB;
 
-    /**
-     * Replica allocation will assign replicas to nodes with at least this number of GB of free disk space regardless
-     * of the number of cores on these nodes rather than assigning replicas to nodes with less than this amount of free
-     * disk space if that's an option (if that's not an option, replicas can still be assigned to nodes with less than this
-     * amount of free space).
-     */
-    private final long deprioritizedFreeDiskGB;
+    private final long prioritizedFreeDiskGB;
 
     private Random random = new Random();
 
     /**
      * The factory has decoded the configuration for the plugin instance and passes it the parameters it needs.
      */
-    private AffinityPlacementPlugin(long minimalFreeDiskGB, long deprioritizedFreeDiskGB) {
+    private AffinityPlacementPlugin(long minimalFreeDiskGB, long prioritizedFreeDiskGB) {
       this.minimalFreeDiskGB = minimalFreeDiskGB;
-      this.deprioritizedFreeDiskGB = deprioritizedFreeDiskGB;
+      this.prioritizedFreeDiskGB = prioritizedFreeDiskGB;
     }
 
     @VisibleForTesting
@@ -415,7 +419,7 @@ public class AffinityPlacementFactory implements PlacementPluginFactory {
         azByExistingReplicas.put(azToNumReplicas.get(e.getKey()), new AzWithNodes(e.getKey(), e.getValue()));
       }
 
-      CoresAndDiskComparator coresAndDiskComparator = new CoresAndDiskComparator(attrValues, coresOnNodes, deprioritizedFreeDiskGB);
+      CoresAndDiskComparator coresAndDiskComparator = new CoresAndDiskComparator(attrValues, coresOnNodes, prioritizedFreeDiskGB);
 
       // Now we have for each AZ on which we might have a chance of placing a replica, the list of candidate nodes for replicas
       // (candidate: does not already have a replica of this shard and is in the corresponding AZ).
@@ -488,7 +492,7 @@ public class AffinityPlacementFactory implements PlacementPluginFactory {
     static class CoresAndDiskComparator implements Comparator<Node> {
       private final AttributeValues attrValues;
       private final Map<Node, Integer> coresOnNodes;
-      private final long deprioritizedFreeDiskGB;
+      private final long prioritizedFreeDiskGB;
 
 
       /**
@@ -498,17 +502,17 @@ public class AffinityPlacementFactory implements PlacementPluginFactory {
        * 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) {
+      CoresAndDiskComparator(AttributeValues attrValues, Map<Node, Integer> coresOnNodes, long prioritizedFreeDiskGB) {
         this.attrValues = attrValues;
         this.coresOnNodes = coresOnNodes;
-        this.deprioritizedFreeDiskGB = deprioritizedFreeDiskGB;
+        this.prioritizedFreeDiskGB = prioritizedFreeDiskGB;
       }
 
       @Override
       public int compare(Node a, Node b) {
         // Note all nodes do have free disk defined. This has been verified earlier.
-        boolean aHasLowFreeSpace = attrValues.getFreeDisk(a).get() < deprioritizedFreeDiskGB;
-        boolean bHasLowFreeSpace = attrValues.getFreeDisk(b).get() < deprioritizedFreeDiskGB;
+        boolean aHasLowFreeSpace = attrValues.getFreeDisk(a).get() < prioritizedFreeDiskGB;
+        boolean bHasLowFreeSpace = attrValues.getFreeDisk(b).get() < prioritizedFreeDiskGB;
         if (aHasLowFreeSpace != bHasLowFreeSpace) {
           // A node with low free space should be considered > node with high free space since it needs to come later in sort order
           return Boolean.compare(aHasLowFreeSpace, bHasLowFreeSpace);
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 3869249..a5d71e7 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
@@ -142,12 +142,6 @@ public class AffinityPlacementFactoryTest extends SolrTestCaseJ4 {
     PlacementPlan pp = plugin.computePlacement(cluster, placementRequest, attributeFetcher, placementPlanFactory);
     // 2 shards, 6 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())
-//        );
     // shard -> AZ -> replica count
     Map<Replica.ReplicaType, Map<String, Map<String, AtomicInteger>>> replicas = new HashMap<>();
     AttributeValues attributeValues = attributeFetcher.fetchAttributes();
@@ -240,6 +234,46 @@ public class AffinityPlacementFactoryTest extends SolrTestCaseJ4 {
   }
 
   @Test
+  public void testFreeDiskConstraints() throws Exception {
+    String collectionName = "testCollection";
+    int NUM_NODES = 3;
+    Builders.ClusterBuilder clusterBuilder = Builders.newClusterBuilder().initializeNodes(NUM_NODES);
+    Node smallNode = null;
+    for (int i = 0; i < NUM_NODES; i++) {
+      Builders.NodeBuilder nodeBuilder = clusterBuilder.getNodeBuilders().get(i);
+      nodeBuilder.setCoreCount(0);
+      if (i == 0) {
+        // default minimalFreeDiskGB == 20
+        nodeBuilder.setFreeDiskGB(1L);
+        smallNode = nodeBuilder.build();
+      } else {
+        nodeBuilder.setFreeDiskGB(100L);
+      }
+    }
+
+    Builders.CollectionBuilder collectionBuilder = Builders.newCollectionBuilder(collectionName);
+    collectionBuilder.initializeShardsReplicas(2, 0, 0, 0, clusterBuilder.getNodeBuilders());
+    clusterBuilder.addCollection(collectionBuilder);
+
+    Cluster cluster = clusterBuilder.build();
+
+    SolrCollection solrCollection = cluster.getCollection(collectionName);
+
+    PlacementRequestImpl placementRequest = new PlacementRequestImpl(solrCollection,
+        StreamSupport.stream(solrCollection.shards().spliterator(), false)
+            .map(Shard::getShardName).collect(Collectors.toSet()),
+        cluster.getLiveNodes(), 2, 0, 2);
+
+    PlacementPlanFactory placementPlanFactory = new PlacementPlanFactoryImpl();
+    AttributeFetcher attributeFetcher = clusterBuilder.buildAttributeFetcher();
+    PlacementPlan pp = plugin.computePlacement(cluster, placementRequest, attributeFetcher, placementPlanFactory);
+    assertEquals(8, pp.getReplicaPlacements().size());
+    for (ReplicaPlacement rp : pp.getReplicaPlacements()) {
+      assertFalse("should not put any replicas on " + smallNode, rp.getNode().equals(smallNode));
+    }
+  }
+
+  @Test
   //@Ignore
   public void testScalability() throws Exception {
     log.info("==== numNodes ====");