From 29b75a6349bb16423af9459b779f786b12aafae3 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Fri, 13 Mar 2026 07:34:59 -0400 Subject: [PATCH 1/3] Small code fixes while reading through this code. --- .../plugins/AffinityPlacementConfig.java | 10 +++--- .../plugins/AffinityPlacementFactory.java | 7 ++-- .../plugins/OrderedNodePlacementPlugin.java | 36 ++++++------------- .../plugins/RandomPlacementFactory.java | 2 +- .../plugins/AffinityPlacementFactoryTest.java | 1 - 5 files changed, 19 insertions(+), 37 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java index addbb94b3f72..73213ddc8a95 100644 --- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java +++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java @@ -40,14 +40,14 @@ public class AffinityPlacementConfig implements PlacementPluginConfig { * *

Nodes on which this system property is not defined are considered being in the same * Availability Zone {@link #UNDEFINED_AVAILABILITY_ZONE} (hopefully the value of this constant is - * not the name of a real Availability Zone :). + * not the name of a real Availability Zone :) ). */ public static final String AVAILABILITY_ZONE_SYSPROP = "availability_zone"; /** * Name of the system property on a node indicating the type of replicas allowed on that node. The * value of that system property is a comma separated list or a single string of value names of - * {@link org.apache.solr.cluster.Replica.ReplicaType} (case insensitive). If that property is not + * {@link org.apache.solr.cluster.Replica.ReplicaType} (case-insensitive). If that property is not * defined, that node is considered accepting all replica types (i.e. undefined is equivalent to * {@code "NRT,Pull,tlog"}). */ @@ -136,8 +136,8 @@ public class AffinityPlacementConfig implements PlacementPluginConfig { /** * Determines the maximum number of replicas of a particular type of a particular shard that can - * be placed within a single domain (as defined by the @link #SPREAD_DOMAIN_SYSPROP} System - * property. + * be placed within a single domain (as defined by the {@link #SPREAD_DOMAIN_SYSPROP} System + * property). */ @JsonProperty public Integer maxReplicasPerShardInDomain = -1; @@ -163,7 +163,7 @@ public AffinityPlacementConfig(long minimalFreeDiskGB, long prioritizedFreeDiskG * @param prioritizedFreeDiskGB prioritized free disk GB. * @param withCollection configuration of co-located collections: keys are primary collection * names and values are secondary collection names. - * @param collectionNodeType configuration of reequired node types per collection. Keys are + * @param collectionNodeType configuration of required node types per collection. Keys are * collection names and values are comma-separated lists of required node types. */ public AffinityPlacementConfig( 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 6c339aa70946..a630b724e2c2 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 @@ -77,7 +77,7 @@ * }' http://localhost:8983/api/cluster/plugin * * - *

In order to delete the placement-plugin section (and to fallback to either Legacy or rule + *

In order to delete the placement-plugin section (and fallback to either Legacy or rule * based placement if configured for a collection), execute: * * 

@@ -295,7 +295,7 @@ protected Map getBaseWeightedNodes(
         }
       }
 
-      // If there are not multiple spreadDomains, then there is nothing to spread across
+      // only spread across if there are multiple spreadDomains
       if (affinityPlacementContext.allSpreadDomains.size() < 2) {
         affinityPlacementContext.doSpreadAcrossDomains = false;
       }
@@ -307,8 +307,7 @@ AffinityNode newNodeFromMetrics(
         Node node,
         AttributeValues attrValues,
         AffinityPlacementContext affinityPlacementContext,
-        boolean skipNodesWithErrors)
-        throws PlacementException {
+        boolean skipNodesWithErrors) {
       Set supportedReplicaTypes =
           attrValues.getSystemProperty(node, AffinityPlacementConfig.REPLICA_TYPE_SYSPROP).stream()
               .flatMap(s -> Arrays.stream(s.split(",")))
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java
index a353b9dd5e24..d6ae55704d62 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java
@@ -93,7 +93,7 @@ public List computePlacements(
       }
 
       List nodesForRequest =
-          weightedNodes.stream().filter(request::isTargetingNode).collect(Collectors.toList());
+          weightedNodes.stream().filter(request::isTargetingNode).toList();
 
       SolrCollection solrCollection = request.getCollection();
       // Now place all replicas of all shards on available nodes
@@ -241,7 +241,7 @@ public BalancePlan computeBalancing(
         List availableReplicasToMove =
             highestWeight.getAllReplicasOnNode().stream()
                 .sorted(Comparator.comparing(Replica::getReplicaName))
-                .collect(Collectors.toList());
+                .toList();
         int combinedNodeWeights = highestWeight.calcWeight() + lowestWeight.calcWeight();
         for (Replica r : availableReplicasToMove) {
           // Only continue if the replica can be removed from the old node and moved to the new node
@@ -284,7 +284,7 @@ public BalancePlan computeBalancing(
           break;
         }
       }
-      // For now we do not have any way to see if there are out-of-date notes in the middle of the
+      // For now, we do not have any way to see if there are out-of-date notes in the middle of the
       // TreeSet. Therefore, we need to re-sort this list after every selection. In the future, we
       // should find a way to re-sort the out-of-date nodes without having to sort all nodes.
       traversedHighNodes.addAll(orderedNodes);
@@ -340,14 +340,13 @@ protected abstract Map getBaseWeightedNodes(
   public void verifyAllowedModification(
       ModificationRequest modificationRequest, PlacementContext placementContext)
       throws PlacementException {
-    if (modificationRequest instanceof DeleteShardsRequest) {
-      log.warn("DeleteShardsRequest not implemented yet, skipping: {}", modificationRequest);
-    } else if (modificationRequest instanceof DeleteCollectionRequest) {
-      verifyDeleteCollection((DeleteCollectionRequest) modificationRequest, placementContext);
-    } else if (modificationRequest instanceof DeleteReplicasRequest) {
-      verifyDeleteReplicas((DeleteReplicasRequest) modificationRequest, placementContext);
-    } else {
-      log.warn("unsupported request type, skipping: {}", modificationRequest);
+    switch (modificationRequest) {
+      case DeleteShardsRequest deleteShardsRequest ->
+          log.warn("DeleteShardsRequest not implemented yet, skipping: {}", modificationRequest);
+      case DeleteCollectionRequest deleteCollectionRequest ->
+          verifyDeleteCollection(deleteCollectionRequest, placementContext);
+      case DeleteReplicasRequest deleteReplicasRequest -> verifyDeleteReplicas(deleteReplicasRequest, placementContext);
+      case null, default -> log.warn("unsupported request type, skipping: {}", modificationRequest);
     }
   }
 
@@ -442,12 +441,6 @@ public Set getShardsOnNode(String collection) {
       return replicas.getOrDefault(collection, Collections.emptyMap()).keySet();
     }
 
-    public boolean hasShardOnNode(Shard shard) {
-      return replicas
-          .getOrDefault(shard.getCollection().getName(), Collections.emptyMap())
-          .containsKey(shard.getShardName());
-    }
-
     public Set getReplicasForShardOnNode(Shard shard) {
       return Optional.ofNullable(replicas.get(shard.getCollection().getName()))
           .map(m -> m.get(shard.getShardName()))
@@ -775,15 +768,6 @@ public void add(WeightedNode node) {
       }
     }
 
-    /**
-     * Get the number of nodes in the heap.
-     *
-     * @return number of nodes
-     */
-    public int size() {
-      return size;
-    }
-
     /**
      * Check if the heap is empty.
      *
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/RandomPlacementFactory.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/RandomPlacementFactory.java
index 0b2279b34fac..8a16d1af438b 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/RandomPlacementFactory.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/RandomPlacementFactory.java
@@ -31,7 +31,7 @@
 /**
  * Factory for creating {@link RandomPlacementPlugin}, a placement plugin implementing random
  * placement for new collection creation while preventing two replicas of same shard from being
- * placed on same node..
+ * placed on same node.
  *
  * 
See {@link RandomNode} for information on how this PlacementFactory weights nodes.
  *
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 ad0999fc6cf1..eaa1b0a0976f 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
@@ -748,7 +748,6 @@ public void testFreeDiskConstraintsWithNewReplicas() throws Exception {
     int NUM_NODES = 3;
     Builders.ClusterBuilder clusterBuilder =
         Builders.newClusterBuilder().initializeLiveNodes(NUM_NODES);
-    Node smallNode = null;
     for (int i = 0; i < NUM_NODES; i++) {
       Builders.NodeBuilder nodeBuilder = clusterBuilder.getLiveNodeBuilders().get(i);
       // Act as if the two replicas were placed on nodes 1 and 2

From 382fd61c539b4728ec08e7485901797c2c1b4072 Mon Sep 17 00:00:00 2001
From: Eric Pugh 
Date: Mon, 16 Mar 2026 14:58:32 -0400
Subject: [PATCH 2/3] Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---
 .../cluster/placement/plugins/OrderedNodePlacementPlugin.java   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java
index d6ae55704d62..ffeb4aeed85b 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java
@@ -284,7 +284,7 @@ public BalancePlan computeBalancing(
           break;
         }
       }
-      // For now, we do not have any way to see if there are out-of-date notes in the middle of the
+      // For now, we do not have any way to see if there are out-of-date nodes in the middle of the
       // TreeSet. Therefore, we need to re-sort this list after every selection. In the future, we
       // should find a way to re-sort the out-of-date nodes without having to sort all nodes.
       traversedHighNodes.addAll(orderedNodes);

From f80bf987e39604474c220f202f2997df3a297a43 Mon Sep 17 00:00:00 2001
From: Eric Pugh 
Date: Tue, 17 Mar 2026 06:29:37 -0400
Subject: [PATCH 3/3] tidy

---
 .../plugins/AffinityPlacementConfig.java          |  2 +-
 .../plugins/AffinityPlacementFactory.java         |  4 ++--
 .../plugins/OrderedNodePlacementPlugin.java       | 15 ++++++++-------
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
index 73213ddc8a95..ae285dcacb25 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementConfig.java
@@ -40,7 +40,7 @@ public class AffinityPlacementConfig implements PlacementPluginConfig {
    *
    * 
Nodes on which this system property is not defined are considered being in the same
    * Availability Zone {@link #UNDEFINED_AVAILABILITY_ZONE} (hopefully the value of this constant is
-   * not the name of a real Availability Zone :) ).
+   * not the name of a real Availability Zone).
    */
   public static final String AVAILABILITY_ZONE_SYSPROP = "availability_zone";
 
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 a630b724e2c2..12f86137e5d9 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
@@ -77,8 +77,8 @@
  * }' http://localhost:8983/api/cluster/plugin
  *
* - *

In order to delete the placement-plugin section (and fallback to either Legacy or rule - * based placement if configured for a collection), execute: + *

In order to delete the placement-plugin section (and fallback to either Legacy or rule based + * placement if configured for a collection), execute: * * 

  *
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java
index ffeb4aeed85b..9d2741315fab 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java
@@ -340,13 +340,14 @@ protected abstract Map getBaseWeightedNodes(
   public void verifyAllowedModification(
       ModificationRequest modificationRequest, PlacementContext placementContext)
       throws PlacementException {
-    switch (modificationRequest) {
-      case DeleteShardsRequest deleteShardsRequest ->
-          log.warn("DeleteShardsRequest not implemented yet, skipping: {}", modificationRequest);
-      case DeleteCollectionRequest deleteCollectionRequest ->
-          verifyDeleteCollection(deleteCollectionRequest, placementContext);
-      case DeleteReplicasRequest deleteReplicasRequest -> verifyDeleteReplicas(deleteReplicasRequest, placementContext);
-      case null, default -> log.warn("unsupported request type, skipping: {}", modificationRequest);
+    if (modificationRequest instanceof DeleteShardsRequest) {
+      log.warn("DeleteShardsRequest not implemented yet, skipping: {}", modificationRequest);
+    } else if (modificationRequest instanceof DeleteCollectionRequest) {
+      verifyDeleteCollection((DeleteCollectionRequest) modificationRequest, placementContext);
+    } else if (modificationRequest instanceof DeleteReplicasRequest) {
+      verifyDeleteReplicas((DeleteReplicasRequest) modificationRequest, placementContext);
+    } else {
+      log.warn("unsupported request type, skipping: {}", modificationRequest);
     }
   }