You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/01/12 12:04:56 UTC

[GitHub] [solr] murblanc commented on a change in pull request #512: SOLR-15209: Make the LegacyAssignStrategy the DefaultPlacementPlugin

murblanc commented on a change in pull request #512:
URL: https://github.com/apache/solr/pull/512#discussion_r782952471



##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/plugins/DefaultPlacementFactory.java
##########
@@ -0,0 +1,157 @@
+/*
+ * 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.plugins;
+
+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.PlacementContext;
+import org.apache.solr.cluster.placement.PlacementException;
+import org.apache.solr.cluster.placement.PlacementPlan;
+import org.apache.solr.cluster.placement.PlacementPlugin;
+import org.apache.solr.cluster.placement.PlacementPluginFactory;
+import org.apache.solr.cluster.placement.PlacementRequest;
+import org.apache.solr.cluster.placement.ReplicaPlacement;
+import org.apache.solr.common.SolrException;
+
+import java.util.ArrayList;
+import java.util.Collection;
+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.stream.Collectors;
+
+/**
+ * <p>Factory for creating {@link DefaultPlacementPlugin}, a placement plugin implementing a smart placement for new
+ * replicas, picking nodes with the fewest cores (especially cores of the same collection).</p>
+ *
+ * <p>See {@link AffinityPlacementFactory} for a more realistic example and documentation.</p>
+ */
+public class DefaultPlacementFactory implements PlacementPluginFactory<PlacementPluginFactory.NoConfig> {
+
+  @Override
+  public PlacementPlugin createPluginInstance() {
+    return new DefaultPlacementPlugin();
+  }
+
+  public static class DefaultPlacementPlugin implements PlacementPlugin {
+    @Override
+    public List<PlacementPlan> computePlacements(Collection<PlacementRequest> requests, PlacementContext placementContext) throws PlacementException {
+      List<PlacementPlan> placementPlans = new ArrayList<>(requests.size());
+      Map<Node, ReplicaCount> nodeVsShardCount = getNodeVsShardCount(placementContext);
+      for (PlacementRequest request : requests) {
+        int totalReplicasPerShard = 0;
+        for (Replica.ReplicaType rt : Replica.ReplicaType.values()) {
+          totalReplicasPerShard += request.getCountReplicasToCreate(rt);
+        }
+
+        Set<ReplicaPlacement> replicaPlacements = new HashSet<>(totalReplicasPerShard * request.getShardNames().size());
+
+        Collection<ReplicaCount> replicaCounts = nodeVsShardCount.values();
+
+        if (request.getTargetNodes().size() < replicaCounts.size()) {
+          replicaCounts = replicaCounts.stream().filter(rc -> request.getTargetNodes().contains(rc.node())).collect(Collectors.toList());
+        } else if (placementContext.getCluster().getLiveDataNodes().isEmpty()) {
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "There are no live nodes in the cluster");
+        }
+
+        if (replicaCounts.size() < totalReplicasPerShard) {
+          throw new PlacementException("Cluster size too small for number of replicas per shard");

Review comment:
       This is a change of behavior from `LegacyAssignStrategy`. Placements that once worked might now fail.

##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/plugins/DefaultPlacementFactory.java
##########
@@ -0,0 +1,157 @@
+/*
+ * 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.plugins;
+
+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.PlacementContext;
+import org.apache.solr.cluster.placement.PlacementException;
+import org.apache.solr.cluster.placement.PlacementPlan;
+import org.apache.solr.cluster.placement.PlacementPlugin;
+import org.apache.solr.cluster.placement.PlacementPluginFactory;
+import org.apache.solr.cluster.placement.PlacementRequest;
+import org.apache.solr.cluster.placement.ReplicaPlacement;
+import org.apache.solr.common.SolrException;
+
+import java.util.ArrayList;
+import java.util.Collection;
+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.stream.Collectors;
+
+/**
+ * <p>Factory for creating {@link DefaultPlacementPlugin}, a placement plugin implementing a smart placement for new
+ * replicas, picking nodes with the fewest cores (especially cores of the same collection).</p>
+ *

Review comment:
       Maybe add a note here about this plugin being a repackaging of the `LegacyAssignStrategy`? Might be helpful for the few initial 9.x releases when people try to understand the changes from 8.x around placement and autoscaling.

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -525,73 +460,20 @@ public AssignRequest build() {
     }
   }
 
-  public static class LegacyAssignStrategy implements AssignStrategy {
-    @Override
-    public List<ReplicaPosition> assign(SolrCloudManager solrCloudManager, List<AssignRequest> assignRequests) throws Assign.AssignmentException, IOException, InterruptedException {
-      ClusterState clusterState = solrCloudManager.getClusterStateProvider().getClusterState();
-
-      List<ReplicaPosition> result = new ArrayList<>();
-
-      HashMap<String, Assign.ReplicaCount> nodeNameVsShardCount = new HashMap<>();
-      addNodeNameVsShardCount(clusterState, nodeNameVsShardCount);
-      for (AssignRequest assignRequest : assignRequests) {
-        Collection<ReplicaCount> replicaCounts = nodeNameVsShardCount.values();
-
-        if (assignRequest.nodes != null && !assignRequest.nodes.isEmpty()) {
-          // Throw an error if there are any non-live nodes.
-          checkLiveNodes(assignRequest.nodes, clusterState);
-          HashSet<String> nodeSet = new HashSet<>(assignRequest.nodes);
-          replicaCounts = replicaCounts.stream().filter(rc -> nodeSet.contains(rc.nodeName)).collect(Collectors.toList());
-        } else if (nodeNameVsShardCount.values().isEmpty()) {
-          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "There are no live nodes in the cluster");
-        }
-
-        for (String aShard : assignRequest.shardNames) {
-          // Reset the ordering of the nodes for each shard, using the replicas added in the previous shards and assign requests
-          List<String> nodeList = replicaCounts.stream()
-                  .sorted(Comparator.<ReplicaCount>comparingInt(rc -> rc.weight(assignRequest.collectionName)).thenComparing(ReplicaCount::nodeName))
-                  .map(ReplicaCount::nodeName)
-                  .collect(Collectors.toList());
-          int i = 0;
-          for (Map.Entry<Replica.Type, Integer> e : countsPerReplicaType(assignRequest).entrySet()) {
-            for (int j = 0; j < e.getValue(); j++) {
-              String assignedNode = nodeList.get(i % nodeList.size());
-              result.add(new ReplicaPosition(assignRequest.collectionName, aShard, j, e.getKey(), assignedNode));
-              i++;
-              ReplicaCount replicaCount = nodeNameVsShardCount.computeIfAbsent(assignedNode, ReplicaCount::new);
-              replicaCount.totalReplicas++;
-              replicaCount.collectionReplicas.merge(assignRequest.collectionName, 1, Integer::sum);
-            }
-          }
-        }
-      }
-
-      return result;
-    }
-
-    // keeps this big ugly construction block out of otherwise legible code
-    private ImmutableMap<Replica.Type, Integer> countsPerReplicaType(AssignRequest assignRequest) {
-      return ImmutableMap.of(
-          Replica.Type.NRT, assignRequest.numNrtReplicas,
-          Replica.Type.TLOG, assignRequest.numTlogReplicas,
-          Replica.Type.PULL, assignRequest.numPullReplicas
-      );
-    }
-  }
-
   /**
    * Creates the appropriate instance of {@link AssignStrategy} based on how the cluster and/or individual collections are
    * configured.
-   * <p>If {@link PlacementPlugin} instance is null this call will return {@link LegacyAssignStrategy}, otherwise
+   * <p>If {@link PlacementPlugin} instance is null this call will return a strategy from {@link DefaultPlacementFactory}, otherwise
    * {@link PlacementPluginAssignStrategy} will be used.</p>
    */
   public static AssignStrategy createAssignStrategy(CoreContainer coreContainer) {
     PlacementPlugin placementPlugin = coreContainer.getPlacementPluginFactory().createPluginInstance();
     if (placementPlugin != null) {
       // If a cluster wide placement plugin is configured (and that's the only way to define a placement plugin)
       return new PlacementPluginAssignStrategy(placementPlugin);
-    }  else {
-        return new LegacyAssignStrategy();
-      }
+    } else {
+      DefaultPlacementFactory defaultPlacementFactory = new DefaultPlacementFactory();
+      return new PlacementPluginAssignStrategy(defaultPlacementFactory.createPluginInstance());

Review comment:
       minor: `return new PlacementPluginAssignStrategy(placementPlugin)` could be moved to after the if/else, with the if changed into something like:
   ```
   if (placementPlugin == null) {
       placementPlugin = (new DefaultPlacementFactory()).createPluginInstance();
   }
   ```

##########
File path: solr/core/src/java/org/apache/solr/cluster/placement/plugins/DefaultPlacementFactory.java
##########
@@ -0,0 +1,157 @@
+/*
+ * 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.plugins;
+
+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.PlacementContext;
+import org.apache.solr.cluster.placement.PlacementException;
+import org.apache.solr.cluster.placement.PlacementPlan;
+import org.apache.solr.cluster.placement.PlacementPlugin;
+import org.apache.solr.cluster.placement.PlacementPluginFactory;
+import org.apache.solr.cluster.placement.PlacementRequest;
+import org.apache.solr.cluster.placement.ReplicaPlacement;
+import org.apache.solr.common.SolrException;
+
+import java.util.ArrayList;
+import java.util.Collection;
+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.stream.Collectors;
+
+/**
+ * <p>Factory for creating {@link DefaultPlacementPlugin}, a placement plugin implementing a smart placement for new
+ * replicas, picking nodes with the fewest cores (especially cores of the same collection).</p>
+ *
+ * <p>See {@link AffinityPlacementFactory} for a more realistic example and documentation.</p>
+ */
+public class DefaultPlacementFactory implements PlacementPluginFactory<PlacementPluginFactory.NoConfig> {
+
+  @Override
+  public PlacementPlugin createPluginInstance() {
+    return new DefaultPlacementPlugin();
+  }
+
+  public static class DefaultPlacementPlugin implements PlacementPlugin {
+    @Override
+    public List<PlacementPlan> computePlacements(Collection<PlacementRequest> requests, PlacementContext placementContext) throws PlacementException {
+      List<PlacementPlan> placementPlans = new ArrayList<>(requests.size());
+      Map<Node, ReplicaCount> nodeVsShardCount = getNodeVsShardCount(placementContext);
+      for (PlacementRequest request : requests) {
+        int totalReplicasPerShard = 0;
+        for (Replica.ReplicaType rt : Replica.ReplicaType.values()) {
+          totalReplicasPerShard += request.getCountReplicasToCreate(rt);
+        }
+
+        Set<ReplicaPlacement> replicaPlacements = new HashSet<>(totalReplicasPerShard * request.getShardNames().size());
+
+        Collection<ReplicaCount> replicaCounts = nodeVsShardCount.values();
+
+        if (request.getTargetNodes().size() < replicaCounts.size()) {
+          replicaCounts = replicaCounts.stream().filter(rc -> request.getTargetNodes().contains(rc.node())).collect(Collectors.toList());
+        } else if (placementContext.getCluster().getLiveDataNodes().isEmpty()) {

Review comment:
       Why do we go back to the context here? I believe `request.getTargetNodes()` was prepared to contain a list of real candidate nodes for the placement.
   See `PlacementRequestImpl.toPlacementRequest()`.
   
   And `request.getTargetNodes()` is never empty.




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org