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

[tflobbe]

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

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


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

commit 8089e6fc6a9d37299df9a63e5372ec5c235e4f9a
Author: Tomas Eduardo Fernandez Lobbe <tflo...@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 d5802fefa7a..022bd7032b2 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
@@ -947,13 +947,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 66fee222e3d..7cbbf3af8db 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
@@ -1496,4 +1496,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());
+    }
+  }
 }