SOLR-11243: Replica Placement rules are ignored if a cluster policy exists

solr/CHANGES.txt

@@ -463,6 +463,8 @@ Bug Fixes
 
 * SOLR-10698: StreamHandler should allow connections to be closed early (Joel Bernstein, Varun Thacker, Erick Erickson)
 
+* SOLR-11243: Replica Placement rules are ignored if a cluster policy exists. (shalin)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/cloud/Assign.java

@@ -250,7 +250,7 @@ public class Assign {
     String policyName = message.getStr(POLICY);
     Map autoScalingJson = Utils.getJson(zkStateReader.getZkClient(), SOLR_AUTOSCALING_CONF_PATH, true);
 
-    if (rulesMap == null && policyName == null) {
+    if (rulesMap == null && policyName == null && autoScalingJson.get(Policy.CLUSTER_POLICY) == null) {
       int i = 0;
       List<ReplicaPosition> result = new ArrayList<>();
       for (String aShard : shardNames) {
@@ -274,10 +274,7 @@ public class Assign {
       }
     }
 
-    if (policyName != null || autoScalingJson.get(Policy.CLUSTER_POLICY) != null) {
+    if (rulesMap != null && !rulesMap.isEmpty()) {
-      return getPositionsUsingPolicy(collectionName,
-          shardNames, numNrtReplicas, policyName, zkStateReader, nodeList);
-    } else {
       List<Rule> rules = new ArrayList<>();
       for (Object map : rulesMap) rules.add(new Rule((Map) map));
       Map<String, Integer> sharVsReplicaCount = new HashMap<>();
@@ -295,6 +292,9 @@ public class Assign {
       return nodeMappings.entrySet().stream()
           .map(e -> new ReplicaPosition(e.getKey().shard, e.getKey().index, e.getKey().type, e.getValue()))
           .collect(Collectors.toList());
+    } else {
+      return getPositionsUsingPolicy(collectionName,
+          shardNames, numNrtReplicas, policyName, zkStateReader, nodeList);
     }
   }
 

solr/core/src/test/org/apache/solr/cloud/rule/RulesTest.java

@@ -23,6 +23,7 @@ import java.util.Map;
 
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.embedded.JettySolrRunner;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
@@ -40,6 +41,7 @@ import org.slf4j.LoggerFactory;
 
 import static org.apache.solr.client.solrj.SolrRequest.METHOD.GET;
 import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.cloud.autoscaling.AutoScalingHandlerTest.createAutoScalingRequest;
 import static org.apache.solr.common.params.CommonParams.COLLECTIONS_HANDLER_PATH;
 import static org.junit.matchers.JUnitMatchers.containsString;
 
@@ -91,6 +93,41 @@ public class RulesTest extends SolrCloudTestCase {
 
   }
 
+  @Test
+  public void testPortRuleInPresenceOfClusterPolicy() throws Exception  {
+    JettySolrRunner jetty = cluster.getRandomJetty(random());
+    String port = Integer.toString(jetty.getLocalPort());
+
+    // this cluster policy prohibits having any replicas on a node with the above port
+    String setClusterPolicyCommand = "{" +
+        " 'set-cluster-policy': [" +
+        "      {'replica': 0, 'port':'" + port + "'}" +
+        "    ]" +
+        "}";
+    SolrRequest req = createAutoScalingRequest(SolrRequest.METHOD.POST, setClusterPolicyCommand);
+    cluster.getSolrClient().request(req);
+
+    // but this collection is created with a replica placement rule that says all replicas must be created
+    // on a node with above port (in direct conflict with the cluster policy)
+    String rulesColl = "portRuleColl2";
+    CollectionAdminRequest.createCollectionWithImplicitRouter(rulesColl, "conf", "shard1", 2)
+        .setRule("port:" + port)
+        .setSnitch("class:ImplicitSnitch")
+        .process(cluster.getSolrClient());
+
+    // now we assert that the replica placement rule is used instead of the cluster policy
+    DocCollection rulesCollection = getCollectionState(rulesColl);
+    List list = (List) rulesCollection.get("rule");
+    assertEquals(1, list.size());
+    assertEquals(port, ((Map) list.get(0)).get("port"));
+    list = (List) rulesCollection.get("snitch");
+    assertEquals(1, list.size());
+    assertEquals ( "ImplicitSnitch", ((Map)list.get(0)).get("class"));
+
+    boolean allOnExpectedPort = rulesCollection.getReplicas().stream().allMatch(replica -> replica.getNodeName().contains(port));
+    assertTrue("Not all replicas were found to be on port: " + port + ". Collection state is: " + rulesCollection, allOnExpectedPort);
+  }
+
   @Test
   public void testPortRule() throws Exception {