Skip to content

allocation: create separate limit for frozen tier concurrent rebalance #135243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

allocation: create separate limit for frozen tier concurrent rebalance #135243

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
1b6837e
allocation: create separate limit for frozen tier concurrent rebalance
schase-es Sep 23, 2025
7f683ba
[CI] Update transport version definitions
Sep 23, 2025
56b76c8
[CI] Auto commit changes from spotless
Sep 23, 2025
763bf97
Merge branch 'main' into ES-11303_concurrent-frozen-tier-activity_rebase
schase-es Sep 23, 2025
2ffa4d3
Formatting fixes
schase-es Sep 23, 2025
c8810f2
Adding unlimited test
schase-es Sep 23, 2025
9638803
Addressed or vs. and logic in the context of the allocation algorithm
schase-es Sep 24, 2025
9d111e9
[CI] Auto commit changes from spotless
Sep 24, 2025
3478f43
- Fixed up cluster-wide algorithm for applying frozen tier limits
schase-es Sep 24, 2025
ce8accd
Adding separate test for frozen
schase-es Sep 24, 2025
1ffda8e
[CI] Auto commit changes from spotless
Sep 24, 2025
cb754db
Check RoutingNode.node() for null in frozenNode test
schase-es Sep 24, 2025
eed71f4
Merge branch 'main' into ES-11303_concurrent-frozen-tier-activity_rebase
schase-es Sep 24, 2025
ddf7eee
Addressed review/extra feedback
schase-es Sep 25, 2025
ecde747
Merge branch 'main' into ES-11303_concurrent-frozen-tier-activity_rebase
schase-es Sep 25, 2025
3dde0a6
Adding in mixed frozen and hot test
schase-es Sep 26, 2025
7db6845
Merge branch 'main' into ES-11303_concurrent-frozen-tier-activity_rebase
schase-es Sep 27, 2025
39b9d6b
Merge branch 'main' into ES-11303_concurrent-frozen-tier-activity_rebase
schase-es Oct 8, 2025
6681750
Test fixes
schase-es Oct 8, 2025
03fbb83
Set default concurrent rebalance for frozen to 2
schase-es Oct 9, 2025
05cbcf2
Default to normal setting
schase-es Oct 9, 2025
84bc726
Renaming strategy to allocationService
schase-es Oct 13, 2025
c798ad8
Fixed up decision message formatting, and added test
schase-es Oct 13, 2025
3d52ec1
Merge branch 'main' into ES-11303_concurrent-frozen-tier-activity_rebase
schase-es Oct 13, 2025
84148e7
Slight improvements to the test structure
schase-es Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions server/src/main/java/org/elasticsearch/cluster/routing/RoutingNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,15 @@ public Stream<ShardRouting> shardsWithState(ShardRoutingState state) {
return internalGetShardsWithState(state).stream();
}

/**
* Determine the number of shards with a specific state
* @param state ShardRoutingState which should be listed
* @return number of shards
*/
public int shardCountsWithState(ShardRoutingState state) {
return internalGetShardsWithState(state).size();
}

/**
* Determine the shards of an index with a specific state
* @param index id of the index
Expand Down
77 changes: 67 additions & 10 deletions ...lasticsearch/cluster/routing/allocation/decider/ConcurrentRebalanceAllocationDecider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.cluster.routing.ShardRoutingState;
import org.elasticsearch.cluster.routing.allocation.RoutingAllocation;
import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Setting;
Expand All @@ -20,13 +21,15 @@
/**
* Similar to the {@link ClusterRebalanceAllocationDecider} this
* {@link AllocationDecider} controls the number of currently in-progress
* re-balance (relocation) operations and restricts node allocations if the
* configured threshold is reached. The default number of concurrent rebalance
* operations is set to {@code 2}
* re-balance (shard relocation) operations and restricts node allocations
* if the configured threshold is reached. Frozen and non-frozen shards are
* considered separately. The default number of concurrent rebalance operations
* is set to {@code 2} for non-frozen shards, and {@code 10} for frozen shards.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doc is out of date with the implementation.

* <p>
* Re-balance operations can be controlled in real-time via the cluster update API using
* {@code cluster.routing.allocation.cluster_concurrent_rebalance}. Iff this
* setting is set to {@code -1} the number of concurrent re-balance operations
* {@code cluster.routing.allocation.cluster_concurrent_rebalance} and
* {@code cluster.routing.allocation.cluster_concurrent_frozen_rebalance}.
* Iff either setting is set to {@code -1} the number of concurrent re-balance operations
* are unlimited.
*/
public class ConcurrentRebalanceAllocationDecider extends AllocationDecider {
Expand All @@ -44,18 +47,42 @@ public class ConcurrentRebalanceAllocationDecider extends AllocationDecider {
);
private volatile int clusterConcurrentRebalance;

/**
* Same as cluster_concurrent_rebalance, but applies separately to frozen tier shards
*/
public static final Setting<Integer> CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_FROZEN_REBALANCE_SETTING = Setting.intSetting(
"cluster.routing.allocation.cluster_concurrent_frozen_rebalance",
10,
-1,
Property.Dynamic,
Property.NodeScope
);
private volatile int clusterConcurrentFrozenRebalance;

public ConcurrentRebalanceAllocationDecider(ClusterSettings clusterSettings) {
clusterSettings.initializeAndWatch(
CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE_SETTING,
this::setClusterConcurrentRebalance
);
logger.debug("using [cluster_concurrent_rebalance] with [{}]", clusterConcurrentRebalance);
clusterSettings.initializeAndWatch(
CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_FROZEN_REBALANCE_SETTING,
this::setClusterConcurrentFrozenRebalance
);
logger.debug(
"using [cluster_concurrent_rebalance] with [concurrent_rebalance={}, concurrent_frozen_rebalance={}]",
clusterConcurrentRebalance,
clusterConcurrentFrozenRebalance
);
}

private void setClusterConcurrentRebalance(int concurrentRebalance) {
clusterConcurrentRebalance = concurrentRebalance;
}

private void setClusterConcurrentFrozenRebalance(int concurrentFrozenRebalance) {
clusterConcurrentFrozenRebalance = concurrentFrozenRebalance;
}

@Override
public Decision canRebalance(ShardRouting shardRouting, RoutingAllocation allocation) {
return canRebalance(allocation);
Expand All @@ -68,18 +95,25 @@ public Decision canRebalance(ShardRouting shardRouting, RoutingAllocation alloca
*/
@Override
public Decision canRebalance(RoutingAllocation allocation) {
int relocatingFrozenShards = countRelocatingFrozenShards(allocation);
int relocatingShards = allocation.routingNodes().getRelocatingShardCount();
if (allocation.isSimulating() && relocatingShards >= 2) {
// BalancedShardAllocator is prone to perform unnecessary moves when cluster_concurrent_rebalance is set to high values (>2).
// (See https://github.com/elastic/elasticsearch/issues/87279)
// Above allocator is used in DesiredBalanceComputer. Since we do not move actual shard data during calculation
// it is possible to artificially set above setting to 2 to avoid unnecessary moves in desired balance.
// Separately: keep overall limit in simulation to two including frozen shards
return allocation.decision(Decision.THROTTLE, NAME, "allocation should move one shard at the time when simulating");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, this whole branch might be redundant (?) since #134786

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is helpful -- can we leave this to a separate PR? This may not be the only decider that needs to change.

}
if (clusterConcurrentRebalance == -1) {
boolean normalRelocationUnlimited = clusterConcurrentRebalance == -1;
boolean frozenRelocationUnlimited = clusterConcurrentFrozenRebalance == -1;
if (normalRelocationUnlimited && frozenRelocationUnlimited) {
return allocation.decision(Decision.YES, NAME, "unlimited concurrent rebalances are allowed");
}
if (relocatingShards >= clusterConcurrentRebalance) {

// separate into frozen/non-frozen counts
relocatingShards = relocatingShards - relocatingFrozenShards;
if (normalRelocationUnlimited == false && relocatingShards >= clusterConcurrentRebalance) {
return allocation.decision(
Decision.THROTTLE,
NAME,
Expand All @@ -89,12 +123,35 @@ public Decision canRebalance(RoutingAllocation allocation) {
clusterConcurrentRebalance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the string template doesn't match the parameters now? we used to have three placeholders, now we only have two. I wonder if we should add a unit test for the decider, there doesn't seem to be one already, but I have seen them implemented for other deciders, it would make it easy to test all these branches. See org.elasticsearch.cluster.routing.allocation.decider.WriteLoadConstraintDeciderTests#testWriteLoadDeciderCanAllocate for example?

);
}
if (frozenRelocationUnlimited == false && relocatingFrozenShards >= clusterConcurrentFrozenRebalance) {
return allocation.decision(
Decision.THROTTLE,
NAME,
"reached the limit of concurrently rebalancing frozen shards [%d], cluster setting [%s=%d]",
relocatingFrozenShards,
CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_FROZEN_REBALANCE_SETTING.getKey(),
clusterConcurrentFrozenRebalance
);
}
return allocation.decision(
Decision.YES,
NAME,
"below threshold [%d] for concurrent rebalances, current rebalance shard count [%d]",
"below threshold [%d] for concurrent rebalances, current rebalance shard count [%d], "
+ "and threshold [%d] for concurrent frozen rebalances, current frozen rebalance shard count [%d]",
clusterConcurrentRebalance,
relocatingShards
relocatingShards,
clusterConcurrentFrozenRebalance,
relocatingFrozenShards
);
}

private int countRelocatingFrozenShards(RoutingAllocation allocation) {
int frozenRelocations = 0;
for (var routingNode : allocation.routingNodes()) {
if (routingNode.node().isDedicatedFrozenNode()) {
frozenRelocations += routingNode.shardCountsWithState(ShardRoutingState.RELOCATING);
}
}
return frozenRelocations;
}
Copy link
Contributor

@nicktindall nicktindall Sep 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic for determining what constitutes a frozen shard we should check with wiser heads. We are considering only shards moving off of a dedicated frozen node as being "frozen" shards.

I think this is an acceptable approximation because:

  • We recommend self managed deployments use dedicated frozen nodes and we only have dedicated frozen nodes in ECH (I believe), so "dedicated frozen node" is equivalent to just "frozen node" if things are configured as we recommend. I think it's OK to not allow more rebalancing in the event someone has gone outside our recommendations and created non-dedicated frozen nodes.
    • If I recall correctly, part of the justification for the change was that the frozen and non-frozen nodes are mutually exclusive
    • There is an edge case where we'd count shards relocating from dedicated to non-dedicated frozen nodes, but not vice versa, but again that seems outside of our recommendations and not worth going to great effort to remedy? if we want to we be strict about that we could also check the relocatingNodeId for dedicated frozen-nes?.
  • To determine whether a shard is a frozen shard we need to look at the index metadata, which we could do in canRebalance(shardRouting, allocation) via the cluster state and checking IndexMetadata#getTierPreference(), but it looks like knowledge of specific data tiers lives in x-pack as it stands, and I wonder if we want that to leak out. It looks like tier preference is largely opaque to the rest of the code base, I may have misinterpreted that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to check with someone else, too.

I wrote the frozen/non-frozen counts, so that frozen is a subset count of all relocations. If a relocation isn't counted as frozen, then it's counted as non-frozen. So if something is mistaken, it's just subjected to a tighter constraint.

I can add a flag to ShardRouting, but as we discussed (and you're reminding me here) it may be too far for the data tier concept to propagate.

}
1 change: 1 addition & 0 deletions server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ public void apply(Settings value, Settings current, Settings previous) {
BreakerSettings.CIRCUIT_BREAKER_TYPE,
ClusterRebalanceAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ALLOW_REBALANCE_SETTING,
ConcurrentRebalanceAllocationDecider.CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE_SETTING,
ConcurrentRebalanceAllocationDecider.CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_FROZEN_REBALANCE_SETTING,
EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING,
EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING,
FilterAllocationDecider.CLUSTER_ROUTING_INCLUDE_GROUP_SETTING,
Expand Down
174 changes: 149 additions & 25 deletions ...st/java/org/elasticsearch/cluster/routing/allocation/ConcurrentRebalanceRoutingTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@
import org.elasticsearch.cluster.TestShardRoutingRoleStrategies;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.cluster.routing.RoutingTable;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexVersion;

import java.util.Collections;

import static org.elasticsearch.cluster.routing.RoutingNodesHelper.shardsWithState;
import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING;
import static org.elasticsearch.cluster.routing.ShardRoutingState.RELOCATING;
Expand All @@ -32,18 +35,41 @@
public class ConcurrentRebalanceRoutingTests extends ESAllocationTestCase {

public void testClusterConcurrentRebalance() {
AllocationService strategy = createAllocationService(
Settings.builder()
.put("cluster.routing.allocation.node_concurrent_recoveries", 10)
.put("cluster.routing.allocation.cluster_concurrent_rebalance", 3)
.build()
);
boolean testFrozen = randomBoolean();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's probably worth having a separate method to test frozen? see https://github.com/elastic/elasticsearch/blob/main/TESTING.asciidoc#use-randomized-testing-for-coverage

AllocationService strategy;
if (testFrozen) {
strategy = createAllocationService(
Settings.builder()
.put("cluster.routing.allocation.node_concurrent_recoveries", 10)
.put("cluster.routing.allocation.cluster_concurrent_frozen_rebalance", 3)
.build()
);
} else {
strategy = createAllocationService(
Settings.builder()
.put("cluster.routing.allocation.node_concurrent_recoveries", 10)
.put("cluster.routing.allocation.cluster_concurrent_rebalance", 3)
.build()
);
}

logger.info("Building initial routing table");

Metadata metadata = Metadata.builder()
.put(IndexMetadata.builder("test").settings(settings(IndexVersion.current())).numberOfShards(5).numberOfReplicas(1))
.build();
Metadata metadata;
if (testFrozen) {
metadata = Metadata.builder()
.put(
IndexMetadata.builder("test")
.settings(settings(IndexVersion.current()).put(DataTier.TIER_PREFERENCE, DataTier.DATA_FROZEN))
.numberOfShards(5)
.numberOfReplicas(1)
)
.build();
} else {
metadata = Metadata.builder()
.put(IndexMetadata.builder("test").settings(settings(IndexVersion.current())).numberOfShards(5).numberOfReplicas(1))
.build();
}

RoutingTable initialRoutingTable = RoutingTable.builder(TestShardRoutingRoleStrategies.DEFAULT_ROLE_ONLY)
.addAsNew(metadata.getProject().index("test"))
Expand All @@ -61,9 +87,19 @@ public void testClusterConcurrentRebalance() {
}

logger.info("start two nodes and fully start the shards");
clusterState = ClusterState.builder(clusterState)
.nodes(DiscoveryNodes.builder().add(newNode("node1")).add(newNode("node2")))
.build();
if (testFrozen) {
clusterState = ClusterState.builder(clusterState)
.nodes(
DiscoveryNodes.builder()
.add(newNode("node1", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node2", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
)
.build();
} else {
clusterState = ClusterState.builder(clusterState)
.nodes(DiscoveryNodes.builder().add(newNode("node1")).add(newNode("node2")))
.build();
}
clusterState = strategy.reroute(clusterState, "reroute", ActionListener.noop());

for (int i = 0; i < clusterState.routingTable().index("test").size(); i++) {
Expand All @@ -82,19 +118,35 @@ public void testClusterConcurrentRebalance() {
}

logger.info("now, start 8 more nodes, and check that no rebalancing/relocation have happened");
clusterState = ClusterState.builder(clusterState)
.nodes(
DiscoveryNodes.builder(clusterState.nodes())
.add(newNode("node3"))
.add(newNode("node4"))
.add(newNode("node5"))
.add(newNode("node6"))
.add(newNode("node7"))
.add(newNode("node8"))
.add(newNode("node9"))
.add(newNode("node10"))
)
.build();
if (testFrozen) {
clusterState = ClusterState.builder(clusterState)
.nodes(
DiscoveryNodes.builder(clusterState.nodes())
.add(newNode("node3", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node4", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node5", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node6", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node7", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node8", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node9", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node10", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
)
.build();
} else {
clusterState = ClusterState.builder(clusterState)
.nodes(
DiscoveryNodes.builder(clusterState.nodes())
.add(newNode("node3"))
.add(newNode("node4"))
.add(newNode("node5"))
.add(newNode("node6"))
.add(newNode("node7"))
.add(newNode("node8"))
.add(newNode("node9"))
.add(newNode("node10"))
)
.build();
}
clusterState = strategy.reroute(clusterState, "reroute", ActionListener.noop());

for (int i = 0; i < clusterState.routingTable().index("test").size(); i++) {
Expand Down Expand Up @@ -131,4 +183,76 @@ public void testClusterConcurrentRebalance() {
assertThat(shardsWithState(clusterState.getRoutingNodes(), STARTED).size(), equalTo(10));
assertThat(shardsWithState(clusterState.getRoutingNodes(), RELOCATING).size(), equalTo(0));
}

public void testClusterConcurrentRebalanceFrozenUnlimited() {
AllocationService strategy = createAllocationService(
Settings.builder()
.put("cluster.routing.allocation.node_concurrent_recoveries", 10)
.put("cluster.routing.allocation.cluster_concurrent_frozen_rebalance", -1)
.build()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to set non-frozen recoveries to something very restrictive (like 1) so it's clear those limits are not in play here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nudge on on this one, could set it to zero maybe to disable non-frozen movement altogether?

);

logger.info("Building initial routing table");

Metadata metadata = Metadata.builder()
.put(
IndexMetadata.builder("test")
.settings(settings(IndexVersion.current()).put(DataTier.TIER_PREFERENCE, DataTier.DATA_FROZEN))
.numberOfShards(5)
.numberOfReplicas(1)
)
.build();

RoutingTable initialRoutingTable = RoutingTable.builder(TestShardRoutingRoleStrategies.DEFAULT_ROLE_ONLY)
.addAsNew(metadata.getProject().index("test"))
.build();

ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).routingTable(initialRoutingTable).build();

assertThat(clusterState.routingTable().index("test").size(), equalTo(5));
for (int i = 0; i < clusterState.routingTable().index("test").size(); i++) {
assertThat(clusterState.routingTable().index("test").shard(i).size(), equalTo(2));
assertThat(clusterState.routingTable().index("test").shard(i).shard(0).state(), equalTo(UNASSIGNED));
assertThat(clusterState.routingTable().index("test").shard(i).shard(1).state(), equalTo(UNASSIGNED));
assertThat(clusterState.routingTable().index("test").shard(i).shard(0).currentNodeId(), nullValue());
assertThat(clusterState.routingTable().index("test").shard(i).shard(1).currentNodeId(), nullValue());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: could store clusterState.routingTable().index("test") in a variable?

}

logger.info("start two nodes and fully start the shards");
clusterState = ClusterState.builder(clusterState)
.nodes(
DiscoveryNodes.builder()
.add(newNode("node1", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node2", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
)
.build();
clusterState = strategy.reroute(clusterState, "reroute", ActionListener.noop());

logger.info("start all the primary shards, replicas will start initializing");
clusterState = startInitializingShardsAndReroute(strategy, clusterState);

logger.info("now, start 8 more nodes, and check that no rebalancing/relocation have happened");
clusterState = ClusterState.builder(clusterState)
.nodes(
DiscoveryNodes.builder(clusterState.nodes())
.add(newNode("node3", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node4", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node5", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node6", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node7", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node8", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node9", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
.add(newNode("node10", Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Perhaps de-dup Collections.singleton(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE)

)
.build();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it'd be better to randomise the numbers here to make it clear that 8 isn't in any way significant


clusterState = strategy.reroute(clusterState, "reroute", ActionListener.noop());

logger.info("start the replica shards, rebalancing should start, but without a limit 8 should be rebalancing");
clusterState = startInitializingShardsAndReroute(strategy, clusterState);

// we only allow any number of relocations at a time
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: comment wording?

perhaps mention that 8 will be migrating off to spread shards evenly

assertThat(shardsWithState(clusterState.getRoutingNodes(), STARTED).size(), equalTo(2));
assertThat(shardsWithState(clusterState.getRoutingNodes(), RELOCATING).size(), equalTo(8));
}
}