[solr] branch branch_9x updated: SOLR-16839: Fix spread-domain bug in AffinityPlacementFactory (#1696)

[tf...@apache.org]

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

tflobbe pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9x by this push:
     new c327f25ecf7 SOLR-16839: Fix spread-domain bug in AffinityPlacementFactory (#1696)
c327f25ecf7 is described below

commit c327f25ecf7a6309ed82982fbbd963f461a25125
Author: Tomas Eduardo Fernandez Lobbe <tf...@apache.org>
AuthorDate: Thu Jun 8 11:52:16 2023 -0700

    SOLR-16839: Fix spread-domain bug in AffinityPlacementFactory (#1696)
    
    This commit fixes a bug where AffinityPlacementFactory would fail to compute placements for shards where one or more replicas are located in down nodes
---
 .../plugins/AffinityPlacementFactory.java          | 11 +++---
 .../plugins/AffinityPlacementFactoryTest.java      | 41 ++++++++++++++++++++++
 2 files changed, 45 insertions(+), 7 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 1ec0de525c7..076fa4a02e6 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
@@ -902,13 +902,10 @@ public class AffinityPlacementFactory implements PlacementPluginFactory<Affinity
               azToNumReplicas.put(az, azToNumReplicas.get(az) + 1);
             }
             if (doSpreadAcrossDomains) {
-              spreadDomainsInUse.merge(
-                  attrValues
-                      .getSystemProperty(
-                          replica.getNode(), AffinityPlacementConfig.SPREAD_DOMAIN_SYSPROP)
-                      .get(),
-                  1,
-                  Integer::sum);
+              attrValues
+                  .getSystemProperty(
+                      replica.getNode(), AffinityPlacementConfig.SPREAD_DOMAIN_SYSPROP)
+                  .ifPresent(nodeDomain -> spreadDomainsInUse.merge(nodeDomain, 1, Integer::sum));
             }
           }
         }
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 0e1665116ac..7cbf0448571 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
@@ -1421,4 +1421,45 @@ public class AffinityPlacementFactoryTest extends SolrTestCaseJ4 {
     assertEquals(
         "group 1 should be greater because of the tie breaker", -1, group1.compareTo(group2));
   }
+
+  @Test
+  public void testSpreadDomainsWithDownNode() throws Exception {
+    defaultConfig.spreadAcrossDomains = true;
+    defaultConfig.maxReplicasPerShardInDomain = -1;
+    configurePlugin(defaultConfig);
+    String collectionName = "basicCollection";
+
+    Builders.ClusterBuilder clusterBuilder = Builders.newClusterBuilder().initializeLiveNodes(3);
+    List<Builders.NodeBuilder> nodeBuilders = clusterBuilder.getLiveNodeBuilders();
+    nodeBuilders.get(0).setSysprop(AffinityPlacementConfig.SPREAD_DOMAIN_SYSPROP, "A");
+    nodeBuilders.get(1).setSysprop(AffinityPlacementConfig.SPREAD_DOMAIN_SYSPROP, "B");
+    nodeBuilders.get(2).setSysprop(AffinityPlacementConfig.SPREAD_DOMAIN_SYSPROP, "A");
+
+    Builders.CollectionBuilder collectionBuilder = Builders.newCollectionBuilder(collectionName);
+    collectionBuilder.initializeShardsReplicas(1, 2, 0, 0, nodeBuilders);
+    clusterBuilder.addCollection(collectionBuilder);
+    clusterBuilder.getLiveNodeBuilders().remove(0);
+
+    PlacementContext placementContext = clusterBuilder.buildPlacementContext();
+    SolrCollection solrCollection = collectionBuilder.build();
+    List<Node> liveNodes = clusterBuilder.buildLiveNodes();
+
+    {
+      // Place a new replica for the (only) existing shard of the collection
+      PlacementRequestImpl placementRequest =
+          new PlacementRequestImpl(
+              solrCollection,
+              Set.of(solrCollection.shards().iterator().next().getShardName()),
+              new HashSet<>(liveNodes),
+              1,
+              0,
+              0);
+
+      PlacementPlan pp = plugin.computePlacement(placementRequest, placementContext);
+
+      assertEquals(1, pp.getReplicaPlacements().size());
+      ReplicaPlacement rp = pp.getReplicaPlacements().iterator().next();
+      assertEquals(liveNodes.get(1), rp.getNode());
+    }
+  }
 }