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/11 18:41:09 UTC

[GitHub] [solr] HoustonPutman opened a new pull request #512: SOLR-15209: Make the LegacyAssignStrategy the DefaultPlacementPlugin

HoustonPutman opened a new pull request #512:
URL: https://github.com/apache/solr/pull/512


   https://issues.apache.org/jira/browse/SOLR-15209
   
   There are some issues with each of the placementPlugins, but will follow up separately.


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


[GitHub] [solr] HoustonPutman commented on pull request #512: SOLR-15209: Make the LegacyAssignStrategy the DefaultPlacementPlugin

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #512:
URL: https://github.com/apache/solr/pull/512#issuecomment-1012448905


   Ok I will then rename the `DefaultPlacementPlugin` as `LegacyPlacementPlugin`, since it would not make sense in the future if the `DefaultPlacementPlugin` is not the default.
   
   I will leave a TODO comment for the future. 


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


[GitHub] [solr] HoustonPutman commented on pull request #512: SOLR-15209: Make the LegacyAssignStrategy the DefaultPlacementPlugin

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #512:
URL: https://github.com/apache/solr/pull/512#issuecomment-1011222062


   > I see you're considering [SOLR-15209](https://issues.apache.org/jira/browse/SOLR-15209) as making `LegacyAssignStrategy` the default placement, when it originally was (and still is in Jira) "Make the AffinityPlacementFactory the default placement plugin".
   
   Fair enough, I thought about creating a new ticket, which I still can. I ended up not because making the legacy strategy a plugin, like the rest, was a part of the description. If you would like a new JIRA to track this, so that [SOLR-15209](https://issues.apache.org/jira/browse/SOLR-15209) can be used to track changing the default, I am happy to make the new one and link it 🙂 
   
   > The original goal of [SOLR-15209](https://issues.apache.org/jira/browse/SOLR-15209) was to make the realistic placement strategy (affinity placement) the default one to 1. have better/more flexible placement overall, and 2. make sure this code is running (in installations, in tests) to find issues with it.
   
   We can always do this (even under a different ticket), then decide to change the default implementation separately. I'm perfectly fine with that as long as we test how the performance of cluster operations are affected.
   
   But I definitely agree that there should be more tests around these. (I think while writing this, I found some issues that I will surface in new JIRAs)
   
   


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


[GitHub] [solr] HoustonPutman merged pull request #512: SOLR-15209: Make the LegacyAssignStrategy the DefaultPlacementPlugin

Posted by GitBox <gi...@apache.org>.
HoustonPutman merged pull request #512:
URL: https://github.com/apache/solr/pull/512


   


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


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

Posted by GitBox <gi...@apache.org>.
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


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

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #512:
URL: https://github.com/apache/solr/pull/512#discussion_r783237167



##########
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:
       Good call, I wrote this originally without that information, then tried to fix it all when I figured out how `getTargetNodes()` was populated. I'll scan and make sure no other checks made it through.

##########
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:
       Yeah that quickly broke many tests. Will remove, that's a good call.




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


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

Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #512:
URL: https://github.com/apache/solr/pull/512#issuecomment-1011537027


   I hope eventually we can make another placement plugin (the affinity placement one) the default placement, but I don't have time any time soon to look at the tests, understand them and make them work with it.
   If you can leave a TODO or comment where the default is set that eventually we'd want another default, I think you can hijack the jira as planned (we'll create a new jira when/if we get to change the default).


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