This is an automated email from the ASF dual-hosted git repository.
janhoy 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 5e58e05f0f3 SOLR-18327: Balance replicas API does not balance
duplicate replicas on same node (#2503)
5e58e05f0f3 is described below
commit 5e58e05f0f34fe2d1c878fd78a3c556297359179
Author: Jan Høydahl <[email protected]>
AuthorDate: Mon Aug 3 09:50:18 2026 +0200
SOLR-18327: Balance replicas API does not balance duplicate replicas on
same node (#2503)
---
.../balance-duplicate-replicas-SOLR-18327.yml | 9 +++
.../src/java/org/apache/solr/cluster/Shard.java | 9 +++
.../plugins/OrderedNodePlacementPlugin.java | 71 ++++++++++++++++++-
.../plugins/AffinityPlacementFactoryTest.java | 81 +++++++++++++++++++++-
.../plugins/MinimizeCoresPlacementFactoryTest.java | 8 ++-
5 files changed, 171 insertions(+), 7 deletions(-)
diff --git a/changelog/unreleased/balance-duplicate-replicas-SOLR-18327.yml
b/changelog/unreleased/balance-duplicate-replicas-SOLR-18327.yml
new file mode 100644
index 00000000000..260b8c51249
--- /dev/null
+++ b/changelog/unreleased/balance-duplicate-replicas-SOLR-18327.yml
@@ -0,0 +1,9 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+
+title: Balance replicas API now enforces the rule of at most one replica per
shard on the same node
+type: fixed
+authors:
+ - name: Jan Høydahl
+links:
+ - name: SOLR-18327
+ url: https://issues.apache.org/jira/browse/SOLR-18327
diff --git a/solr/core/src/java/org/apache/solr/cluster/Shard.java
b/solr/core/src/java/org/apache/solr/cluster/Shard.java
index f62443e897b..cb89e68ad6d 100644
--- a/solr/core/src/java/org/apache/solr/cluster/Shard.java
+++ b/solr/core/src/java/org/apache/solr/cluster/Shard.java
@@ -28,6 +28,15 @@ public interface Shard {
*/
SolrCollection getCollection();
+ /**
+ * @return an id for this shard that is unique across the cluster, combining
the collection name
+ * and the shard name. Useful e.g. as a map key when grouping replicas
of the same shard,
+ * possibly from different collections.
+ */
+ default String getUniqueShardId() {
+ return getCollection().getName() + "%" + getShardName();
+ }
+
/**
* Returns the {@link Replica} of the given name for that shard, if such a
replica exists.
*
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 8eaaa13089e..4165c1e4599 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
@@ -201,14 +201,21 @@ public abstract class OrderedNodePlacementPlugin
implements PlacementPlugin {
public BalancePlan computeBalancing(
BalanceRequest balanceRequest, PlacementContext placementContext) throws
PlacementException {
Map<Replica, Node> replicaMovements = new HashMap<>();
- TreeSet<WeightedNode> orderedNodes = new TreeSet<>();
- orderedNodes.addAll(
+ Collection<WeightedNode> weightedNodes =
getWeightedNodes(
placementContext,
balanceRequest.getNodes(),
placementContext.getCluster().collections(),
true)
- .values());
+ .values();
+
+ // First move replicas that share a node with another replica of the same
shard, a state the
+ // weight-based balancing below cannot necessarily detect since such
duplicates do not have to
+ // make their node weigh more than its peers.
+ moveDuplicateShardReplicas(weightedNodes, replicaMovements);
+
+ TreeSet<WeightedNode> orderedNodes = new TreeSet<>();
+ orderedNodes.addAll(weightedNodes);
// While the node with the lowest weight still has room to take a replica
from the node with the
// highest weight, loop
@@ -305,6 +312,64 @@ public abstract class OrderedNodePlacementPlugin
implements PlacementPlugin {
.createBalancePlan(balanceRequest, replicaMovements);
}
+ /**
+ * Move replicas that share a node with another replica of the same shard to
other nodes, since
+ * multiple replicas of the same shard on one node give neither availability
nor capacity
+ * benefits. Placement will never create such a state, per the default {@link
+ * WeightedNode#canAddReplica(Replica)}, but users can, e.g. by adding a
replica to an explicit
+ * node. Each duplicate replica is moved to the accepting node with the
lowest projected weight
+ * with the replica added, like {@link #computePlacements(Collection,
PlacementContext)} does.
+ */
+ private static void moveDuplicateShardReplicas(
+ Collection<WeightedNode> weightedNodes, Map<Replica, Node>
replicaMovements) {
+ List<WeightedNode> sourceNodes = new ArrayList<>(weightedNodes);
+ sourceNodes.sort(Comparator.comparing(node -> node.getNode().getName()));
+ for (WeightedNode sourceNode : sourceNodes) {
+ Map<String, List<Replica>> replicasPerShard =
+ sourceNode.getAllReplicasOnNode().stream()
+ .collect(Collectors.groupingBy(replica ->
replica.getShard().getUniqueShardId()));
+ for (List<Replica> shardReplicas : replicasPerShard.values()) {
+ if (shardReplicas.size() < 2) {
+ continue;
+ }
+ // Move extra replicas of this shard away until only one remains on
the node, in replica
+ // name order like the weight-based balancing below. A replica that
cannot be removed or has
+ // no eligible target is left in place and does not stop us from
moving the others, so a
+ // single stuck replica no longer leaves the duplicate unresolved.
+ shardReplicas.sort(Comparator.comparing(Replica::getReplicaName));
+ int remainingOnNode = shardReplicas.size();
+ for (Replica replica : shardReplicas) {
+ if (remainingOnNode < 2) {
+ break;
+ }
+ if (!sourceNode.canRemoveReplicas(Set.of(replica)).isEmpty()) {
+ continue;
+ }
+ Optional<WeightedNode> targetNode =
+ weightedNodes.stream()
+ .filter(node -> !node.equals(sourceNode))
+ .filter(node -> node.canAddReplica(replica))
+ .min(
+ Comparator.<WeightedNode>comparingInt(
+ node ->
node.calcRelevantWeightWithReplica(replica))
+ .thenComparing(Comparator.naturalOrder()));
+ if (targetNode.isPresent()) {
+ WeightedNode target = targetNode.get();
+ log.debug(
+ "Duplicate replica movement chosen. From: {}, To: {}, Replica:
{}",
+ sourceNode,
+ target,
+ replica);
+ target.addReplica(replica);
+ sourceNode.removeReplica(replica);
+ replicaMovements.put(replica, target.getNode());
+ remainingOnNode--;
+ }
+ }
+ }
+ }
+ }
+
protected Map<Node, WeightedNode> getWeightedNodes(
PlacementContext placementContext,
Set<Node> 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 0dd6c451b17..8b1d31bb742 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
@@ -1632,7 +1632,82 @@ public class AffinityPlacementFactoryTest extends
AbstractPlacementFactoryTest {
// Each expected placement is represented as a string "col shard
replica-type fromNode ->
// toNode"
- Set<String> expectedPlacements = Set.of("b 1 TLOG 0 -> 2", "b 1 NRT 3 ->
4");
+ // The duplicate replica of shard 1 of collection "a" is first moved off
of node 0, then the
+ // regular weight-based balancing evens out the remaining core counts
+ Set<String> expectedPlacements = Set.of("a 1 NRT 0 -> 4", "a 2 TLOG 3 ->
2");
+ verifyBalancing(
+ expectedPlacements,
+ balancePlan,
+ collectionBuilder.getShardBuilders(),
+ clusterBuilder.buildLiveNodes());
+ }
+
+ @Test
+ public void testBalancingAvoidMultiReplicaOnNode() throws Exception {
+ // Cluster nodes and their attributes
+ Builders.ClusterBuilder clusterBuilder =
Builders.newClusterBuilder().initializeLiveNodes(4);
+ List<Builders.NodeBuilder> nodeBuilders =
clusterBuilder.getLiveNodeBuilders();
+
+ // The collection already exists with shards and replicas
+ Builders.CollectionBuilder collectionBuilder =
Builders.newCollectionBuilder("a");
+ // Note that the collection as defined below is in a state that would NOT
be returned by the
+ // placement plugin: shard 1 has two replicas on node 0 and two replicas
on node 2.
+ // The plugin should still be able to place additional replicas as long as
they don't break the
+ // rules.
+ List<List<String>> shardsReplicas = List.of(List.of("NRT 0", "NRT 0", "NRT
2", "NRT 2"));
+ collectionBuilder.customCollectionSetup(shardsReplicas, nodeBuilders);
+ clusterBuilder.addCollection(collectionBuilder);
+
+ // Add another collection. Note that this is also unbalanced with two
replicas on node 1 and two
+ // replicas on node 3.
+ // The intent of this test is to demonstrate that the plugin can move
replicas so that
+ // a given node does not have more than one replica of a given shard.
+ collectionBuilder = Builders.newCollectionBuilder("b");
+ shardsReplicas = List.of(List.of("NRT 1", "NRT 1", "NRT 3", "NRT 3"));
+ collectionBuilder.customCollectionSetup(shardsReplicas, nodeBuilders);
+ clusterBuilder.addCollection(collectionBuilder);
+
+ BalanceRequestImpl balanceRequest =
+ new BalanceRequestImpl(new HashSet<>(clusterBuilder.buildLiveNodes()));
+ BalancePlan balancePlan =
+ plugin.computeBalancing(balanceRequest,
clusterBuilder.buildPlacementContext());
+
+ // Each expected placement is represented as a string "col shard
replica-type fromNode ->
+ // toNode"
+ Set<String> expectedPlacements =
+ Set.of("a 1 NRT 0 -> 1", "a 1 NRT 2 -> 3", "b 1 NRT 3 -> 2", "b 1 NRT
1 -> 0");
+ verifyBalancing(
+ expectedPlacements,
+ balancePlan,
+ collectionBuilder.getShardBuilders(),
+ clusterBuilder.buildLiveNodes());
+ }
+
+ @Test
+ public void testBalancingAvoidMultiReplicaOnNodeAcrossAZs() throws Exception
{
+ // Cluster with two nodes in each of two availability zones
+ Builders.ClusterBuilder clusterBuilder =
Builders.newClusterBuilder().initializeLiveNodes(4);
+ List<Builders.NodeBuilder> nodeBuilders =
clusterBuilder.getLiveNodeBuilders();
+ for (int i = 0; i < nodeBuilders.size(); i++) {
+ nodeBuilders
+ .get(i)
+ .setSysprop(AffinityPlacementConfig.AVAILABILITY_ZONE_SYSPROP, i < 2
? "az1" : "az2");
+ }
+
+ // Shard 1 has both its replicas on node 0 in az1
+ Builders.CollectionBuilder collectionBuilder =
Builders.newCollectionBuilder("a");
+ List<List<String>> shardsReplicas = List.of(List.of("NRT 0", "NRT 0"));
+ collectionBuilder.customCollectionSetup(shardsReplicas, nodeBuilders);
+ clusterBuilder.addCollection(collectionBuilder);
+
+ BalanceRequestImpl balanceRequest =
+ new BalanceRequestImpl(new HashSet<>(clusterBuilder.buildLiveNodes()));
+ BalancePlan balancePlan =
+ plugin.computeBalancing(balanceRequest,
clusterBuilder.buildPlacementContext());
+
+ // The duplicate replica must move to a node in az2 to keep the
availability zones balanced,
+ // not to the empty az1 node 1
+ Set<String> expectedPlacements = Set.of("a 1 NRT 0 -> 2");
verifyBalancing(
expectedPlacements,
balancePlan,
@@ -1671,7 +1746,9 @@ public class AffinityPlacementFactoryTest extends
AbstractPlacementFactoryTest {
// Each expected placement is represented as a string "col shard
replica-type fromNode ->
// toNode"
- Set<String> expectedPlacements = Set.of("a 1 NRT 3 -> 1", "a 2 NRT 3 ->
0");
+ // The duplicate replica of shard 1 is first moved off of node 0, then the
regular weight-based
+ // balancing evens out the remaining core counts
+ Set<String> expectedPlacements = Set.of("a 1 NRT 0 -> 1", "a 1 NRT 3 ->
2", "a 2 NRT 3 -> 0");
verifyBalancing(
expectedPlacements,
balancePlan,
diff --git
a/solr/core/src/test/org/apache/solr/cluster/placement/plugins/MinimizeCoresPlacementFactoryTest.java
b/solr/core/src/test/org/apache/solr/cluster/placement/plugins/MinimizeCoresPlacementFactoryTest.java
index bb058a734bb..75862df2ca5 100644
---
a/solr/core/src/test/org/apache/solr/cluster/placement/plugins/MinimizeCoresPlacementFactoryTest.java
+++
b/solr/core/src/test/org/apache/solr/cluster/placement/plugins/MinimizeCoresPlacementFactoryTest.java
@@ -260,7 +260,9 @@ public class MinimizeCoresPlacementFactoryTest extends
AbstractPlacementFactoryT
// Each expected placement is represented as a string "col shard
replica-type fromNode ->
// toNode"
- Set<String> expectedPlacements = Set.of("b 1 TLOG 0 -> 2", "b 1 NRT 3 ->
4");
+ // The duplicate replica of shard 1 of collection "a" is first moved off
of node 0, then the
+ // regular weight-based balancing evens out the remaining core counts
+ Set<String> expectedPlacements = Set.of("a 1 NRT 0 -> 4", "a 2 TLOG 3 ->
2");
verifyBalancing(
expectedPlacements,
balancePlan,
@@ -299,7 +301,9 @@ public class MinimizeCoresPlacementFactoryTest extends
AbstractPlacementFactoryT
// Each expected placement is represented as a string "col shard
replica-type fromNode ->
// toNode"
- Set<String> expectedPlacements = Set.of("a 1 NRT 3 -> 1", "a 2 NRT 3 ->
0");
+ // The duplicate replica of shard 1 is first moved off of node 0, then the
regular weight-based
+ // balancing evens out the remaining core counts
+ Set<String> expectedPlacements = Set.of("a 1 NRT 0 -> 1", "a 1 NRT 3 ->
2", "a 2 NRT 3 -> 0");
verifyBalancing(
expectedPlacements,
balancePlan,
